| Message ID | 20200615194044.GB4137376@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Fix bogus "does not declare anything" warning (PR 66159) | expand |
On 6/15/20 3:40 PM, Jonathan Wakely wrote: > Ping ... > > > > G++ gives a bogus warning for 'struct A; using B = struct ::A;' > complaining that the elaborated-type-specifier doesn't declare anything. > That's true, but it's not trying to declare struct ::A, just refer to it > unambiguously. Do not emit the warning unless we're actually parsing a > declaration. > > This also makes the relevant warning depend on -Wredundant-decls (which > is not part of -Wall or -Wextra) so it can be disabled on the command > line or by using #pragma. This means the warning will no longer be given > by default, so some tests need -Wredundant-decls added. > > gcc/cp/ChangeLog: > > PR c++/66159 > * parser.c (cp_parser_elaborated_type_specifier): Do not warn > unless in a declaration. Make warning depend on > WOPT_redundant_decls. > > gcc/testsuite/ChangeLog: > > PR c++/66159 > * g++.dg/parse/specialization1.C: Remove dg-warning. > * g++.dg/warn/forward-inner.C: Add -Wredundant-decls. Check > alias-declaration using elaborated-type-specifier. > * g++.dg/warn/pr36999.C: Add -Wredundant-decls. > > > Is it OK to make this warning no longer emitted by default, and not > even with -Wall -Wextra? I'd like to keep it on by default. > Would it be better to add a new option for this specific warning, > which would be enabled by -Wall and also by -Wredundant-decls? Maybe > -Wredundant-decls-elaborated-type or something. That sounds fine. But do you think people will really want to turn this off after you fix the bug? Jason
On 17/06/20 12:32 -0400, Jason Merrill wrote: >On 6/15/20 3:40 PM, Jonathan Wakely wrote: >>Ping ... >> >> >> >>G++ gives a bogus warning for 'struct A; using B = struct ::A;' >>complaining that the elaborated-type-specifier doesn't declare anything. >>That's true, but it's not trying to declare struct ::A, just refer to it >>unambiguously. Do not emit the warning unless we're actually parsing a >>declaration. >> >>This also makes the relevant warning depend on -Wredundant-decls (which >>is not part of -Wall or -Wextra) so it can be disabled on the command >>line or by using #pragma. This means the warning will no longer be given >>by default, so some tests need -Wredundant-decls added. >> >>gcc/cp/ChangeLog: >> >> PR c++/66159 >> * parser.c (cp_parser_elaborated_type_specifier): Do not warn >> unless in a declaration. Make warning depend on >> WOPT_redundant_decls. >> >>gcc/testsuite/ChangeLog: >> >> PR c++/66159 >> * g++.dg/parse/specialization1.C: Remove dg-warning. >> * g++.dg/warn/forward-inner.C: Add -Wredundant-decls. Check >> alias-declaration using elaborated-type-specifier. >> * g++.dg/warn/pr36999.C: Add -Wredundant-decls. >> >> >>Is it OK to make this warning no longer emitted by default, and not >>even with -Wall -Wextra? > >I'd like to keep it on by default. > >>Would it be better to add a new option for this specific warning, >>which would be enabled by -Wall and also by -Wredundant-decls? Maybe >>-Wredundant-decls-elaborated-type or something. > >That sounds fine. But do you think people will really want to turn >this off after you fix the bug? I don't know how to fix the bug, so I'll have to leave that to somebody else. Looks like tweaking the warning without fixing the actual bug isn't worth doing.
On 6/17/20 12:49 PM, Jonathan Wakely wrote: > On 17/06/20 12:32 -0400, Jason Merrill wrote: >> On 6/15/20 3:40 PM, Jonathan Wakely wrote: >>> Ping ... >>> >>> >>> >>> G++ gives a bogus warning for 'struct A; using B = struct ::A;' >>> complaining that the elaborated-type-specifier doesn't declare anything. >>> That's true, but it's not trying to declare struct ::A, just refer to it >>> unambiguously. Do not emit the warning unless we're actually parsing a >>> declaration. >>> >>> This also makes the relevant warning depend on -Wredundant-decls (which >>> is not part of -Wall or -Wextra) so it can be disabled on the command >>> line or by using #pragma. This means the warning will no longer be given >>> by default, so some tests need -Wredundant-decls added. >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/66159 >>> * parser.c (cp_parser_elaborated_type_specifier): Do not warn >>> unless in a declaration. Make warning depend on >>> WOPT_redundant_decls. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/66159 >>> * g++.dg/parse/specialization1.C: Remove dg-warning. >>> * g++.dg/warn/forward-inner.C: Add -Wredundant-decls. Check >>> alias-declaration using elaborated-type-specifier. >>> * g++.dg/warn/pr36999.C: Add -Wredundant-decls. >>> >>> >>> Is it OK to make this warning no longer emitted by default, and not >>> even with -Wall -Wextra? >> >> I'd like to keep it on by default. >> >>> Would it be better to add a new option for this specific warning, >>> which would be enabled by -Wall and also by -Wredundant-decls? Maybe >>> -Wredundant-decls-elaborated-type or something. >> >> That sounds fine. But do you think people will really want to turn >> this off after you fix the bug? > > I don't know how to fix the bug, so I'll have to leave that to > somebody else. By fixing the bug, I mean your "Do not emit the warning unless we're actually parsing a declaration." Please do apply your change to the condition for the warning. Jason
On 17/06/20 12:56 -0400, Jason Merrill wrote: >On 6/17/20 12:49 PM, Jonathan Wakely wrote: >>On 17/06/20 12:32 -0400, Jason Merrill wrote: >>>On 6/15/20 3:40 PM, Jonathan Wakely wrote: >>>>Ping ... >>>> >>>> >>>> >>>>G++ gives a bogus warning for 'struct A; using B = struct ::A;' >>>>complaining that the elaborated-type-specifier doesn't declare anything. >>>>That's true, but it's not trying to declare struct ::A, just refer to it >>>>unambiguously. Do not emit the warning unless we're actually parsing a >>>>declaration. >>>> >>>>This also makes the relevant warning depend on -Wredundant-decls (which >>>>is not part of -Wall or -Wextra) so it can be disabled on the command >>>>line or by using #pragma. This means the warning will no longer be given >>>>by default, so some tests need -Wredundant-decls added. >>>> >>>>gcc/cp/ChangeLog: >>>> >>>>    PR c++/66159 >>>>    * parser.c (cp_parser_elaborated_type_specifier): Do not warn >>>>    unless in a declaration. Make warning depend on >>>>    WOPT_redundant_decls. >>>> >>>>gcc/testsuite/ChangeLog: >>>> >>>>    PR c++/66159 >>>>    * g++.dg/parse/specialization1.C: Remove dg-warning. >>>>    * g++.dg/warn/forward-inner.C: Add -Wredundant-decls. Check >>>>    alias-declaration using elaborated-type-specifier. >>>>    * g++.dg/warn/pr36999.C: Add -Wredundant-decls. >>>> >>>> >>>>Is it OK to make this warning no longer emitted by default, and not >>>>even with -Wall -Wextra? >>> >>>I'd like to keep it on by default. >>> >>>>Would it be better to add a new option for this specific warning, >>>>which would be enabled by -Wall and also by -Wredundant-decls? Maybe >>>>-Wredundant-decls-elaborated-type or something. >>> >>>That sounds fine. But do you think people will really want to >>>turn this off after you fix the bug? >> >>I don't know how to fix the bug, so I'll have to leave that to >>somebody else. > >By fixing the bug, I mean your "Do not emit the warning unless we're >actually parsing a declaration." Please do apply your change to the >condition for the warning. Ah yes, I got confused about what the patch I eventually submitted actually did, sorry! I've pushed what's attached to this mail. Thanks.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 54ca875ce54..5287ab34752 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -18917,8 +18917,10 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, here. */ if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON) - && !is_friend && !processing_explicit_instantiation) - warning (0, "declaration %qD does not declare anything", decl); + && !is_friend && is_declaration + && !processing_explicit_instantiation) + warning (OPT_Wredundant_decls, + "declaration %qD does not declare anything", decl); type = TREE_TYPE (decl); } diff --git a/gcc/testsuite/g++.dg/parse/specialization1.C b/gcc/testsuite/g++.dg/parse/specialization1.C index 44a98baa2f4..6d83bc4f254 100644 --- a/gcc/testsuite/g++.dg/parse/specialization1.C +++ b/gcc/testsuite/g++.dg/parse/specialization1.C @@ -4,4 +4,3 @@ template <typename T> class A; template <typename T> class A<T>::B; // { dg-error "declaration" "err" } -// { dg-warning "declaration" "warn" { target *-*-* } .-1 } diff --git a/gcc/testsuite/g++.dg/warn/forward-inner.C b/gcc/testsuite/g++.dg/warn/forward-inner.C index 5336d4ed946..1c10ec44a54 100644 --- a/gcc/testsuite/g++.dg/warn/forward-inner.C +++ b/gcc/testsuite/g++.dg/warn/forward-inner.C @@ -1,5 +1,6 @@ // Check that the compiler warns about inner-style forward declarations in // contexts where they're not actually illegal, but merely useless. +// { dg-options "-Wredundant-decls" } // Verify warnings for and within classes, and by extension, struct and union. class C1; @@ -70,7 +71,7 @@ template class TC6<int>::TC7; // Valid explicit instantiation, no warning // Verify that friend declarations, also easy to confuse with forward -// declrations, are similarly not warned about. +// declarations, are similarly not warned about. class C8 { public: class C9 { }; @@ -79,3 +80,10 @@ class C10 { public: friend class C8::C9; // Valid friend declaration, no warning }; + +#if __cplusplus >= 201103L +// Verify that alias-declarations using an elaborated-type-specifier and +// nested-name-specifier are not warned about (PR c++/66159). +struct C11; +using A1 = struct ::C11; // Valid alias-decl, no warning +#endif diff --git a/gcc/testsuite/g++.dg/warn/pr36999.C b/gcc/testsuite/g++.dg/warn/pr36999.C index ce2286efcf4..6f69e192d02 100644 --- a/gcc/testsuite/g++.dg/warn/pr36999.C +++ b/gcc/testsuite/g++.dg/warn/pr36999.C @@ -1,5 +1,6 @@ /* PR36999: Erroneous "does not declare anything" warnings. */ /* { dg-do compile } */ +/* { dg-options "-Wredundant-decls" } */ class C1 { public: class C2 { };
Ping ... G++ gives a bogus warning for 'struct A; using B = struct ::A;' complaining that the elaborated-type-specifier doesn't declare anything. That's true, but it's not trying to declare struct ::A, just refer to it unambiguously. Do not emit the warning unless we're actually parsing a declaration. This also makes the relevant warning depend on -Wredundant-decls (which is not part of -Wall or -Wextra) so it can be disabled on the command line or by using #pragma. This means the warning will no longer be given by default, so some tests need -Wredundant-decls added. gcc/cp/ChangeLog: PR c++/66159 * parser.c (cp_parser_elaborated_type_specifier): Do not warn unless in a declaration. Make warning depend on WOPT_redundant_decls. gcc/testsuite/ChangeLog: PR c++/66159 * g++.dg/parse/specialization1.C: Remove dg-warning. * g++.dg/warn/forward-inner.C: Add -Wredundant-decls. Check alias-declaration using elaborated-type-specifier. * g++.dg/warn/pr36999.C: Add -Wredundant-decls. Is it OK to make this warning no longer emitted by default, and not even with -Wall -Wextra? Would it be better to add a new option for this specific warning, which would be enabled by -Wall and also by -Wredundant-decls? Maybe -Wredundant-decls-elaborated-type or something. commit c254d7cb3977484fd4737b973a87c1df98c30e01 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed May 27 10:40:38 2020 +0100 c++: Fix bogus "does not declare anything" warning (PR 66159) G++ gives a bogus warning for 'struct A; using B = struct ::A;' complaining that the elaborated-type-specifier doesn't declare anything. That's true, but it's not trying to declare struct ::A, just refer to it unambiguously. Do not emit the warning unless we're actually parsing a declaration. This also makes the relevant warning depend on -Wredundant-decls (which is not part of -Wall or -Wextra) so it can be disabled on the command line or by using #pragma. This means the warning will no longer be given by default, so some tests need -Wredundant-decls added. gcc/cp/ChangeLog: PR c++/66159 * parser.c (cp_parser_elaborated_type_specifier): Do not warn unless in a declaration. Make warning depend on WOPT_redundant_decls. gcc/testsuite/ChangeLog: PR c++/66159 * g++.dg/parse/specialization1.C: Remove dg-warning. * g++.dg/warn/forward-inner.C: Add -Wredundant-decls. Check alias-declaration using elaborated-type-specifier. * g++.dg/warn/pr36999.C: Add -Wredundant-decls.