From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf42.google.com (mail-qv1-xf42.google.com [IPv6:2607:f8b0:4864:20::f42]) by sourceware.org (Postfix) with ESMTPS id 1DC4F387701C for ; Thu, 12 Mar 2020 22:38:48 +0000 (GMT) Received: by mail-qv1-xf42.google.com with SMTP id fc12so3561407qvb.6 for ; Thu, 12 Mar 2020 15:38:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=FeOyvquWzmTGvjezi84zQZxNUQVRbMZA31MjwJhWH8c=; b=P9kpBUkZeTFrVV7PQFSW3U3AK9LOl5C2K8qcFhouHghmnVrX/lhjW83HmUERGhB5vf lG06D5KReHxnEgr58b7SfbV0WpM7zznHHXrjBRLUGd7Ym571cCHqHNDxK7lUv7SHd/ey cAIsgIvUvN7NO/St1OsboaMQI0D7eXbJzEH67eOxL1jf52uKdpoGlkZ/1nxRQW3Mx0zc wUU/5mQiEBHZKT4lZvLM973CgVPJ8W5CxkhnmvGPEMxbwEWdxDhExk03H4PXLPlZw+cb SrGhWmjf0xK8/N5bIgF5L4UsUwYTYd9NPCddUaYxkIk6e/E3HZyhdqPPVHrMojeZyBWQ eRFw== X-Gm-Message-State: ANhLgQ2/mCgmz3dV1R4FzW/LpD6z0URYcN4S/aM8Oj8UzLcBxsweAZP7 UYt8Z1BSqC9kbBVtPXi8JdKMGqUR X-Google-Smtp-Source: ADFU+vv7DdPyA8RPdL7TW2RgAMiQyS90hzB4p0W2DyCx+hCbKBsSOCYAjN2bpuZexXzPYQ3utfsdYQ== X-Received: by 2002:a0c:ec07:: with SMTP id y7mr6034183qvo.142.1584052727291; Thu, 12 Mar 2020 15:38:47 -0700 (PDT) Received: from [192.168.0.41] (97-118-124-45.hlrn.qwest.net. [97.118.124.45]) by smtp.gmail.com with ESMTPSA id z9sm9569309qtu.20.2020.03.12.15.38.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 12 Mar 2020 15:38:46 -0700 (PDT) Subject: Re: [PATCH] avoid -Wredundant-tags on a first declaration in use (PR 93824) From: Martin Sebor To: Jason Merrill , gcc-patches References: <010f6d68-a64e-45a4-744e-c040a4ea94d6@redhat.com> <65013bfc-23c3-47fb-a58d-9ab802d1febb@gmail.com> <2698a399-4176-2b5f-a134-52a0d82c2121@redhat.com> <13b8faa9-74b1-6131-5dda-d4f34dfa8af0@gmail.com> <77dc94af-40f1-46fd-f3f4-ceb7e3afc4b6@gmail.com> <00f21176-ea92-937c-5c8a-c04d5d813257@redhat.com> <90a6aebc-8fef-0b96-cd2b-3234bd82b9a0@gmail.com> <2c553c0d-c0ac-88ba-2a85-12c523a9a164@redhat.com> <2744364d-e705-06f8-064a-5425ade11669@gmail.com> Message-ID: Date: Thu, 12 Mar 2020 16:38:45 -0600 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: <2744364d-e705-06f8-064a-5425ade11669@gmail.com> Content-Type: multipart/mixed; boundary="------------389E6CD266C71115B8CDB6B8" Content-Language: en-US X-Spam-Status: No, score=-24.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GARBLED_BODY, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Mar 2020 22:38:50 -0000 This is a multi-part message in MIME format. --------------389E6CD266C71115B8CDB6B8 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 3/12/20 11:03 AM, Martin Sebor wrote: > On 3/11/20 3:30 PM, Martin Sebor wrote: >> On 3/11/20 2:10 PM, Jason Merrill wrote: >>> On 3/11/20 12:57 PM, Martin Sebor wrote: >>>> On 3/9/20 6:08 PM, Jason Merrill wrote: >>>>> On 3/9/20 5:39 PM, Martin Sebor wrote: >>>>>> On 3/9/20 1:40 PM, Jason Merrill wrote: >>>>>>> On 3/9/20 12:31 PM, Martin Sebor wrote: >>>>>>>> On 2/28/20 1:24 PM, Jason Merrill wrote: >>>>>>>>> On 2/28/20 12:45 PM, Martin Sebor wrote: >>>>>>>>>> On 2/28/20 9:58 AM, Jason Merrill wrote: >>>>>>>>>>> On 2/24/20 6:58 PM, Martin Sebor wrote: >>>>>>>>>>>> -Wredundant-tags doesn't consider type declarations that are >>>>>>>>>>>> also >>>>>>>>>>>> the first uses of the type, such as in 'void f (struct S);' and >>>>>>>>>>>> issues false positives for those.  According to the reported >>>>>>>>>>>> that's >>>>>>>>>>>> making it harder to use the warning to clean up LibreOffice. >>>>>>>>>>>> >>>>>>>>>>>> The attached patch extends -Wredundant-tags to avoid these >>>>>>>>>>>> false >>>>>>>>>>>> positives by relying on the same class_decl_loc_t::class2loc >>>>>>>>>>>> mapping >>>>>>>>>>>> as -Wmismatched-tags.  The patch also somewhat improves the >>>>>>>>>>>> detection >>>>>>>>>>>> of both issues in template declarations (though more work is >>>>>>>>>>>> still >>>>>>>>>>>> needed there). >>>>>>>>>>> >>>>>>>>>>>> +         a new entry for it and return unless it's a >>>>>>>>>>>> declaration >>>>>>>>>>>> +         involving a template that may need to be diagnosed by >>>>>>>>>>>> +         -Wredundant-tags.  */ >>>>>>>>>>>>        *rdl = class_decl_loc_t (class_key, false, def_p); >>>>>>>>>>>> -      return; >>>>>>>>>>>> +      if (TREE_CODE (decl) != TEMPLATE_DECL) >>>>>>>>>>>> +        return; >>>>>>>>>>> >>>>>>>>>>> How can the first appearance of a class template be redundant? >>>>>>>>>> >>>>>>>>>> I'm not sure I correctly understand the question.  The comment >>>>>>>>>> says >>>>>>>>>> "involving a template" (i.e., not one of the first declaration of >>>>>>>>>> a template).  The test case that corresponds to this test is: >>>>>>>>>> >>>>>>>>>>    template struct S7 { }; >>>>>>>>>>    struct S7 s7v;  // { dg-warning "\\\[-Wredundant-tags" } >>>>>>>>>> >>>>>>>>>> where DECL is the TEPLATE_DECL of S7. >>>>>>>>>> >>>>>>>>>> As I mentioned, more work is still needed to handle templates >>>>>>>>>> right >>>>>>>>>> because some redundant tags are still not diagnosed.  For >>>>>>>>>> example: >>>>>>>>>> >>>>>>>>>>    template struct S7 { }; >>>>>>>>>>    template >>>>>>>>>>    using U = struct S7;   // missing warning >>>>>>>>> >>>>>>>>> When we get here for an instance of a template, it doesn't make >>>>>>>>> sense to treat it as a new type. >>>>>>>>> >>>>>>>>> If decl is a template and type_decl is an instance of that >>>>>>>>> template, do we want to (before the lookup) change type_decl to >>>>>>>>> the template or the corresponding generic TYPE_DECL, which >>>>>>>>> should already be in the table? >>>>>>>> >>>>>>>> I'm struggling with how to do this.  Given type (a RECORD_TYPE) and >>>>>>>> type_decl (a TEMPLATE_DECL) representing the use of a template, how >>>>>>>> do I get the corresponding template (or its explicit or partial >>>>>>>> specialization) in the three cases below? >>>>>>>> >>>>>>>>    1) Instance of the primary: >>>>>>>>       template class A; >>>>>>>>       struct A a; >>>>>>>> >>>>>>>>    2) Instance of an explicit specialization: >>>>>>>>       template class B; >>>>>>>>       template <> struct B; >>>>>>>>       class B b; >>>>>>>> >>>>>>>>    3) Instance of a partial specialization: >>>>>>>>       template class C; >>>>>>>>       template struct C; >>>>>>>>       class C c; >>>>>>>> >>>>>>>> By trial and (lots of) error I figured out that in both (1) and >>>>>>>> (2), >>>>>>>> but not in (3), TYPE_MAIN_DECL (TYPE_TI_TEMPLATE (type)) returns >>>>>>>> the template's type_decl. >>>>>>>> >>>>>>>> Is there some function to call to get it in (3), or even better, >>>>>>>> in all three cases? >>>>>>> >>>>>>> I think you're looking for most_general_template. >>>>>> >>>>>> I don't think that's quite what I'm looking for.  At least it doesn't >>>>>> return the template or its specialization in all three cases above. >>>>> >>>>> Ah, true, that function stops at specializations.  Oddly, I don't >>>>> think there's currently a similar function that looks through them. >>>>> You could create one that does a simple loop through >>>>> DECL_TI_TEMPLATE like is_specialization_of. >>>> >>>> Thanks for the tip.  Even with that I'm having trouble with partial >>>> specializations.  For example in: >>>> >>>>    template    struct S; >>>>    template class S; >>>>    extern class  S s1; >>>>    extern struct S s2;  // expect -Wmismatched-tags >>>> >>>> how do I find the declaration of the partial specialization when given >>>> the type in the extern declaration?  A loop in my find_template_for() >>>> function (similar to is_specialization_of) only visits the implicit >>>> specialization S (i.e., its own type) and the primary. >>> >>> Is that a problem?  The name is from the primary template, so does it >>> matter for this warning whether there's an explicit specialization >>> involved? >> >> I don't understand the question.  S is an instance of >> the partial specialization.  To diagnose the right mismatch the warning >> needs to know how to find the template (i.e., either the primary, or >> the explicit or partial specialization) the instance corresponds to and >> the class-key it was declared with.  As it is, while GCC does diagnose >> the right declaration (that of s2), it does that thanks to a bug: >> because it finds and uses the type and class-key used to declare s1. >> If we get rid of s1 it doesn't diagnose anything. >> >> I tried using DECL_TEMPLATE_SPECIALIZATIONS() to get the list of >> the partial specializations but it doesn't like any of the arguments >> I've given it (it ICEs). > > With this fixed, here's the algorithm I tried: > > 1) for a type T of a template instantiation (s1 above), get the primary >    P that T was instantiated from using >    P = TYPE_MAIN_DECL (CLASSTYPE_PRIMARY_TEMPLATE_TYPE (T)), > 2) from P, get the chain of its specializations using >    SC = DECL_TEMPLATE_SPECIALIZATIONS (P) > 3) for each (partial) specialization S on the SC chain get the chain >    of its instantiations IC using DECL_TEMPLATE_INSTANTIATIONS, if >    is_specialization_of (T, TREE_VALUE (IC)) is non-zero take >    TREE_VALUE (SC) as the declaration of the partial specialization >    that the template instanstantiaton T was generated from. > > Unfortunately, in the example above, DECL_TEMPLATE_INSTANTIATIONS for > the partial specialization 'class S' is null (even after all > the declarations have been parsed) so I'm at a dead end. After a lot more trial and error I discovered most_specialized_partial_spec in pt.c with whose help I have been able to get templates to work the way I think they should (at least the cases I've tested do). Besides fixing the original problem that motivated it, the attached patch also corrects how template specializations are handled: the first declaration of either a primary template or its specialization (either explicit or partial) determines the class-key in subsequent uses of the type or its instantiations. To do this for uses of first-time template instantiations such as in the declaration of s1 in the test case above, class_decl_loc_t::diag_mismatched_tags looks up the template (either the primary or the partial specialization) in the CLASS2LOC map and uses it and its class-key as the guide when issuing diagnostics. This also means that the first instance of every template needs to have a record in the CLASS2LOC map (it also needs it to compare its class-key to subsequent redeclarations of the template). This has been unexpectedly difficult. A big part of it is that I've never before worked with templates in the front-end so I had to learn how they're organized (I'm far from having mastered it). What's made the learning curve especially steep, besides the sparse documentation, is the problems hinted at in the paragraph below below. This whole area could really stand to be documented in some sort of a writeup: a high-level overview of how templates are put together (i.e., what hangs off what in the tree representation) and what APIs to use to work with them. The revised patch has been tested on x86_64-linux. Martin > The other recurring problem I'm running into is that many of the C++ > FE macros (and APIs) don't always return the expected/documented result. > I think this is at least in part because until a declaration has been > fully parsed not all the bits are in place to make it possible to tell > if it's an implicit or explicit specialization, for example (so > CLASSTYPE_USE_TEMPLATE (T) is 1 for the first-time declaration of > an explicit specialization, for example).  But in view of the problem > above I'm not sure that's the only reason. > >> >> Martin >> >> PS As an aside, both Clang and VC++ have trouble with templates as >> well, just slightly different kinds of trouble.   Clang diagnoses >> the declaration of the partial specialization while VC++ diagnoses >> s1.  In other similar cases like the one below VC++ does the right >> thing and Clang is silent. >> >>    template    struct S { }; >>    template class S { }; >> >>    template void f (class S);          // VC++ warns >>    template void g (struct S);   // GCC & VC++ warn >> >>> >>>> Martin >>>> >>>>> >>>>>> In (2) and (3) it won't distinguish between specializations of B or >>>>>> C on different types.  In (2), the function returns the same result >>>>>> for both: >>>>>> >>>>>>    template <> struct B; >>>>>>    template <> struct B; >>>>>> >>>>>> In (3), it similarly returns the same result for both of >>>>>> >>>>>>    template struct C; >>>>>>    template struct C; >>>>>> >>>>>> even though they are declarations of distinct types. >>>>> >>>>> >>>>> Jason >>>>> >>>> >>> >> > --------------389E6CD266C71115B8CDB6B8 Content-Type: text/x-patch; name="gcc-93824.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-93824.diff" PR c++/93824 - bogus -Wredundant-tags on a first declaration in use PR c++/93810 - missing -Wmismatched-tags and -Wredundant-tags on a typedef of an implicit class template specialization gcc/cp/ChangeLog: PR c++/93824 PR c++/93810 * cp-tree.h (most_specialized_partial_spec): Declare. * parser.c (cp_parser_elaborated_type_specifier): Update a comment. (class_decl_loc_t::class_decl_loc_t): Add an argument. (class_decl_loc_t::add): Same. (class_decl_loc_t::add_or_diag_mismatched_tag): Same. (class_decl_loc_t::is_decl): New member function. (class_decl_loc_t::class_key_loc_t): Add a new member. (specialization_of): New function. (cp_parser_check_class_key): Move code... (class_decl_loc_t::add): ...to here. Add parameters. Avoid issuing -Wredundant-tags on first-time declarations in other declarators. Correct handling of template specializations. (class_decl_loc_t::diag_mismatched_tags): Also expect to be called when -Wredundant-tags is enabled. Use primary template or partial specialization as the guide for uses of implicit instantiations. * pt.c (most_specialized_partial_spec): Declare extern. gcc/testsuite/ChangeLog: PR c++/93824 PR c++/93810 * g++.dg/warn/Wmismatched-tags-3.C: New test. * g++.dg/warn/Wmismatched-tags-4.C: New test. * g++.dg/warn/Wmismatched-tags-5.C: New test. * g++.dg/warn/Wredundant-tags-3.C: Remove xfails. * g++.dg/warn/Wredundant-tags-6.C: New test. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 0a7381cee3f..436e4a27fe3 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6943,6 +6943,7 @@ extern int comp_template_args (tree, tree, tree * = NULL, extern int template_args_equal (tree, tree, bool = false); extern tree maybe_process_partial_specialization (tree); extern tree most_specialized_instantiation (tree); +extern tree most_specialized_partial_spec (tree, tsubst_flags_t); extern void print_candidates (tree); extern void instantiate_pending_templates (int); extern tree tsubst_default_argument (tree, int, tree, tree, diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 24f71671469..eae1c1503c4 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -18937,7 +18937,10 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, cp_parser_maybe_warn_enum_key (parser, key_loc, type, scoped_key); else { - /* Diagnose class/struct/union mismatches. */ + /* Diagnose class/struct/union mismatches. + FIXME: Relying on cp_parser_declares_only_class_p to diffeerentiate + declarations of a class from its uses doesn't work for type aliases + (as in using T = class C;). */ cp_parser_check_class_key (parser, key_loc, tag_type, type, false, cp_parser_declares_only_class_p (parser)); @@ -30885,12 +30888,12 @@ class class_decl_loc_t 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) + bool decl_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); + key_redundant, def_p || decl_p); locvec.quick_push (ckl); } @@ -30924,12 +30927,13 @@ class class_decl_loc_t /* 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); + /* Adds TYPE_DECL to the collection of class decls and diagnoses + redundant tags (if -Wredundant-tags is enabled). */ + static void add (cp_parser *, location_t, tag_types, tree, 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); + void add_or_diag_mismatched_tag (tree, tag_types, bool, bool, bool); private: @@ -30948,6 +30952,11 @@ private: return locvec[i].key_redundant; } + bool is_decl (unsigned i) const + { + return locvec[i].is_decl; + } + tag_types class_key (unsigned i) const { return locvec[i].class_key; @@ -30957,8 +30966,10 @@ private: 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) { } + class_key_loc_t (tree func, location_t loc, tag_types key, bool redundant, + bool decl) + : func (func), loc (loc), class_key (key), key_redundant (redundant), + is_decl (decl) { } /* The function the type is mentioned in. */ tree func; @@ -30968,7 +30979,11 @@ private: 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; + bool key_redundant: 1; + /* True for an entry that corresponds to a dedicated declaration + (including a definition) of a class, false for its other uses + (such as in declarations of other entities). */ + bool is_decl: 1; }; /* Avoid using auto_vec here since it's not safe to copy due to pr90904. */ vec locvec; @@ -31021,6 +31036,64 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc, && class_key != union_type) return; + class_decl_loc_t::add (parser, key_loc, class_key, type, def_p, decl_p); +} + +/* Returns the template or specialization of one to which the RECORD_TYPE + DECL corresponsds. IS_USE should be set when DECL refers to a class + that's being used as opposed to being declared. In that case, if + the specialization is the same as itself, returns the most specialized + partial specialization of the template to which it corresponds if one + exists, or the primary template otherwise. */ + +static tree +specialization_of (tree decl, bool is_use = false) +{ + gcc_assert (TREE_CODE (decl) == TYPE_DECL); + tree decl_type = TREE_TYPE (decl); + + tree ret = decl_type; + + for (tree t = decl_type; + t != NULL_TREE; + t = CLASSTYPE_USE_TEMPLATE (t) + ? TREE_TYPE (CLASSTYPE_TI_TEMPLATE (t)) : NULL_TREE + ) + { + if (same_type_ignoring_top_level_qualifiers_p (t, decl_type)) + { + ret = t; + break; + } + } + + if (ret == decl_type && is_use) + { /* RET is the first specialization of the type; since its also + a USE of it, it's the first implicit instantiation of it. + Determine the template or its partial specialization to which + it corresponds. */ + if (tree spec = most_specialized_partial_spec (ret, tsubst_flags_t ())) + if (spec != error_mark_node) + ret = TREE_VALUE (spec); + + if (ret == decl_type) + ret = CLASSTYPE_PRIMARY_TEMPLATE_TYPE (decl_type); + } + + return ret; +} + + +/* Adds the class TYPE to the collection of class decls and diagnoses + redundant tags (if -Wredundant-tags is enabled). + DEF_P is expected to be set for a definition of class TYPE. DECL_P + is set for a (likely, based on syntactic context) declaration of class + TYPE and clear for a reference to it that is not a declaration of it. */ + +void +class_decl_loc_t::add (cp_parser *parser, location_t key_loc, + tag_types class_key, tree type, bool def_p, bool decl_p) +{ 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 @@ -31032,7 +31105,10 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc, /* 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; + bool key_redundant = (!def_p && !decl_p + && (decl == type_decl + || TREE_CODE (decl) == TEMPLATE_DECL + || TYPE_BEING_DEFINED (type))); if (key_redundant && class_key != class_type @@ -31050,29 +31126,19 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc, key_redundant = false; } - if (key_redundant) + if (!decl_p && !def_p && TREE_CODE (decl) == TEMPLATE_DECL) { - 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); + /* When TYPE is the use of an implicit specialization of a previously + declared template set TYPE_DECL to the type of the primary template + for the specialization and look it up in CLASS2LOC below. For uses + of explicit or partial specializations TYPE_DECL already points to + the declaration of the specialization. */ + type_decl = specialization_of (type_decl); + type_decl = TYPE_MAIN_DECL (type_decl); } - 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) -{ + /* Set if a declaration of TYPE has previously been seen or if it + must exist in a precompiled header. */ bool exist; class_decl_loc_t *rdl = &class2loc.get_or_insert (type_decl, &exist); if (!exist) @@ -31082,30 +31148,52 @@ class_decl_loc_t::add (tree type_decl, tag_types class_key, bool redundant, { /* 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; + a new entry for it and return unless it's a declaration + involving a template that may need to be diagnosed by + -Wredundant-tags. */ + *rdl = class_decl_loc_t (class_key, false, def_p, decl_p); + if (TREE_CODE (decl) != TEMPLATE_DECL) + return; + } + else + { + /* TYPE was previously defined in some unknown precompiled header. + 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); + exist = true; } - - /* 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 (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 (!exist) + /* Do nothing if this is the first declaration of the type. */ + return; + 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); + rdl->add_or_diag_mismatched_tag (type_decl, class_key, key_redundant, + def_p, decl_p); } /* Either adds this DECL corresponding to the TYPE_DECL to the collection @@ -31115,7 +31203,7 @@ void class_decl_loc_t::add_or_diag_mismatched_tag (tree type_decl, tag_types class_key, bool redundant, - bool def_p) + bool def_p, bool decl_p) { /* Reset the CLASS_KEY associated with this type on mismatch. This is an optimization that lets the diagnostic code skip @@ -31130,7 +31218,7 @@ class_decl_loc_t::add_or_diag_mismatched_tag (tree type_decl, /* Append a record of this declaration to the vector. */ class_key_loc_t ckl (current_function_decl, input_location, class_key, - redundant); + redundant, def_p || decl_p); locvec.safe_push (ckl); if (idxdef == UINT_MAX) @@ -31158,35 +31246,76 @@ class_decl_loc_t::add_or_diag_mismatched_tag (tree type_decl, 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) + if (!warn_mismatched_tags) 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. */ + /* Number of usesn of the class. */ + unsigned ndecls = locvec.length (); + + /* Set if a definition for the class has been seen. */ bool def_p = idxdef < ndecls; - unsigned idxguide = def_p ? idxdef : 0; + + /* The class (or template) declaration guiding the decisions about + the diagnostic. For ordinary classes it's the same as THIS. For + uses of instantiations of templates other than their declarations + it points to the record for the declaration of the corresponding + primary template or partial specialization. */ + class_decl_loc_t *cdlguide = this; + + tree type = TREE_TYPE (type_decl); + if (CLASSTYPE_USE_TEMPLATE (type) == 1 && !is_decl (0)) + { + /* For implicit instantiations of a primary template look up + up the primary or partial specialization and use it as + the expected class-key rather than using the class-key of + the first reference to the instantiation. The primary must + be (and inevitably is) at index zero. */ + tree pt = specialization_of (TYPE_MAIN_DECL (type), true); + if (TREE_CODE (pt) == TEMPLATE_DECL) + pt = TREE_TYPE (pt); + pt = TYPE_MAIN_DECL (pt); + cdlguide = class2loc.get (pt); + gcc_assert (cdlguide != NULL); + } + else + { + /* 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; + } + + /* The index of the declaration whose class-key this declaration + is expected to match. It's either the class-key of the class + definintion if one exists or the first declaration otherwise. */ + const 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)) + while (class_key (idx) == cdlguide->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"; + if no definition has been provided. + For implicit instantiations of a primary template it's + the class-key used to declare the primary with. The primary + must be at index zero. */ + const tag_types xpect_key + = cdlguide->class_key (cdlguide == this ? idxguide : 0); + if (cdlguide != this && xpect_key == class_key (idx)) + return; + + /* Save the current function before changing it below. */ + tree save_func = current_function_decl; /* Set the function declaration to print in diagnostic context. */ current_function_decl = function (idx); + const char *xmatchkstr = xpect_key == record_type ? "class" : "struct"; + const char *xpectkstr = xpect_key == record_type ? "struct" : "class"; + location_t loc = location (idx); bool key_redundant_p = key_redundant (idx); auto_diagnostic_group d; @@ -31207,7 +31336,7 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) /* Also point to the first declaration or definition that guided the decision to issue the warning above. */ - inform (location (idxguide), + inform (cdlguide->location (idxguide), (def_p ? G_("%qT defined as %qs here") : G_("%qT first declared as %qs here")), @@ -31249,20 +31378,26 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) 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 ()); + /* CLASS2LOC should be empty if both -Wmismatched-tags and + -Wredundant-tags are disabled. */ + gcc_assert (warn_mismatched_tags + || warn_redundant_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) + if (warn_mismatched_tags) { - tree type_decl = (*it).first; - class_decl_loc_t &recloc = (*it).second; - recloc.diag_mismatched_tags (type_decl); + /* Iterate over the collected class/struct/template 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 (); diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index cb237ba0d9d..ef232ca4fb1 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -185,7 +185,7 @@ static int unify_pack_expansion (tree, tree, tree, tree, unification_kind_t, bool, bool); static tree copy_template_args (tree); static tree tsubst_template_parms (tree, tree, tsubst_flags_t); -static tree most_specialized_partial_spec (tree, tsubst_flags_t); +tree most_specialized_partial_spec (tree, tsubst_flags_t); static tree tsubst_aggr_type (tree, tree, tsubst_flags_t, tree, int); static tree tsubst_arg_types (tree, tree, tree, tsubst_flags_t, tree); static tree tsubst_function_type (tree, tree, tsubst_flags_t, tree); @@ -24317,7 +24317,7 @@ most_general_template (tree decl) partial specializations matching TARGET, then NULL_TREE is returned, indicating that the primary template should be used. */ -static tree +tree most_specialized_partial_spec (tree target, tsubst_flags_t complain) { tree list = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-3.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-3.C new file mode 100644 index 00000000000..ecbe66d037e --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-3.C @@ -0,0 +1,14 @@ +/* { dg-do compile } + { dg-options "-Wall -Wmismatched-tags" } */ + +extern class C1 c1; // { dg-message "declared as 'class'" } +extern struct C1 c1; // { dg-warning "\\\[-Wmismatched-tags" } + +void fs1 (struct S1); // { dg-message "declared as 'struct'" } +void fs1 (class S1); // { dg-warning "\\\[-Wmismatched-tags" } + +enum +{ + ec2 = sizeof (struct C2*), // { dg-message "declared as 'struct'" } + fc2 = sizeof (class C2*) // { dg-warning "\\\[-Wmismatched-tags" } +}; diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-4.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-4.C new file mode 100644 index 00000000000..7570264b1d3 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-4.C @@ -0,0 +1,141 @@ +/* PR c++/94078 - bogus and missing -Wmismatched-tags on an instance + of a template + Verify that -Wmismatched-tags is issued for redeclarations and + instances of the appropriate primary template or specialization. + { dg-do compile } + { dg-options "-Wmismatched-tags" } */ + +// Exercise explicit specialization. +template class S1; +template <> struct S1; + +template class S1; +template struct S1; // { dg-warning "\\\[-Wmismatched-tags" } + +template <> class S1; +template <> struct S1; // { dg-warning "\\\[-Wmismatched-tags" } + +template <> class S1; // { dg-warning "\\\[-Wmismatched-tags" } +template <> struct S1; + +extern S1 s1v; +extern class S1 s1v; +extern struct S1 s1v; // { dg-warning "\\\[-Wmismatched-tags" } + +extern S1 s1i; +extern class S1 s1i; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S1 s1i; + +extern S1 s1c; +extern class S1 s1c; +extern struct S1 s1c; // { dg-warning "\\\[-Wmismatched-tags" } + + +// Exercise partial specialization. +template struct S2; +template class S2; + +template class S2; // { dg-warning "\\\[-Wmismatched-tags" } +template struct S2; + +template class S2; +template struct S2;// { dg-warning "\\\[-Wmismatched-tags" } + +extern S2 s2i; +extern class S2 s2i; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S2 s2i; + +extern S2 s2ci; +extern class S2 s2ci; +extern struct S2 s2ci; // { dg-warning "\\\[-Wmismatched-tags" } + + +template struct S3; +template class S3; +template struct S3; + +template class S3; // { dg-warning "\\\[-Wmismatched-tags" } +template struct S3; + +template class S3; +template struct S3; // { dg-warning "\\\[-Wmismatched-tags" } + +template class S3; // { dg-warning "\\\[-Wmismatched-tags" } +template struct S3; + +extern S3 s3i; +extern class S3 s3i; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S3 s3i; + +extern S3 s3p; +extern class S3 s3p; +extern struct S3 s3p; // { dg-warning "\\\[-Wmismatched-tags" } + +extern S3 s3r; +extern class S3 s3r; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S3 s3r; + +// Repeat exactly the same as above. +extern S3 s3i; +extern class S3 s3i; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S3 s3i; + +extern S3 s3p; +extern class S3 s3p; +extern struct S3 s3p; // { dg-warning "\\\[-Wmismatched-tags" } + +extern S3 s3r; +extern class S3 s3r; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S3 s3r; + +// Repeat the same as above just with different type. +extern S3 s3l; +extern class S3 s3l; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S3 s3l; + +extern S3 s3lp; +extern class S3 s3lp; +extern struct S3 s3lp; // { dg-warning "\\\[-Wmismatched-tags" } + +extern S3 s3lr; +extern class S3 s3lr; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct S3 s3lr; + +// Repeat with the class-keys swapped. +extern S3 s3l; +extern struct S3 s3l; +extern class S3 s3l; // { dg-warning "\\\[-Wmismatched-tags" } + +extern S3 s3lp; +extern struct S3 s3lp; // { dg-warning "\\\[-Wmismatched-tags" } +extern class S3 s3lp; + +extern S3 s3lr; +extern struct S3 s3lr; +extern class S3 s3lr; // { dg-warning "\\\[-Wmismatched-tags" } + + +namespace N +{ +template struct A; + +extern class A ai; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct A ai; + +typedef class A AI; // { dg-warning "\\\[-Wmismatched-tags" } +typedef struct A AI; + +template struct B; +template <> class B; +template <> struct B; + +extern class B bi; +extern struct B bi; // { dg-warning "\\\[-Wmismatched-tags" } + +extern class B bc; // { dg-warning "\\\[-Wmismatched-tags" } +extern struct B bc; + +typedef class B BC; // { dg-warning "\\\[-Wmismatched-tags" } +typedef struct B BC; + +} diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-5.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-5.C new file mode 100644 index 00000000000..7ab2fbbf55a --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-5.C @@ -0,0 +1,78 @@ +/* PR c++/93810 - missing -Wmismatched-tags and -Wredundant-tags on a typedef + of an implicit class template specialization + { dg-do compile } + { dg-options "-Wall -Wmismatched-tags" } + { dg-require-effective-target c++11 } */ + +class A; // { dg-message "declared as 'class'" } +typedef A A0; +typedef class A A0; +typedef struct A A0; // { dg-warning "-Wmismatched-tags" } + +template struct B; // { dg-message "declared as 'struct'" } +typedef B<0> B0; +typedef class B<0> B0; // { dg-warning "-Wmismatched-tags" } +typedef struct B<0> B0; + +template struct C; // { dg-message "declared as 'struct'" } + +template +struct X_CNp1 { + typedef C CNp1; +}; + +template +struct X_class_CNp1 { + typedef class C CNp1; // { dg-warning "-Wmismatched-tags" } +}; + +template +struct X_struct_CNp1 { + typedef struct C CNp1; +}; + +template class D; +template struct D; +template class D; +template struct D; +template class D; +template struct D; +template class D; +template struct D; +template class D; + +typedef class D DIP; // { dg-warning "-Wmismatched-tags" } +typedef struct D DIP; +typedef class D DIP; // { dg-warning "-Wmismatched-tags" } +typedef struct D DIP; + +typedef class D DIR; +typedef struct D DIR; // { dg-warning "-Wmismatched-tags" } +typedef class D DIR; + + +typedef struct D DCIP; +typedef class D DCIP; // { dg-warning "-Wmismatched-tags" } +typedef struct D DCIP; + +typedef struct D DCIR; // { dg-warning "-Wmismatched-tags" } +typedef class D DCIR; +typedef struct D DCIR; // { dg-warning "-Wmismatched-tags" } + + +typedef struct D DVIP; +typedef class D DVIP; // { dg-warning "-Wmismatched-tags" } +typedef struct D DVIP; + +typedef struct D DVIR; // { dg-warning "-Wmismatched-tags" } +typedef class D DVIR; +typedef struct D DVIR; // { dg-warning "-Wmismatched-tags" } + + +typedef struct D DCVIP; +typedef class D DCVIP; // { dg-warning "-Wmismatched-tags" } +typedef struct D DCVIP; + +typedef struct D DCVIR; // { dg-warning "-Wmismatched-tags" } +typedef class D DCVIR; +typedef struct D DCVIR; // { dg-warning "-Wmismatched-tags" } diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C index 7b30e949d0c..0eeee34dae7 100644 --- a/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C +++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-3.C @@ -34,12 +34,12 @@ union N::U u3; // { dg-warning "-Wredundant-tags" } typedef N::TC<0> TC0; typedef typename N::TC<0> TC0; -typedef class N::TC<0> TC0; // { dg-warning "-Wredundant-tags" "pr93809" { xfail *-*-*} .-1 } +typedef class N::TC<0> TC0; // { dg-warning "-Wredundant-tags" } typedef N::TS<0> TS0; typedef typename N::TS<0> TS0; -typedef struct N::TS<0> TS0; // { dg-warning "-Wredundant-tags" "pr93809" { xfail *-*-*} .-1 } +typedef struct N::TS<0> TS0; // { dg-warning "-Wredundant-tags" } typedef N::TS<0> TS0; typedef typename N::TS<0> TS0; -typedef struct N::TS<0> TS0; // { dg-warning "-Wredundant-tags" "pr93809" { xfail *-*-*} .-1 } +typedef struct N::TS<0> TS0; // { dg-warning "-Wredundant-tags" } diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-6.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags-6.C new file mode 100644 index 00000000000..ce5cb967b79 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-6.C @@ -0,0 +1,91 @@ +/* PR c++/93824 - bogus -Wredundant-tags on a first declaration in use + { dg-do compile } + { dg-options "-Wredundant-tags" } */ + +extern class C1 &c1; // { dg-bogus "\\\[-Wredundant-tags" } +extern class C1 &c1; // { dg-warning "\\\[-Wredundant-tags" } + +void fc2 (class C2); // { dg-bogus "\\\[-Wredundant-tags" } +void fc2 (class C2); // { dg-warning "\\\[-Wredundant-tags" } + +const int +npc3 = sizeof (class C3*); // { dg-bogus "\\\[-Wredundant-tags" } +const int +nppc3 = sizeof (class C3**); // { dg-warning "\\\[-Wredundant-tags" } + +extern struct S1 *s1p; // { dg-bogus "\\\[-Wredundant-tags" } +extern struct S1 s1a[]; // { dg-warning "\\\[-Wredundant-tags" } + +struct S3 +{ + struct S3 *p1s3; // { dg-warning "\\\[-Wredundant-tags" } + S3 *p2s3; + + union U1 *p1u1; // { dg-bogus "\\\[-Wredundant-tags" } + union U1 *p2u1; // { dg-warning "\\\[-Wredundant-tags" } +} s3; + +typedef struct S3 T_S3; // { dg-warning "\\\[-Wredundant-tags" } + +typedef struct S4 T_S4; + +struct S5 +{ + struct S6 + { + private: + // 'struct' is redundant in a declaration of a pointer to ::S5; + struct S5 *ps5; // { dg-warning "\\\[-Wredundant-tags" } + // 'struct' is required in a definition of a new type. + struct S5 { } *ps5_2; + struct S5 *p2s5_2; // { dg-warning "\\\[-Wredundant-tags" } + }; +}; + + +template struct TS1; + +// Verify redeclaration with no definition. +template <> struct TS1<0>; +template <> struct TS1<0>; + +// Verify definition after a declaration and vice versa. +template <> struct TS1<1>; +template <> struct TS1<1> { }; +template <> struct TS1<1>; + +// Verify typedefs of an explicit specialization with a definition. +typedef struct TS1<1> TS1_1; // { dg-warning "\\\[-Wredundant-tags" } +typedef TS1<1> TS1_1; +typedef struct TS1<1> TS1_1; // { dg-warning "\\\[-Wredundant-tags" } + +// Verify object declarations of an expplicit specialization. +extern struct TS1<1> ts1_1; // { dg-warning "\\\[-Wredundant-tags" } +extern TS1<1> ts1_1; +extern struct TS1<1> ts1_1; // { dg-warning "\\\[-Wredundant-tags" } + +// Verify typedefs of an implicit specialization without a definition. +typedef struct TS1<2> TS1_2; // { dg-warning "\\\[-Wredundant-tags" } +typedef TS1<2> TS1_2; +typedef struct TS1<2> TS1_2; // { dg-warning "\\\[-Wredundant-tags" } + +// Verify object declarations of an implicit specialization. +extern struct TS1<2> ts1_2; // { dg-warning "\\\[-Wredundant-tags" } +extern TS1<2> ts1_2; +extern struct TS1<2> ts1_2; // { dg-warning "\\\[-Wredundant-tags" } + + +// Verify partial template specialization. +template struct TS2; +template struct TS2; +template struct TS2; + +template +struct TS4 +{ + typedef struct TS2 TS2_CT1; // { dg-warning "\\\[-Wredundant-tags" } + typedef TS2 TS2_CT2; + + typedef struct TS2 TS2_T1; // { dg-warning "\\\[-Wredundant-tags" } + typedef TS2 TS2_T2; +}; --------------389E6CD266C71115B8CDB6B8--