From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122338 invoked by alias); 16 Dec 2019 23:32:02 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 122324 invoked by uid 89); 16 Dec 2019 23:32:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*MI:sk:16f974f, H*i:sk:16f974f, H*RU:209.85.161.68, H*f:sk:16f974f X-HELO: mail-yw1-f68.google.com Received: from mail-yw1-f68.google.com (HELO mail-yw1-f68.google.com) (209.85.161.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Dec 2019 23:31:56 +0000 Received: by mail-yw1-f68.google.com with SMTP id s187so3241327ywe.10 for ; Mon, 16 Dec 2019 15:31:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=zC7YHt/cM+n6HIgl51VCJq08SWb33MR9xMYocEBNBoY=; b=UcnWdIVT6w/wnuZj+RkmjTzyAN3ZUVI8whSdOIpwAqHRzuEWt/F6AKmipAfBxRQxfC rJXArbwy2CgzrTwluf9/QkkgHmiPSwwTDd3O1pwpbIzVXNldm5Q6xE+ALDGSPwxq1ZMk OAXMlAAClASuiKBoG6d1/eiDhK3QNdmoBsWvmNZTru5Dp2w0otydJGKUd+5PLb6UNeyW xFYoFDXlKk0ihpm49qFqUhW3UpuKXLvZnLelhJMEbi3QS9inXsMLI6dJxUBfBKm+qDCD qxxY//3Q1dhPpY0fGrOvaycTcOj14K3oRsY0BKcSA2y9z6Mp1RHsY6meZJtdm1yEA3S1 BJhw== Return-Path: Received: from [192.168.0.41] (75-166-99-91.hlrn.qwest.net. [75.166.99.91]) by smtp.gmail.com with ESMTPSA id d10sm7800187ywd.107.2019.12.16.15.31.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Dec 2019 15:31:54 -0800 (PST) Subject: Re: [PATCH] add -Wmismatched-tags (PR 61339) To: Jason Merrill , Jakub Jelinek Cc: gcc-patches , Jonathan Wakely References: <2052e38a-aa18-22a4-2f12-bc40bbe8ad6f@gmail.com> <96319d03-ce28-cb20-a0ef-e9b8c6c0acb9@gmail.com> <8fe6adc1-959c-69cc-5542-18bcc8329731@redhat.com> <793a6ab5-90a9-b4ea-b871-2d0b6baad7a3@gmail.com> <0c037e9a-d26e-7557-7b0b-225b7ff70f48@gmail.com> <89471328-6c0b-8eca-cd68-f4b95177ded3@gmail.com> <20191205234708.GW10088@tucnak> <2b18ee83-d8f6-8e89-8495-0a6c375d9c10@redhat.com> <5f89a5e7-8908-e341-1da4-0d94182ecc55@gmail.com> <16f974f3-cfc6-4970-1877-d5b40b9ac564@redhat.com> From: Martin Sebor Message-ID: <931a2d01-985e-abc4-c06c-4e63ac33d31a@gmail.com> Date: Mon, 16 Dec 2019 23:36:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <16f974f3-cfc6-4970-1877-d5b40b9ac564@redhat.com> Content-Type: multipart/mixed; boundary="------------DC8501B08AD7E31413CE4AB0" X-IsSubscribed: yes X-SW-Source: 2019-12/txt/msg01155.txt.bz2 This is a multi-part message in MIME format. --------------DC8501B08AD7E31413CE4AB0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 2233 On 12/16/19 3:51 PM, Jason Merrill wrote: > On 12/9/19 7:29 PM, Martin Sebor wrote: > > Just a few nits: > >> +/* A mapping between a TYPE_DECL for a class and the class_decl_loc_t >> +   description above.  */ >> +typedef hash_map class_to_loc_map_t; >> +static class_to_loc_map_t class2loc; > > I think we can make these members of class_decl_loc_t, now that we > aren't marking them with GTY. > >> +  class_decl_loc_t *rdl = class2loc.get (type_decl); >> +  if (!rdl) >> +    { >> +      tree type = TREE_TYPE (type_decl); >> +      if (def_p || !COMPLETE_TYPE_P (type)) >> +    { >> +      /* TYPE_DECL is the first declaration or definition of the type >> +         (outside precompiled headers -- see below).  Just create >> +         a new entry for it.  */ >> +      class2loc.put (type_decl, class_decl_loc_t (class_key, false, >> def_p)); >> +      return; >> +    } >> + >> +      /* TYPE was previously defined in some unknown precompiled >> hdeader. >> +     Simply add a record of its definition at an unknown location and >> +     proceed below to add a reference to it at the current location. >> +     (Declarations in precompiled headers that are not definitions >> +     are ignored.)  */ >> +      tag_types def_key >> +    = CLASSTYPE_DECLARED_CLASS (type) ? class_type : record_type; >> +      location_t def_loc = DECL_SOURCE_LOCATION (type_decl); >> +      rdl = &class2loc.get_or_insert (type_decl); >> +      *rdl = class_decl_loc_t (def_key, false, true, def_loc); >> +    } > > It seems that you could use get_or_insert at the top, since you always > end up creating an element if there wasn't one already. > >> +  /* A prior declaration of TYPE_DECL has been seen.  */ >> + >> +  if (rdl->idxdef != UINT_MAX && rdl->def_class_key == class_key) >> +    /* Do nothing if the class-key in this declaration matches >> +       the definition.  */ >> +    return; > ... > > I still think that the rest of this function could be a non-static > member function to avoid so many "rdl->". Attached is another revision with these changes. Martin --------------DC8501B08AD7E31413CE4AB0 Content-Type: text/x-patch; name="gcc-61339.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-61339.diff" Content-length: 39736 PR c++/61339 - add warning for mismatch between struct and class gcc/c-family/ChangeLog: PR c++/61339 * c.opt (-Wmismatched-tags, -Wredundant-tags): New options. gcc/cp/ChangeLog: PR c++/61339 * parser.c (cp_parser_maybe_warn_enum_key): New function. (class_decl_loc_t): New class. (cp_parser_elaborated_type_specifier): Call cp_parser_maybe_warn_enum_key. (cp_parser_class_head): Call cp_parser_check_class_key. (cp_parser_check_class_key): Add arguments. Call class_decl_loc_t::add. (c_parse_file): Call class_decl_loc_t::diag_mismatched_tags. gcc/testsuite/ChangeLog: PR c++/61339 * g++.dg/warn/Wmismatched-tags.C: New test. * g++.dg/warn/Wredundant-tags.C: New test. * g++.dg/pch/Wmismatched-tags.C: New test. * g++.dg/pch/Wmismatched-tags.Hs: New test header. gcc/ChangeLog: PR c++/61339 * doc/invoke.texi (-Wmismatched-tags, -Wredundant-tags): Document new C++ options. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 914a2f0ef44..4f3d3cf0d43 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -755,6 +755,10 @@ Wmisleading-indentation C C++ Common Var(warn_misleading_indentation) Warning LangEnabledBy(C C++,Wall) Warn when the indentation of the code does not reflect the block structure. +Wmismatched-tags +C++ Objc++ Var(warn_mismatched_tags) Warning +Warn when a class is redeclared or referenced using a mismatched class-key. + Wmissing-braces C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall) Warn about possibly missing braces around initializers. @@ -783,6 +787,10 @@ Wpacked-not-aligned C ObjC C++ ObjC++ Var(warn_packed_not_aligned) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn when fields in a struct with the packed attribute are misaligned. +Wredundant-tags +C++ Objc++ Var(warn_redundant_tags) Warning +Warn when a class or enumerated type is referenced using a redundant class-key. + Wsized-deallocation C++ ObjC++ Var(warn_sized_deallocation) Warning EnabledBy(Wextra) Warn about missing sized deallocation functions. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 16d1359c47d..d02a4afd736 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -2599,8 +2599,9 @@ static enum tag_types cp_parser_token_is_class_key (cp_token *); static enum tag_types cp_parser_token_is_type_parameter_key (cp_token *); +static void cp_parser_maybe_warn_enum_key (cp_parser *, location_t, tree, rid); static void cp_parser_check_class_key - (enum tag_types, tree type); +(cp_parser *, location_t, enum tag_types, tree type, bool, bool); static void cp_parser_check_access_in_redeclaration (tree type, location_t location); static bool cp_parser_optional_template_keyword @@ -18498,6 +18499,11 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, tree globalscope; cp_token *token = NULL; + /* For class and enum types the location of the class-key or enum-key. */ + location_t key_loc = cp_lexer_peek_token (parser->lexer)->location; + /* For a scoped enum, the 'class' or 'struct' keyword id. */ + rid scoped_key = RID_MAX; + /* See if we're looking at the `enum' keyword. */ if (cp_lexer_next_token_is_keyword (parser->lexer, RID_ENUM)) { @@ -18508,10 +18514,11 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, /* Issue a warning if the `struct' or `class' key (for C++0x scoped enums) is used here. */ cp_token *token = cp_lexer_peek_token (parser->lexer); - if (cp_parser_is_keyword (token, RID_CLASS) - || cp_parser_is_keyword (token, RID_STRUCT)) + if (cp_parser_is_keyword (token, scoped_key = RID_CLASS) + || cp_parser_is_keyword (token, scoped_key = RID_STRUCT)) { - gcc_rich_location richloc (token->location); + location_t loc = token->location; + gcc_rich_location richloc (loc); richloc.add_range (input_location); richloc.add_fixit_remove (); pedwarn (&richloc, 0, "elaborated-type-specifier for " @@ -18519,7 +18526,12 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, token->u.value); /* Consume the `struct' or `class' and parse it anyway. */ cp_lexer_consume_token (parser->lexer); + /* Create a combined location for the whole scoped-enum-key. */ + key_loc = make_location (key_loc, key_loc, loc); } + else + scoped_key = RID_MAX; + /* Parse the attributes. */ attributes = cp_parser_attributes_opt (parser); } @@ -18535,6 +18547,7 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, /* Otherwise it must be a class-key. */ else { + key_loc = cp_lexer_peek_token (parser->lexer)->location; tag_type = cp_parser_class_key (parser); if (tag_type == none_type) return error_mark_node; @@ -18845,13 +18858,18 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, "attributes ignored on elaborated-type-specifier that is not a forward declaration"); } - if (tag_type != enum_type) + if (tag_type == enum_type) + cp_parser_maybe_warn_enum_key (parser, key_loc, type, scoped_key); + else { + /* Diagnose class/struct/union mismatches. */ + cp_parser_check_class_key (parser, key_loc, tag_type, type, false, + cp_parser_declares_only_class_p (parser)); + /* Indicate whether this class was declared as a `class' or as a `struct'. */ - if (CLASS_TYPE_P (type)) + if (CLASS_TYPE_P (type) && !currently_open_class (type)) CLASSTYPE_DECLARED_CLASS (type) = (tag_type == class_type); - cp_parser_check_class_key (tag_type, type); } /* A "<" cannot follow an elaborated type specifier. If that @@ -24389,11 +24407,14 @@ cp_parser_class_head (cp_parser* parser, parser->num_template_parameter_lists); } + /* Diagnose class/struct/union mismatches. */ + cp_parser_check_class_key (parser, UNKNOWN_LOCATION, class_key, type, + true, true); + /* Indicate whether this class was declared as a `class' or as a `struct'. */ if (TREE_CODE (type) == RECORD_TYPE) - CLASSTYPE_DECLARED_CLASS (type) = (class_key == class_type); - cp_parser_check_class_key (class_key, type); + CLASSTYPE_DECLARED_CLASS (type) = class_key == class_type; /* If this type was already complete, and we see another definition, that's an error. Likewise if the type is already being defined: @@ -30617,14 +30638,169 @@ cp_parser_token_is_type_parameter_key (cp_token* token) } } -/* Issue an error message if the CLASS_KEY does not match the TYPE. */ +/* Diagnose redundant enum-keys. */ + +static void +cp_parser_maybe_warn_enum_key (cp_parser *parser, location_t key_loc, + tree type, rid scoped_key) +{ + tree type_decl = TYPE_MAIN_DECL (type); + tree name = DECL_NAME (type_decl); + /* Look up the NAME to see if it unambiguously refers to the TYPE + and set KEY_REDUNDANT if so. */ + tree decl = cp_parser_lookup_name_simple (parser, name, input_location); + + /* The enum-key is redundant for uses of the TYPE that are not + declarations and for which name lookup returns just the type + itself. */ + if (decl == type_decl) + { + gcc_rich_location richloc (key_loc); + richloc.add_fixit_remove (key_loc); + warning_at (&richloc, OPT_Wredundant_tags, + "redundant enum-key % in reference to %q#T", + (scoped_key == RID_CLASS ? " class" + : scoped_key == RID_STRUCT ? " struct" : ""), type); + } +} + +/* Describes the set of declarations of a struct, class, or class template + or its specializations. Used for -Wmismatched-tags. */ + +class class_decl_loc_t +{ + public: + + class_decl_loc_t () + : locvec (), idxdef (), def_class_key () + { + locvec.create (4); + } + + /* Constructs an object for a single declaration of a class with + CLASS_KEY at the current location in the current function (or + at another scope). KEY_REDUNDANT is true if the class-key may + be omitted in the current context without an ambiguity with + another symbol with the same name. + DEF_P is true for a class declaration that is a definition. + CURLOC is the associated location. */ + class_decl_loc_t (tag_types class_key, bool key_redundant, bool def_p, + location_t curloc = input_location) + : locvec (), idxdef (def_p ? 0 : UINT_MAX), def_class_key (class_key) + { + locvec.create (4); + class_key_loc_t ckl (current_function_decl, curloc, class_key, + key_redundant); + locvec.quick_push (ckl); + } + + /* Copy, assign, and destroy the object. Necessary because LOCVEC + isn't safely copyable and assignable and doesn't release storage + on its own. */ + class_decl_loc_t (const class_decl_loc_t &rhs) + : locvec (rhs.locvec.copy ()), idxdef (rhs.idxdef), + def_class_key (rhs.def_class_key) + { } + + class_decl_loc_t& operator= (const class_decl_loc_t &rhs) + { + if (this == &rhs) + return *this; + locvec.release (); + locvec = rhs.locvec.copy (); + idxdef = rhs.idxdef; + def_class_key = rhs.def_class_key; + return *this; + } + + ~class_decl_loc_t () + { + locvec.release (); + } + + /* Issues -Wmismatched-tags for a single class. */ + void diag_mismatched_tags (tree); + + /* Issues -Wmismatched-tags for all classes. */ + static void diag_mismatched_tags (); + + /* Adds TYPE_DECL to the collection of class decls. */ + static void add (tree, tag_types, bool, bool); + + /* Either adds this decl to the collection of class decls + or diagnoses it, whichever is appropriate. */ + void add_or_diag_mismatched_tag (tree, tag_types, bool, bool); + +private: + + tree function (unsigned i) const + { + return locvec[i].func; + } + + location_t location (unsigned i) const + { + return locvec[i].loc; + } + + bool key_redundant (unsigned i) const + { + return locvec[i].key_redundant; + } + + tag_types class_key (unsigned i) const + { + return locvec[i].class_key; + } + + /* The location of a single mention of a class type with the given + class-key. */ + struct class_key_loc_t + { + class_key_loc_t (tree func, location_t loc, tag_types key, bool redundant) + : func (func), loc (loc), class_key (key), key_redundant (redundant) { } + + /* The function the type is mentioned in. */ + tree func; + /* The exact location. */ + location_t loc; + /* The class-key used in the mention of the type. */ + tag_types class_key; + /* True when the class-key could be omitted at this location + without an ambiguity with another symbol of the same name. */ + bool key_redundant; + }; + /* Avoid using auto_vec here since it's not safe to copy due to pr90904. */ + vec locvec; + /* LOCVEC index of the definition or UINT_MAX if none exists. */ + unsigned idxdef; + /* The class-key the class was last declared with or none_type when + it has been declared with a mismatched key. */ + tag_types def_class_key; + + /* A mapping between a TYPE_DECL for a class and the class_decl_loc_t + description above. */ + typedef hash_map class_to_loc_map_t; + static class_to_loc_map_t class2loc; +}; + +class_decl_loc_t::class_to_loc_map_t class_decl_loc_t::class2loc; + +/* Issue an error message if the CLASS_KEY does not match the TYPE. + DEF_P is expected to be set for a definition of class TYPE. DECL_P + is set for a declaration of class TYPE and clear for a reference to + it that is not a declaration of it. */ static void -cp_parser_check_class_key (enum tag_types class_key, tree type) +cp_parser_check_class_key (cp_parser *parser, location_t key_loc, + tag_types class_key, tree type, bool def_p, + bool decl_p) { if (type == error_mark_node) return; - if ((TREE_CODE (type) == UNION_TYPE) != (class_key == union_type)) + + bool seen_as_union = TREE_CODE (type) == UNION_TYPE; + if (seen_as_union != (class_key == union_type)) { if (permerror (input_location, "%qs tag used in naming %q#T", class_key == union_type ? "union" @@ -30632,7 +30808,241 @@ cp_parser_check_class_key (enum tag_types class_key, tree type) type)) inform (DECL_SOURCE_LOCATION (TYPE_NAME (type)), "%q#T was previously declared here", type); + return; } + + if (!warn_mismatched_tags && !warn_redundant_tags) + return; + + tree type_decl = TYPE_MAIN_DECL (type); + tree name = DECL_NAME (type_decl); + /* Look up the NAME to see if it unambiguously refers to the TYPE + and set KEY_REDUNDANT if so. */ + tree decl = cp_parser_lookup_name_simple (parser, name, input_location); + + /* The class-key is redundant for uses of the CLASS_TYPE that are + neither definitions of it nor declarations, and for which name + lookup returns just the type itself. */ + bool key_redundant = !def_p && !decl_p && decl == type_decl; + if (key_redundant) + { + gcc_rich_location richloc (key_loc); + richloc.add_fixit_remove (key_loc); + warning_at (&richloc, OPT_Wredundant_tags, + "redundant class-key %qs in reference to %q#T", + class_key == union_type ? "union" + : class_key == record_type ? "struct" : "class", + type); + } + + if (seen_as_union || !warn_mismatched_tags) + return; + + class_decl_loc_t::add (type_decl, class_key, key_redundant, def_p); +} + +/* Adds TYPE_DECL to the collection of class decls. */ + +void +class_decl_loc_t::add (tree type_decl, tag_types class_key, bool redundant, + bool def_p) +{ + class_decl_loc_t *rdl = class2loc.get (type_decl); + if (!rdl) + { + rdl = &class2loc.get_or_insert (type_decl); + + tree type = TREE_TYPE (type_decl); + if (def_p || !COMPLETE_TYPE_P (type)) + { + /* TYPE_DECL is the first declaration or definition of the type + (outside precompiled headers -- see below). Just create + a new entry for it. */ + *rdl = class_decl_loc_t (class_key, false, def_p); + return; + } + + /* TYPE was previously defined in some unknown precompiled hdeader. + Simply add a record of its definition at an unknown location and + proceed below to add a reference to it at the current location. + (Declarations in precompiled headers that are not definitions + are ignored.) */ + tag_types def_key + = CLASSTYPE_DECLARED_CLASS (type) ? class_type : record_type; + location_t def_loc = DECL_SOURCE_LOCATION (type_decl); + *rdl = class_decl_loc_t (def_key, false, true, def_loc); + } + + /* A prior declaration of TYPE_DECL has been seen. */ + + if (rdl->idxdef != UINT_MAX && rdl->def_class_key == class_key) + /* Do nothing if the class-key in this declaration matches + the definition. */ + return; + + rdl->add_or_diag_mismatched_tag (type_decl, class_key, redundant, def_p); +} + +/* Either adds this DECL corresponding to the TYPE_DECL to the collection + of class decls or diagnoses it, whichever is appropriate. */ + +void +class_decl_loc_t::add_or_diag_mismatched_tag (tree type_decl, + tag_types class_key, + bool redundant, + bool def_p) +{ + /* Reset the CLASS_KEY associated with this type on mismatch. + This is an optimization that lets the diagnostic code skip + over classes that use the same class-key in all declarations. */ + if (def_class_key != class_key) + def_class_key = none_type; + + /* Set IDXDEF to the index of the vector corresponding to + the definition. */ + if (def_p) + idxdef = locvec.length (); + + /* Append a record of this declaration to the vector. */ + class_key_loc_t ckl (current_function_decl, input_location, class_key, + redundant); + locvec.safe_push (ckl); + + if (idxdef == UINT_MAX) + return; + + /* As a space optimization diagnose declarations of a class + whose definition has been seen and purge the LOCVEC of + all entries except the definition. */ + diag_mismatched_tags (type_decl); + if (idxdef) + { + class_decl_loc_t::class_key_loc_t ent = locvec[idxdef]; + locvec.release (); + locvec.reserve (2); + locvec.safe_push (ent); + idxdef = 0; + } + else + /* Pop the entry pushed above for this declaration. */ + locvec.pop (); +} + +/* Issues -Wmismatched-tags for a single class. */ + +void +class_decl_loc_t::diag_mismatched_tags (tree type_decl) +{ + unsigned ndecls = locvec.length (); + + /* Skip a declaration that consistently uses the same class-key + or one with just a solitary declaration (i.e., TYPE_DECL). */ + if (def_class_key != none_type || ndecls < 2) + return; + + /* Save the current function before changing it below. */ + tree save_func = current_function_decl; + /* Set if a class definition for RECLOC has been seen. */ + bool def_p = idxdef < ndecls; + unsigned idxguide = def_p ? idxdef : 0; + unsigned idx = 0; + /* Advance IDX to the first declaration that either is not + a definition or that doesn't match the first declaration + if no definition is provided. */ + while (class_key (idx) == class_key (idxguide)) + if (++idx == ndecls) + return; + + /* The class-key the class is expected to be declared with: it's + either the key used in its definition or the first declaration + if no definition has been provided. */ + tag_types xpect_key = class_key (def_p ? idxguide : 0); + const char *xmatchkstr = xpect_key == record_type ? "class" : "struct"; + const char *xpectkstr = xpect_key == record_type ? "struct" : "class"; + /* Set the function declaration to print in diagnostic context. */ + current_function_decl = function (idx); + + location_t loc = location (idx); + bool key_redundant_p = key_redundant (idx); + auto_diagnostic_group d; + /* Issue a warning for the first mismatched declaration. + Avoid using "%#qT" since the class-key for the same type will + be the same regardless of which one was used in the declaraion. */ + warning_at (loc, OPT_Wmismatched_tags, + "%qT declared with a mismatched class-key %qs", + type_decl, xmatchkstr); + + /* Suggest how to avoid the warning for each instance since + the guidance may be different depending on context. */ + inform (loc, + (key_redundant_p + ? G_("remove the class-key or replace it with %qs") + : G_("replace the class-key with %qs")), + xpectkstr); + + /* Also point to the first declaration or definition that guided + the decision to issue the warning above. */ + inform (location (idxguide), + (def_p + ? G_("%qT defined as %qs here") + : G_("%qT first declared as %qs here")), + type_decl, xpectkstr); + + /* Issue warnings for the remaining inconsistent declarations. */ + for (unsigned i = idx + 1; i != ndecls; ++i) + { + tag_types clskey = class_key (i); + /* Skip over the declarations that match either the definition + if one was provided or the first declaration. */ + if (clskey == xpect_key) + continue; + + loc = location (i); + key_redundant_p = key_redundant (i); + /* Set the function declaration to print in diagnostic context. */ + current_function_decl = function (i); + warning_at (loc, OPT_Wmismatched_tags, + "%qT declared with a mismatched class-key %qs", + type_decl, xmatchkstr); + /* Suggest how to avoid the warning for each instance since + the guidance may be different depending on context. */ + inform (loc, + (key_redundant_p + ? G_("remove the class-key or replace it with %qs") + : G_("replace the class-key with %qs")), + xpectkstr); + } + + /* Restore the current function in case it was replaced above. */ + current_function_decl = save_func; +} + +/* Issues -Wmismatched-tags for all classes. Called at the end + of processing a translation unit, after declarations of all class + types and their uses have been recorded. */ + +void +class_decl_loc_t::diag_mismatched_tags () +{ + /* CLASS2LOC should be empty if -Wmismatched-tags is disabled. */ + gcc_assert (warn_mismatched_tags || class2loc.is_empty ()); + + /* Save the current function before changing it below. It should + be null at this point. */ + tree save_func = current_function_decl; + + /* Iterate over the collected class/struct declarations. */ + typedef class_to_loc_map_t::iterator iter_t; + for (iter_t it = class2loc.begin (); it != class2loc.end (); ++it) + { + tree type_decl = (*it).first; + class_decl_loc_t &recloc = (*it).second; + recloc.diag_mismatched_tags (type_decl); + } + + class2loc.empty (); + /* Restore the current function. */ + current_function_decl = save_func; } /* Issue an error message if DECL is redeclared with different @@ -43065,6 +43475,8 @@ c_parse_file (void) push_deferring_access_checks (flag_access_control ? dk_no_deferred : dk_no_check); cp_parser_translation_unit (the_parser); + class_decl_loc_t::diag_mismatched_tags (); + the_parser = NULL; finish_translation_unit (); diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f04e9151196..216839537de 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -236,7 +236,7 @@ in the following sections. -Wliteral-suffix @gol -Wmultiple-inheritance -Wno-init-list-lifetime @gol -Wnamespaces -Wnarrowing @gol --Wpessimizing-move -Wredundant-move @gol +-Wpessimizing-move -Wredundant-move -Wredundant-tags @gol -Wnoexcept -Wnoexcept-type -Wclass-memaccess @gol -Wnon-virtual-dtor -Wreorder -Wregister @gol -Weffc++ -Wstrict-null-sentinel -Wtemplates @gol @@ -3323,6 +3323,21 @@ treats the return value as if it were designated by an rvalue. This warning is enabled by @option{-Wextra}. +@item -Wredundant-tags @r{(C++ and Objective-C++ only)} +@opindex Wredundant-tags +@opindex Wno-redundant-tags +Warn about redundant class-key and enum-key in references to class types +and enumerated types in contexts where the key can be eliminated without +causing an ambiguity. For example + +@smallexample +struct foo; +struct foo *p; // -Wredundant-tags, keyword struct can be eliminated + +void foo (); // "hides" struct foo +void bar (struct foo&); // no warning, keyword struct cannot be eliminated +@end smallexample + @item -fext-numeric-literals @r{(C++ and Objective-C++ only)} @opindex fext-numeric-literals @opindex fno-ext-numeric-literals @@ -3458,6 +3473,32 @@ The warning is inactive inside a system header file, such as the STL, so one can still use the STL. One may also instantiate or specialize templates. +@item -Wmismatched-tags @r{(C++ and Objective-C++ only)} +@opindex Wmismatched-tags +@opindex Wno-mismatched-tags +Warn for declarations of structs, classes, and class templates and their +specializations with a class-key that does not match either the definition +or the first declaration if no definition is provided. + +For example, the declaration of @code{struct Object} in the argument list +of @code{draw} triggers the warning. To avoid it, either remove the redundant +class-key @code{struct} or replace it with @code{class} to match its definition. +@smallexample +class Object @{ +public: + virtual ~Object () = 0; +@}; +void draw (struct Object*); +@end smallexample + +It is not wrong to declare a class with the class-key @code{struct} as +the example above shows. The @option{-Wmismatched-tags} option is intended +to help achieve a consistent style of class declarations. In code that is +intended to be portable to Windows-based compilers the warning helps prevent +unresolved references due to the difference in the mangling of symbols +declared with different class-keys. The option can be used either on its +own or in conjunction with @option{-Wredundant-tags}. + @item -Wmultiple-inheritance @r{(C++ and Objective-C++ only)} @opindex Wmultiple-inheritance @opindex Wno-multiple-inheritance diff --git a/gcc/testsuite/g++.dg/pch/Wmismatched-tags.C b/gcc/testsuite/g++.dg/pch/Wmismatched-tags.C new file mode 100644 index 00000000000..89b6ba55688 --- /dev/null +++ b/gcc/testsuite/g++.dg/pch/Wmismatched-tags.C @@ -0,0 +1,15 @@ +/* PR c++/61339 - add mismatch between struct and class + Verify that declarations that don't match definitions in precompiled + headers are diagnosed. + { dg-options "-Wall -Wmismatched-tags" } */ + +#include "Wmismatched-tags.H" + +class PCHDeclaredClass; +struct PCHDeclaredStruct; + +struct PCHDefinedClass; // { dg-warning "declared with a mismatched class-key 'struct'" } +class PCHDefinedStruct; // { dg-warning "declared with a mismatched class-key 'class'" } + +class PCHDeclaredClass { }; +struct PCHDeclaredStruct { }; diff --git a/gcc/testsuite/g++.dg/pch/Wmismatched-tags.Hs b/gcc/testsuite/g++.dg/pch/Wmismatched-tags.Hs new file mode 100644 index 00000000000..f4c5dc557e9 --- /dev/null +++ b/gcc/testsuite/g++.dg/pch/Wmismatched-tags.Hs @@ -0,0 +1,7 @@ +class PCHDeclaredClass; + +struct PCHDeclaredStruct; + +class PCHDefinedClass { }; + +struct PCHDefinedStruct { }; \ No newline at end of file diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags.C new file mode 100644 index 00000000000..36a7903234a --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags.C @@ -0,0 +1,278 @@ +/* PR c++/61339 - add mismatch between struct and class + Test to verify that -Wmismatched-tags is issued for declarations + of the same class using different class-ids. + { dg-do compile } + { dg-options "-Wmismatched-tags" } */ + +namespace Classes +{ +class A; +class A; + +struct B; +struct B; + +union C; +union C; + +struct D; // { dg-warning "Classes::D' declared with a mismatched class-key 'struct'" } +class D { }; // { dg-message "Classes::D' defined as 'class' here" } + +class E; // { dg-warning "Classes::E' declared with a mismatched class-key 'class'" } +struct E { }; // { dg-message "Classes::E' defined as 'struct' here" } + +class D; +struct E; + +class D; +struct E; + +struct D; // { dg-warning "Classes::D' declared with a mismatched class-key" } + +class E; // { dg-warning "Classes::E' declared with a mismatched class-key" } + +class F; // { dg-message "Classes::F' first declared as 'class' here" } +class F; + +struct G { }; // { dg-message "Classes::G' defined as 'struct' here" } +} // namespace Classes + + +namespace Classes +{ +class A; +struct B; +union C; +class D; +struct E; + +struct F; // { dg-warning "Classes::F' declared with a mismatched class-key" } + +struct G; +} + +// Verify that the correct hint is provided, one to remove the class-key +// when it's redundant, and one to (only) replace it with the correct one +// when it's needed to disambiguate the reference to the class type. +namespace RemoveOrReplace +{ +struct Func; +class Func; // { dg-warning "RemoveOrReplace::Func' declared with a mismatched class-key 'class'" } + // { dg-message "replace the class-key with 'struct'" "hint to remove" { target *-*-* } .-1 } + +void Func (); + +class Func; // { dg-warning "RemoveOrReplace::Func' declared with a mismatched class-key 'class'" } + // { dg-message "replace the class-key with 'struct'" "hint to replace" { target *-*-* } .-1 } + +class Var; +struct Var; // { dg-warning "RemoveOrReplace::Var' declared with a mismatched class-key 'struct'" } + // { dg-message "replace the class-key with 'class'" "hint to remove" { target *-*-* } .-1 } +void f (struct Var*); // { dg-warning "RemoveOrReplace::Var' declared with a mismatched class-key 'struct'" } + // { dg-message "remove the class-key or replace it with 'class'" "hint to remove" { target *-*-* } .-1 } + +int Var; + +struct Var; // { dg-warning "RemoveOrReplace::Var' declared with a mismatched class-key 'struct'" } + // { dg-message "replace the class-key with 'class'" "hint to replace" { target *-*-* } .-1 } +} + +namespace GlobalObjects +{ +class A; // { dg-message "'GlobalObjects::A' first declared as 'class' here" } +struct B; // { dg-message "'GlobalObjects::B' first declared as 'struct' here" } +class C { }; // { dg-message "'GlobalObjects::C' defined as 'class' here" } + +extern A a0; +extern class A a1; +extern class A a2; + +extern B b0; +extern struct B b1; +extern struct B b2; + +extern struct A a3; // { dg-warning "GlobalObjects::A' declared with a mismatched class-key" } +extern class A a4; + +extern class B b3; // { dg-warning "GlobalObjects::B' declared with a mismatched class-key" } +extern struct B b4; + +extern struct C c[]; // { dg-warning "GlobalObjects::C' declared with a mismatched class-key" } + // { dg-message "remove the class-key or replace it with 'class'" "hint to remove" { target *-*-* } .-1 } + +extern char +arr[sizeof (struct C)]; // { dg-warning "GlobalObjects::C' declared with a mismatched class-key" } + // { dg-message "remove the class-key or replace it with 'class'" "hint to remove" { target *-*-* } .-1 } +} // namespace GlobalObjects + + +namespace LocalObjects +{ +class A; // { dg-message "LocalObjects::A' first declared as 'class' here" } +struct B; // { dg-message "LocalObjects::B' first declared as 'struct' here" } + +void f (A*, B&) +{ + class A *a1; + class A *a2; + + struct B *b1; + struct B *b2; + + struct A *a3; // { dg-warning "LocalObjects::A' declared with a mismatched class-key" } + class A *a4; + + class B *b3; // { dg-warning "LocalObjects::B' declared with a mismatched class-key" } + struct B *b4; +} + +void g (struct A*); // { dg-warning "LocalObjects::A' declared with a mismatched class-key" } + +} // namespace LocalObjects + + +namespace MemberClasses +{ +struct A { struct B; }; +struct C { struct D; struct D; struct D { }; }; +struct E { class F; class F { }; class F; }; + +struct G { + struct H; // { dg-message "MemberClasses::G::H' first declared as 'struct' here" } + class H; // { dg-warning "MemberClasses::G::H' declared with a mismatched class-key" } + class I { }; // { dg-message "MemberClasses::G::I' defined as 'class' here" } + struct I; // { dg-warning "MemberClasses::G::I' declared with a mismatched class-key" } +}; +} // namespace MemberClasses + + +namespace DataMembers +{ +struct A { struct B *p; }; +struct C { struct D *p; struct D *q; struct D { } d; }; +struct E { class F &r; class F { } f; class F *p; }; + +class G; // { dg-message "DataMembers::G' first declared as 'class' here" } +struct H; // { dg-message "DataMembers::H' first declared as 'struct' here" } + +struct I { + struct G *p0; // { dg-warning "DataMembers::G' declared with a mismatched class-key" } + class G *p1; + + struct H &r0; + class H &r1; // { dg-warning "DataMembers::H' declared with a mismatched class-key" } + + class J { }; // { dg-message "DataMembers::I::J' defined as 'class' here" } + struct K { }; // { dg-message "DataMembers::I::K' defined as 'struct' here" } + + class J j0; + class K k0; // { dg-warning "DataMembers::I::K' declared with a mismatched class-key" } + + struct J j1; // { dg-warning "DataMembers::I::J' declared with a mismatched class-key" } + struct K k1; +}; +} // namespace DataMembers + + +namespace Templates +{ +template class A; +template class A; + +template struct B; +template struct B; + +template union C; +template union C; + +template struct D; // { dg-warning "Templates::D\[^\n\r]*' declared with a mismatched class-key" } +template +class D // { dg-message "Templates::D\[^\n\r]*' defined as 'class' here" } +{ public: D (); }; + +template class E; // { dg-warning "Templates::E\[^\n\r]*' declared with a mismatched class-key" } +template +struct E // { dg-message "Templates::E\[^\n\r]*' defined as 'struct' here" } +{ int i; }; + +template class D; +template struct E; + +template +struct D; // { dg-warning "Templates::D\[^\n\r]*' declared with a mismatched class-key" } + // { dg-message "replace the class-key with 'class'" "hint" { target *-*-* } .-1 } +} // namespace Templates + + +namespace ExplicitSpecializations +{ +template class A; +template <> class A<0>; +template <> struct A<1>; +template <> struct A<1> { }; + +template struct B; +template <> struct B<0>; +template <> class B<1>; +template <> class B<2> { public: B (); }; + +template union C; +template <> union C<0>; + +template class D; +template <> class D<0>; // { dg-warning "ExplicitSpecializations::D\[^\n\r]*' declared with a mismatched class-key " } +template <> +struct D<0> { }; // { dg-message "ExplicitSpecializations::D\[^\n\r]*' defined as 'struct' here" } + +template struct E; +template <> struct E<0>; // { dg-warning "ExplicitSpecializations::E\[^\n\r]*' declared with a mismatched class-key" } +template <> +class E<0> { }; // { dg-message "ExplicitSpecializations::E\[^\n\r]*' defined as 'class' here" } + +template struct F; +template <> class F<0> { }; // { dg-message "ExplicitSpecializations::F\[^\n\r]*' defined as 'class' here" } + +template <> +struct F<0>; // { dg-warning "ExplicitSpecializations::F\[^\n\r]*' declared with a mismatched class-key" } +} // namespace ExplicitSpecializations + + +namespace PartialSpecializations +{ +template class A; +template struct A; +template struct A; + +template struct B; +template class B; +template class B; + +template class C { }; +template struct C { }; +template struct C { }; + +template struct D { }; +template class D { }; +template class D { }; + +template class E; +template +struct E; // { dg-message "PartialSpecializations::E' first declared as 'struct' here" } + +template +class E; // { dg-warning "PartialSpecializations::E' declared with a mismatched class-key" } + +template class F; +template +class F; // { dg-message "PartialSpecializations::F' first declared as 'class' here" } +template +struct F; // { dg-warning "PartialSpecializations::F' declared with a mismatched class-key" } +} // namespace PartialSpecializations + + +namespace Classes +{ +struct G; + +class G; // { dg-warning "Classes::G' declared with a mismatched class-key 'class'" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags.C new file mode 100644 index 00000000000..ac5afa912a1 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags.C @@ -0,0 +1,128 @@ +/* PR c++/61339 - add mismatch between struct and class + Test to verify that -Wredundant-tags is issued for references to class + types that use the class-key even though they don't need to. + { dg-do compile } + { dg-options "-Wredundant-tags" } */ + +struct A; + +extern A *pa; +extern struct A *pa; // { dg-warning "redundant class-key 'struct' in reference to 'struct A'" } + +extern A aa[]; +extern struct A aa[]; // { dg-warning "redundant class-key 'struct' in reference to 'struct A'" } + +void func (A*); +void func (struct A*); // { dg-warning "redundant class-key 'struct' in reference to 'struct A'" } + +int A; + +extern struct A *pa; +extern struct A aa[]; +void func (struct A*); + + +class B; + +extern B *pb; +extern class B *pb; // { dg-warning "redundant class-key 'class' in reference to 'class B'" } + +extern B ab[]; +extern class B ab[]; // { dg-warning "redundant class-key 'class' in reference to 'class B'" } + +void func (B*); +void func (class B*); // { dg-warning "redundant class-key 'class' in reference to 'class B'" } + +int B; + +extern class B *pb; +extern class B ab[]; +void func (class B*); + + +enum C { c0 }; + +extern C *pc; +extern enum C *pc; // { dg-warning "redundant enum-key 'enum' in reference to 'enum C'" } + +extern C ac[]; +extern enum C ac[]; // { dg-warning "redundant enum-key 'enum' in reference to 'enum C'" } + +void func (C*); +void func (enum C*); // { dg-warning "redundant enum-key 'enum' in reference to 'enum C'" } + +int C; + +extern enum C *pc; +extern enum C ac[]; +void func (enum C*); + + +#if __cplusplus > 199711L + +enum class D1 { d1 }; +enum struct D2 { d2 }; + +#else + +enum D1 { d1 }; +enum D2 { d2 }; + +#endif + +extern D1 *pd1; +extern D2 *pd2; +extern enum D1 *pd1; // { dg-warning "redundant enum-key 'enum' in reference to 'enum class D1'" "C++ 11 and above" { target c++11 } } + // { dg-warning "redundant enum-key 'enum' in reference to 'enum D1'" "C++ 98" { target c++98_only } .-1 } + +extern enum D2 *pd2; // { dg-warning "redundant enum-key 'enum' in reference to 'enum class D2'" "C++ 11 and above" { target c++11 } } + // { dg-warning "redundant enum-key 'enum' in reference to 'enum D2'" "C++ 98" { target c++98_only } .-1 } + +extern D1 ad1[]; +extern D2 ad2[]; + +#if __cplusplus > 199711L +extern enum class D1 ad1[]; // { dg-warning "redundant enum-key 'enum class' in reference to 'enum class D1'" "C++ 11 and above" { target c++11 } } + // { dg-warning "elaborated-type-specifier for a scoped enum must not use the 'class' keyword" "C++ 11 and above" { target c++11 } .-1 } +/* The pretty printer cannot differentiate between enum class and enum struct + because the C++ front-end doesn't encode it so allow for both in the text + of the warning below. */ +extern enum struct D2 ad2[]; // { dg-warning "redundant enum-key 'enum struct' in reference to 'enum \(class|struct\) D2'" "C++ 11 and above" { target c++11 } } + // { dg-warning "elaborated-type-specifier for a scoped enum must not use the 'struct' keyword" "C++ 11 and above" { target c++11 } .-1 } +#else +extern enum D1 ad1[]; // { dg-warning "redundant enum-key 'enum' in reference to 'enum D1'" "C++ 98" { target c++98_only } } +#endif + +void func (D1*); +void func (enum D1*); // { dg-warning "redundant enum-key 'enum' in reference to 'enum " } + +void func (D2*); +void func (enum D2*); // { dg-warning "redundant enum-key 'enum' in reference to 'enum " } + +int D1, D2; + +extern enum D1 *pd1; +extern enum D1 ad1[]; +void func (enum D1*); + +extern enum D2 *pd2; +extern enum D2 ad2[]; +void func (enum D2*); + + +union U; + +extern U *pu; +extern union U *pu; // { dg-warning "redundant class-key 'union' in reference to 'union U'" } + +extern U au[]; +extern union U au[]; // { dg-warning "redundant class-key 'union' in reference to 'union U'" } + +void func (U*); +void func (union U*); // { dg-warning "redundant class-key 'union' in reference to 'union U'" } + +int U; + +extern union U *pu; +extern union U au[]; +void func (union U*); --------------DC8501B08AD7E31413CE4AB0--