* [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] @ 2023-03-01 20:33 Marek Polacek 2023-03-01 21:30 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Marek Polacek @ 2023-03-01 20:33 UTC (permalink / raw) To: Jason Merrill, GCC Patches -Wmismatched-tags warns about the (harmless) struct/class mismatch. For, e.g., template<typename T> struct A { }; class A<int> a; it works by adding A<T> to the class2loc hash table while parsing the class-head and then, while parsing the elaborate type-specifier, we add A<int>. At the end of c_parse_file we go through the table and warn about the class-key mismatches. In this PR we crash though; we have template<typename T> struct A { template<typename U> struct W { }; }; struct A<int>::W<int> w; // #1 where while parsing A and #1 we've stashed A<T> A<T>::W<U> A<int>::W<int> into class2loc. Then in class_decl_loc_t::diag_mismatched_tags TYPE is A<int>::W<int>, and specialization_of gets us A<int>::W<U>, which is not in class2loc, so we crash on gcc_assert (cdlguide). But it's OK not to have found A<int>::W<U>, we should just look one "level" up, that is, A<T>::W<U>. It's important to handle class specializations, so e.g. template<> struct A<char> { template<typename U> class W { }; }; where W's class-key is different than in the primary template above, so we should warn depending on whether we're looking into A<char> or into a different instantiation. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/106259 gcc/cp/ChangeLog: * parser.cc (class_decl_loc_t::diag_mismatched_tags): If the first lookup of SPEC didn't find anything, try to look for most_general_template. gcc/testsuite/ChangeLog: * g++.dg/warn/Wmismatched-tags-11.C: New test. --- gcc/cp/parser.cc | 30 +++++++++++++++---- .../g++.dg/warn/Wmismatched-tags-11.C | 23 ++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 1a124f5395e..b528ee7b1d9 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -34473,14 +34473,32 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) be (and inevitably is) at index zero. */ tree spec = specialization_of (type); cdlguide = class2loc.get (spec); + /* It's possible that we didn't find SPEC. Consider: + + template<typename T> struct A { + template<typename U> struct W { }; + }; + struct A<int>::W<int> w; // #1 + + where while parsing A and #1 we've stashed + A<T> + A<T>::W<U> + A<int>::W<int> + into CLASS2LOC. If TYPE is A<int>::W<int>, specialization_of + will yield A<int>::W<U> which may be in CLASS2LOC if we had + an A<int> class specialization, but otherwise won't be in it. + So try to look up A<T>::W<U>. */ + if (!cdlguide) + { + spec = DECL_TEMPLATE_RESULT (most_general_template (spec)); + cdlguide = class2loc.get (spec); + } + /* Now we really should have found something. */ gcc_assert (cdlguide != NULL); } - else - { - /* Skip declarations that consistently use the same class-key. */ - if (def_class_key != none_type) - return; - } + /* Skip declarations that consistently use the same class-key. */ + else if (def_class_key != none_type) + return; /* Set if a definition for the class has been seen. */ const bool def_p = cdlguide->def_p (); diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C new file mode 100644 index 00000000000..6c4e571726a --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C @@ -0,0 +1,23 @@ +// PR c++/106259 +// { dg-do compile } +// { dg-options "-Wmismatched-tags" } + +template<typename T> struct A { + template<typename U> + struct W { }; +}; + +template<> +struct A<char> { + template<typename U> + class W { }; +}; + +void +g () +{ + struct A<char>::W<int> w1; // { dg-warning "mismatched" } + struct A<int>::W<int> w2; + class A<char>::W<int> w3; + class A<int>::W<int> w4; // { dg-warning "mismatched" } +} base-commit: 096f034a8f5df41f610e62c1592fb90a3f551cd5 -- 2.39.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] 2023-03-01 20:33 [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] Marek Polacek @ 2023-03-01 21:30 ` Jason Merrill 2023-03-01 21:40 ` Marek Polacek 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2023-03-01 21:30 UTC (permalink / raw) To: Marek Polacek, GCC Patches On 3/1/23 15:33, Marek Polacek wrote: > -Wmismatched-tags warns about the (harmless) struct/class mismatch. > For, e.g., > > template<typename T> struct A { }; > class A<int> a; > > it works by adding A<T> to the class2loc hash table while parsing the > class-head and then, while parsing the elaborate type-specifier, we > add A<int>. At the end of c_parse_file we go through the table and > warn about the class-key mismatches. In this PR we crash though; we > have > > template<typename T> struct A { > template<typename U> struct W { }; > }; > struct A<int>::W<int> w; // #1 > > where while parsing A and #1 we've stashed > A<T> > A<T>::W<U> > A<int>::W<int> > into class2loc. Then in class_decl_loc_t::diag_mismatched_tags TYPE > is A<int>::W<int>, and specialization_of gets us A<int>::W<U>, which > is not in class2loc, so we crash on gcc_assert (cdlguide). But it's > OK not to have found A<int>::W<U>, we should just look one "level" up, > that is, A<T>::W<U>. > > It's important to handle class specializations, so e.g. > > template<> > struct A<char> { > template<typename U> > class W { }; > }; > > where W's class-key is different than in the primary template above, > so we should warn depending on whether we're looking into A<char> > or into a different instantiation. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR c++/106259 > > gcc/cp/ChangeLog: > > * parser.cc (class_decl_loc_t::diag_mismatched_tags): If the first > lookup of SPEC didn't find anything, try to look for > most_general_template. > > gcc/testsuite/ChangeLog: > > * g++.dg/warn/Wmismatched-tags-11.C: New test. > --- > gcc/cp/parser.cc | 30 +++++++++++++++---- > .../g++.dg/warn/Wmismatched-tags-11.C | 23 ++++++++++++++ > 2 files changed, 47 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 1a124f5395e..b528ee7b1d9 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -34473,14 +34473,32 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) > be (and inevitably is) at index zero. */ > tree spec = specialization_of (type); > cdlguide = class2loc.get (spec); > + /* It's possible that we didn't find SPEC. Consider: > + > + template<typename T> struct A { > + template<typename U> struct W { }; > + }; > + struct A<int>::W<int> w; // #1 > + > + where while parsing A and #1 we've stashed > + A<T> > + A<T>::W<U> > + A<int>::W<int> > + into CLASS2LOC. If TYPE is A<int>::W<int>, specialization_of > + will yield A<int>::W<U> which may be in CLASS2LOC if we had > + an A<int> class specialization, but otherwise won't be in it. > + So try to look up A<T>::W<U>. */ > + if (!cdlguide) > + { > + spec = DECL_TEMPLATE_RESULT (most_general_template (spec)); Would it make sense to only look at most_general_template, not A<int>::W<U> at all? > + cdlguide = class2loc.get (spec); > + } > + /* Now we really should have found something. */ > gcc_assert (cdlguide != NULL); > } > - else > - { > - /* Skip declarations that consistently use the same class-key. */ > - if (def_class_key != none_type) > - return; > - } > + /* Skip declarations that consistently use the same class-key. */ > + else if (def_class_key != none_type) > + return; > > /* Set if a definition for the class has been seen. */ > const bool def_p = cdlguide->def_p (); > diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C > new file mode 100644 > index 00000000000..6c4e571726a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C > @@ -0,0 +1,23 @@ > +// PR c++/106259 > +// { dg-do compile } > +// { dg-options "-Wmismatched-tags" } > + > +template<typename T> struct A { > + template<typename U> > + struct W { }; > +}; > + > +template<> > +struct A<char> { > + template<typename U> > + class W { }; > +}; > + > +void > +g () > +{ > + struct A<char>::W<int> w1; // { dg-warning "mismatched" } > + struct A<int>::W<int> w2; > + class A<char>::W<int> w3; > + class A<int>::W<int> w4; // { dg-warning "mismatched" } > +} > > base-commit: 096f034a8f5df41f610e62c1592fb90a3f551cd5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] 2023-03-01 21:30 ` Jason Merrill @ 2023-03-01 21:40 ` Marek Polacek 2023-03-01 21:44 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Marek Polacek @ 2023-03-01 21:40 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Mar 01, 2023 at 04:30:16PM -0500, Jason Merrill wrote: > On 3/1/23 15:33, Marek Polacek wrote: > > -Wmismatched-tags warns about the (harmless) struct/class mismatch. > > For, e.g., > > > > template<typename T> struct A { }; > > class A<int> a; > > > > it works by adding A<T> to the class2loc hash table while parsing the > > class-head and then, while parsing the elaborate type-specifier, we > > add A<int>. At the end of c_parse_file we go through the table and > > warn about the class-key mismatches. In this PR we crash though; we > > have > > > > template<typename T> struct A { > > template<typename U> struct W { }; > > }; > > struct A<int>::W<int> w; // #1 > > > > where while parsing A and #1 we've stashed > > A<T> > > A<T>::W<U> > > A<int>::W<int> > > into class2loc. Then in class_decl_loc_t::diag_mismatched_tags TYPE > > is A<int>::W<int>, and specialization_of gets us A<int>::W<U>, which > > is not in class2loc, so we crash on gcc_assert (cdlguide). But it's > > OK not to have found A<int>::W<U>, we should just look one "level" up, > > that is, A<T>::W<U>. > > > > It's important to handle class specializations, so e.g. > > > > template<> > > struct A<char> { > > template<typename U> > > class W { }; > > }; > > > > where W's class-key is different than in the primary template above, > > so we should warn depending on whether we're looking into A<char> > > or into a different instantiation. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > PR c++/106259 > > > > gcc/cp/ChangeLog: > > > > * parser.cc (class_decl_loc_t::diag_mismatched_tags): If the first > > lookup of SPEC didn't find anything, try to look for > > most_general_template. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/warn/Wmismatched-tags-11.C: New test. > > --- > > gcc/cp/parser.cc | 30 +++++++++++++++---- > > .../g++.dg/warn/Wmismatched-tags-11.C | 23 ++++++++++++++ > > 2 files changed, 47 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C > > > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > > index 1a124f5395e..b528ee7b1d9 100644 > > --- a/gcc/cp/parser.cc > > +++ b/gcc/cp/parser.cc > > @@ -34473,14 +34473,32 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) > > be (and inevitably is) at index zero. */ > > tree spec = specialization_of (type); > > cdlguide = class2loc.get (spec); > > + /* It's possible that we didn't find SPEC. Consider: > > + > > + template<typename T> struct A { > > + template<typename U> struct W { }; > > + }; > > + struct A<int>::W<int> w; // #1 > > + > > + where while parsing A and #1 we've stashed > > + A<T> > > + A<T>::W<U> > > + A<int>::W<int> > > + into CLASS2LOC. If TYPE is A<int>::W<int>, specialization_of > > + will yield A<int>::W<U> which may be in CLASS2LOC if we had > > + an A<int> class specialization, but otherwise won't be in it. > > + So try to look up A<T>::W<U>. */ > > + if (!cdlguide) > > + { > > + spec = DECL_TEMPLATE_RESULT (most_general_template (spec)); > > Would it make sense to only look at most_general_template, not A<int>::W<U> > at all? I think that would break with class specialization, as in... > > +template<typename T> struct A { > > + template<typename U> > > + struct W { }; > > +}; > > + > > +template<> > > +struct A<char> { > > + template<typename U> > > + class W { }; > > +}; > > + > > +void > > +g () > > +{ > > + struct A<char>::W<int> w1; // { dg-warning "mismatched" } ...this, where we should first look into A<char>, and only if not found, go to A<T>. class2loc will be filled with A<char>::W<U>, added while parsing the specialization. > > + struct A<int>::W<int> w2; > > + class A<char>::W<int> w3; > > + class A<int>::W<int> w4; // { dg-warning "mismatched" } > > +} > > > > base-commit: 096f034a8f5df41f610e62c1592fb90a3f551cd5 > Marek ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] 2023-03-01 21:40 ` Marek Polacek @ 2023-03-01 21:44 ` Jason Merrill 2023-03-01 22:33 ` Marek Polacek 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2023-03-01 21:44 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 3/1/23 16:40, Marek Polacek wrote: > On Wed, Mar 01, 2023 at 04:30:16PM -0500, Jason Merrill wrote: >> On 3/1/23 15:33, Marek Polacek wrote: >>> -Wmismatched-tags warns about the (harmless) struct/class mismatch. >>> For, e.g., >>> >>> template<typename T> struct A { }; >>> class A<int> a; >>> >>> it works by adding A<T> to the class2loc hash table while parsing the >>> class-head and then, while parsing the elaborate type-specifier, we >>> add A<int>. At the end of c_parse_file we go through the table and >>> warn about the class-key mismatches. In this PR we crash though; we >>> have >>> >>> template<typename T> struct A { >>> template<typename U> struct W { }; >>> }; >>> struct A<int>::W<int> w; // #1 >>> >>> where while parsing A and #1 we've stashed >>> A<T> >>> A<T>::W<U> >>> A<int>::W<int> >>> into class2loc. Then in class_decl_loc_t::diag_mismatched_tags TYPE >>> is A<int>::W<int>, and specialization_of gets us A<int>::W<U>, which >>> is not in class2loc, so we crash on gcc_assert (cdlguide). But it's >>> OK not to have found A<int>::W<U>, we should just look one "level" up, >>> that is, A<T>::W<U>. >>> >>> It's important to handle class specializations, so e.g. >>> >>> template<> >>> struct A<char> { >>> template<typename U> >>> class W { }; >>> }; >>> >>> where W's class-key is different than in the primary template above, >>> so we should warn depending on whether we're looking into A<char> >>> or into a different instantiation. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >>> >>> PR c++/106259 >>> >>> gcc/cp/ChangeLog: >>> >>> * parser.cc (class_decl_loc_t::diag_mismatched_tags): If the first >>> lookup of SPEC didn't find anything, try to look for >>> most_general_template. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/warn/Wmismatched-tags-11.C: New test. >>> --- >>> gcc/cp/parser.cc | 30 +++++++++++++++---- >>> .../g++.dg/warn/Wmismatched-tags-11.C | 23 ++++++++++++++ >>> 2 files changed, 47 insertions(+), 6 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C >>> >>> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc >>> index 1a124f5395e..b528ee7b1d9 100644 >>> --- a/gcc/cp/parser.cc >>> +++ b/gcc/cp/parser.cc >>> @@ -34473,14 +34473,32 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) >>> be (and inevitably is) at index zero. */ >>> tree spec = specialization_of (type); >>> cdlguide = class2loc.get (spec); >>> + /* It's possible that we didn't find SPEC. Consider: >>> + >>> + template<typename T> struct A { >>> + template<typename U> struct W { }; >>> + }; >>> + struct A<int>::W<int> w; // #1 >>> + >>> + where while parsing A and #1 we've stashed >>> + A<T> >>> + A<T>::W<U> >>> + A<int>::W<int> >>> + into CLASS2LOC. If TYPE is A<int>::W<int>, specialization_of >>> + will yield A<int>::W<U> which may be in CLASS2LOC if we had >>> + an A<int> class specialization, but otherwise won't be in it. >>> + So try to look up A<T>::W<U>. */ >>> + if (!cdlguide) >>> + { >>> + spec = DECL_TEMPLATE_RESULT (most_general_template (spec)); >> >> Would it make sense to only look at most_general_template, not A<int>::W<U> >> at all? > > I think that would break with class specialization, as in... > >>> +template<typename T> struct A { >>> + template<typename U> >>> + struct W { }; >>> +}; >>> + >>> +template<> >>> +struct A<char> { >>> + template<typename U> >>> + class W { }; >>> +}; >>> + >>> +void >>> +g () >>> +{ >>> + struct A<char>::W<int> w1; // { dg-warning "mismatched" } > > ...this, where we should first look into A<char>, and only if not > found, go to A<T>. I'd expect the > /* Stop if we run into an explicitly specialized class template. */ code in most_general_template to avoid that problem. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] 2023-03-01 21:44 ` Jason Merrill @ 2023-03-01 22:33 ` Marek Polacek 2023-03-02 15:43 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Marek Polacek @ 2023-03-01 22:33 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Wed, Mar 01, 2023 at 04:44:12PM -0500, Jason Merrill wrote: > On 3/1/23 16:40, Marek Polacek wrote: > > On Wed, Mar 01, 2023 at 04:30:16PM -0500, Jason Merrill wrote: > > > On 3/1/23 15:33, Marek Polacek wrote: > > > > -Wmismatched-tags warns about the (harmless) struct/class mismatch. > > > > For, e.g., > > > > > > > > template<typename T> struct A { }; > > > > class A<int> a; > > > > > > > > it works by adding A<T> to the class2loc hash table while parsing the > > > > class-head and then, while parsing the elaborate type-specifier, we > > > > add A<int>. At the end of c_parse_file we go through the table and > > > > warn about the class-key mismatches. In this PR we crash though; we > > > > have > > > > > > > > template<typename T> struct A { > > > > template<typename U> struct W { }; > > > > }; > > > > struct A<int>::W<int> w; // #1 > > > > > > > > where while parsing A and #1 we've stashed > > > > A<T> > > > > A<T>::W<U> > > > > A<int>::W<int> > > > > into class2loc. Then in class_decl_loc_t::diag_mismatched_tags TYPE > > > > is A<int>::W<int>, and specialization_of gets us A<int>::W<U>, which > > > > is not in class2loc, so we crash on gcc_assert (cdlguide). But it's > > > > OK not to have found A<int>::W<U>, we should just look one "level" up, > > > > that is, A<T>::W<U>. > > > > > > > > It's important to handle class specializations, so e.g. > > > > > > > > template<> > > > > struct A<char> { > > > > template<typename U> > > > > class W { }; > > > > }; > > > > > > > > where W's class-key is different than in the primary template above, > > > > so we should warn depending on whether we're looking into A<char> > > > > or into a different instantiation. > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > > > > > PR c++/106259 > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * parser.cc (class_decl_loc_t::diag_mismatched_tags): If the first > > > > lookup of SPEC didn't find anything, try to look for > > > > most_general_template. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.dg/warn/Wmismatched-tags-11.C: New test. > > > > --- > > > > gcc/cp/parser.cc | 30 +++++++++++++++---- > > > > .../g++.dg/warn/Wmismatched-tags-11.C | 23 ++++++++++++++ > > > > 2 files changed, 47 insertions(+), 6 deletions(-) > > > > create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C > > > > > > > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > > > > index 1a124f5395e..b528ee7b1d9 100644 > > > > --- a/gcc/cp/parser.cc > > > > +++ b/gcc/cp/parser.cc > > > > @@ -34473,14 +34473,32 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) > > > > be (and inevitably is) at index zero. */ > > > > tree spec = specialization_of (type); > > > > cdlguide = class2loc.get (spec); > > > > + /* It's possible that we didn't find SPEC. Consider: > > > > + > > > > + template<typename T> struct A { > > > > + template<typename U> struct W { }; > > > > + }; > > > > + struct A<int>::W<int> w; // #1 > > > > + > > > > + where while parsing A and #1 we've stashed > > > > + A<T> > > > > + A<T>::W<U> > > > > + A<int>::W<int> > > > > + into CLASS2LOC. If TYPE is A<int>::W<int>, specialization_of > > > > + will yield A<int>::W<U> which may be in CLASS2LOC if we had > > > > + an A<int> class specialization, but otherwise won't be in it. > > > > + So try to look up A<T>::W<U>. */ > > > > + if (!cdlguide) > > > > + { > > > > + spec = DECL_TEMPLATE_RESULT (most_general_template (spec)); > > > > > > Would it make sense to only look at most_general_template, not A<int>::W<U> > > > at all? > > > > I think that would break with class specialization, as in... > > > > > > +template<typename T> struct A { > > > > + template<typename U> > > > > + struct W { }; > > > > +}; > > > > + > > > > +template<> > > > > +struct A<char> { > > > > + template<typename U> > > > > + class W { }; > > > > +}; > > > > + > > > > +void > > > > +g () > > > > +{ > > > > + struct A<char>::W<int> w1; // { dg-warning "mismatched" } > > > > ...this, where we should first look into A<char>, and only if not > > found, go to A<T>. > > I'd expect the > > > /* Stop if we run into an explicitly specialized class template. */ > > code in most_general_template to avoid that problem. Ah, I had no idea it does that. The unconditional most_general_template works fine for the new test, but some of the existing tests then fail. Reduced: template <class Z> struct S2; // #1 template <class T> class S2<const T>; // #2 extern class S2<const int> s2ci; // #3 extern struct S2<const int> s2ci; // { dg-warning "\\\[-Wmismatched-tags" } where the unconditional most_general_template changes spec from "class S2<const T>" to "struct S2<Z>" (both of which are in class2loc). So it regresses the diagnostic, complaining that #3 should have "struct" since #1 has "struct". I think we want to keep the current diagnostic, saying that the last line should have "class" since the specialization in line #2 has "class". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] 2023-03-01 22:33 ` Marek Polacek @ 2023-03-02 15:43 ` Jason Merrill 0 siblings, 0 replies; 6+ messages in thread From: Jason Merrill @ 2023-03-02 15:43 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On 3/1/23 17:33, Marek Polacek wrote: > On Wed, Mar 01, 2023 at 04:44:12PM -0500, Jason Merrill wrote: >> On 3/1/23 16:40, Marek Polacek wrote: >>> On Wed, Mar 01, 2023 at 04:30:16PM -0500, Jason Merrill wrote: >>>> On 3/1/23 15:33, Marek Polacek wrote: >>>>> -Wmismatched-tags warns about the (harmless) struct/class mismatch. >>>>> For, e.g., >>>>> >>>>> template<typename T> struct A { }; >>>>> class A<int> a; >>>>> >>>>> it works by adding A<T> to the class2loc hash table while parsing the >>>>> class-head and then, while parsing the elaborate type-specifier, we >>>>> add A<int>. At the end of c_parse_file we go through the table and >>>>> warn about the class-key mismatches. In this PR we crash though; we >>>>> have >>>>> >>>>> template<typename T> struct A { >>>>> template<typename U> struct W { }; >>>>> }; >>>>> struct A<int>::W<int> w; // #1 >>>>> >>>>> where while parsing A and #1 we've stashed >>>>> A<T> >>>>> A<T>::W<U> >>>>> A<int>::W<int> >>>>> into class2loc. Then in class_decl_loc_t::diag_mismatched_tags TYPE >>>>> is A<int>::W<int>, and specialization_of gets us A<int>::W<U>, which >>>>> is not in class2loc, so we crash on gcc_assert (cdlguide). But it's >>>>> OK not to have found A<int>::W<U>, we should just look one "level" up, >>>>> that is, A<T>::W<U>. >>>>> >>>>> It's important to handle class specializations, so e.g. >>>>> >>>>> template<> >>>>> struct A<char> { >>>>> template<typename U> >>>>> class W { }; >>>>> }; >>>>> >>>>> where W's class-key is different than in the primary template above, >>>>> so we should warn depending on whether we're looking into A<char> >>>>> or into a different instantiation. >>>>> >>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >>>>> >>>>> PR c++/106259 >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> * parser.cc (class_decl_loc_t::diag_mismatched_tags): If the first >>>>> lookup of SPEC didn't find anything, try to look for >>>>> most_general_template. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> * g++.dg/warn/Wmismatched-tags-11.C: New test. >>>>> --- >>>>> gcc/cp/parser.cc | 30 +++++++++++++++---- >>>>> .../g++.dg/warn/Wmismatched-tags-11.C | 23 ++++++++++++++ >>>>> 2 files changed, 47 insertions(+), 6 deletions(-) >>>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-tags-11.C >>>>> >>>>> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc >>>>> index 1a124f5395e..b528ee7b1d9 100644 >>>>> --- a/gcc/cp/parser.cc >>>>> +++ b/gcc/cp/parser.cc >>>>> @@ -34473,14 +34473,32 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl) >>>>> be (and inevitably is) at index zero. */ >>>>> tree spec = specialization_of (type); >>>>> cdlguide = class2loc.get (spec); >>>>> + /* It's possible that we didn't find SPEC. Consider: >>>>> + >>>>> + template<typename T> struct A { >>>>> + template<typename U> struct W { }; >>>>> + }; >>>>> + struct A<int>::W<int> w; // #1 >>>>> + >>>>> + where while parsing A and #1 we've stashed >>>>> + A<T> >>>>> + A<T>::W<U> >>>>> + A<int>::W<int> >>>>> + into CLASS2LOC. If TYPE is A<int>::W<int>, specialization_of >>>>> + will yield A<int>::W<U> which may be in CLASS2LOC if we had >>>>> + an A<int> class specialization, but otherwise won't be in it. >>>>> + So try to look up A<T>::W<U>. */ >>>>> + if (!cdlguide) >>>>> + { >>>>> + spec = DECL_TEMPLATE_RESULT (most_general_template (spec)); >>>> >>>> Would it make sense to only look at most_general_template, not A<int>::W<U> >>>> at all? >>> >>> I think that would break with class specialization, as in... >>> >>>>> +template<typename T> struct A { >>>>> + template<typename U> >>>>> + struct W { }; >>>>> +}; >>>>> + >>>>> +template<> >>>>> +struct A<char> { >>>>> + template<typename U> >>>>> + class W { }; >>>>> +}; >>>>> + >>>>> +void >>>>> +g () >>>>> +{ >>>>> + struct A<char>::W<int> w1; // { dg-warning "mismatched" } >>> >>> ...this, where we should first look into A<char>, and only if not >>> found, go to A<T>. >> >> I'd expect the >> >>> /* Stop if we run into an explicitly specialized class template. */ >> >> code in most_general_template to avoid that problem. > > Ah, I had no idea it does that. The unconditional most_general_template > works fine for the new test, but some of the existing tests then fail. > Reduced: > > template <class Z> struct S2; // #1 > template <class T> class S2<const T>; // #2 > > extern class S2<const int> s2ci; // #3 > extern struct S2<const int> s2ci; // { dg-warning "\\\[-Wmismatched-tags" } > > where the unconditional most_general_template changes spec from > "class S2<const T>" to "struct S2<Z>" (both of which are in class2loc). > So it regresses the diagnostic, complaining that #3 should have "struct" > since #1 has "struct". I think we want to keep the current diagnostic, > saying that the last line should have "class" since the specialization > in line #2 has "class". Makes sense, the patch is OK. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-02 15:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-01 20:33 [PATCH] c++: ICE with -Wmismatched-tags and member template [PR106259] Marek Polacek 2023-03-01 21:30 ` Jason Merrill 2023-03-01 21:40 ` Marek Polacek 2023-03-01 21:44 ` Jason Merrill 2023-03-01 22:33 ` Marek Polacek 2023-03-02 15:43 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).