Message ID | 20200322211438.3303999-1-ppalka@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: Avoid a suspicious -Wnoexcept warning [PR93805] | expand |
On 3/22/20 5:14 PM, Patrick Palka wrote: > In this PR we're emitting -Wnoexcept warnings about potentially-throwing NSDMIs > when computing the noexcept specification of a class's defaulted default > constructor. Alhough these warnings are in some sense valid, this patch takes > the route of suppressing them, because: > > 1. the warning message is confusing in its current form; > 2. warning for 'struct C { B b = B(); };' but not for 'struct C { B b; };' > is inconsistent; and > 3. emitting a warning here arguably doesn't fall under the umbrella of > -Wnoexcept, whose documentation says it warns only when a > noexcept-expression evaluates to false, but there are noexcept-expressions > here. > > Tested on x86_64-pc-linux-gnu, does this look OK to commit? Let's add a comment to maybe_noexcept_warning suggesting that we might in future want to do something like walk_subtrees in the case of a defaulted function. OK with that change. Jason > gcc/cp/ChangeLog: > > PR c++/93805 > * method.c (walk_field_subobs): Pass tf_none as the complain argument to > expr_noexcept_p. > > gcc/testsuite/ChangeLog: > > PR c++/93805 > * g++.dg/warn/Wnoexcept2.C: New test. > --- > gcc/cp/method.c | 2 +- > gcc/testsuite/g++.dg/warn/Wnoexcept2.C | 15 +++++++++++++++ > 2 files changed, 16 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wnoexcept2.C > > diff --git a/gcc/cp/method.c b/gcc/cp/method.c > index c131fd41536..41b9ff86bdd 100644 > --- a/gcc/cp/method.c > +++ b/gcc/cp/method.c > @@ -1988,7 +1988,7 @@ walk_field_subobs (tree fields, special_function_kind sfk, tree fnname, > if (nsdmi == error_mark_node) > *spec_p = error_mark_node; > else if (*spec_p != error_mark_node > - && !expr_noexcept_p (nsdmi, complain)) > + && !expr_noexcept_p (nsdmi, tf_none)) > *spec_p = noexcept_false_spec; > } > /* Don't do the normal processing. */ > diff --git a/gcc/testsuite/g++.dg/warn/Wnoexcept2.C b/gcc/testsuite/g++.dg/warn/Wnoexcept2.C > new file mode 100644 > index 00000000000..60541be3575 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wnoexcept2.C > @@ -0,0 +1,15 @@ > +// PR c++/93805 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options "-Wnoexcept" } > + > +struct B > +{ > + B() {} > +}; > + > +struct C > +{ > + B b = B(); > +}; > + > +C c; // { dg-bogus "noexcept-expression" } >
On Mon, 23 Mar 2020, Jason Merrill wrote: > On 3/22/20 5:14 PM, Patrick Palka wrote: > > In this PR we're emitting -Wnoexcept warnings about potentially-throwing > > NSDMIs > > when computing the noexcept specification of a class's defaulted default > > constructor. Alhough these warnings are in some sense valid, this patch > > takes > > the route of suppressing them, because: > > > > 1. the warning message is confusing in its current form; > > 2. warning for 'struct C { B b = B(); };' but not for 'struct C { B b; > > };' > > is inconsistent; and > > 3. emitting a warning here arguably doesn't fall under the umbrella of > > -Wnoexcept, whose documentation says it warns only when a > > noexcept-expression evaluates to false, but there are > > noexcept-expressions > > here. > > > > Tested on x86_64-pc-linux-gnu, does this look OK to commit? > > Let's add a comment to maybe_noexcept_warning suggesting that we might in > future want to do something like walk_subtrees in the case of a defaulted > function. OK with that change. > > Jason Thanks, patch committed with a comment to that effect added to maybe_noexcept_warning. Here is the final patch: -- >8 -- gcc/cp/ChangeLog: PR c++/93805 * except.c (maybe_noexcept_warning): Add TODO comment. * method.c (walk_field_subobs): Pass tf_none to expr_noexcept_p. gcc/testsuite/ChangeLog: PR c++/93805 * g++.dg/warn/Wnoexcept2.C: New test. --- gcc/cp/ChangeLog | 6 ++++++ gcc/cp/except.c | 5 ++++- gcc/cp/method.c | 2 +- gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/g++.dg/warn/Wnoexcept2.C | 15 +++++++++++++++ 5 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wnoexcept2.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 3340f47d506..59db03c0b07 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2020-03-23 Patrick Palka <ppalka@redhat.com> + + PR c++/93805 + * except.c (maybe_noexcept_warning): Add TODO. + * method.c (walk_field_subobs): Pass tf_none to expr_noexcept_p. + 2020-03-23 nathans <nathan@acm.org> PR c++/94044 diff --git a/gcc/cp/except.c b/gcc/cp/except.c index 262ba5d309c..7e93c51f9ea 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1160,7 +1160,10 @@ struct GTY(()) pending_noexcept { static GTY(()) vec<pending_noexcept, va_gc> *pending_noexcept_checks; /* FN is a FUNCTION_DECL that caused a noexcept-expr to be false. Warn if - it can't throw. */ + it can't throw. + + TODO: Consider extending -Wnoexcept to do something like walk_subtrees in the + case of a defaulted function that obtained a noexcept(false) spec. */ static void maybe_noexcept_warning (tree fn) diff --git a/gcc/cp/method.c b/gcc/cp/method.c index c131fd41536..41b9ff86bdd 100644 --- a/gcc/cp/method.c +++ b/gcc/cp/method.c @@ -1988,7 +1988,7 @@ walk_field_subobs (tree fields, special_function_kind sfk, tree fnname, if (nsdmi == error_mark_node) *spec_p = error_mark_node; else if (*spec_p != error_mark_node - && !expr_noexcept_p (nsdmi, complain)) + && !expr_noexcept_p (nsdmi, tf_none)) *spec_p = noexcept_false_spec; } /* Don't do the normal processing. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 1d053e07721..5f079f1fca9 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-03-23 Patrick Palka <ppalka@redhat.com> + + PR c++/93805 + * g++.dg/warn/Wnoexcept2.C: New test. + 2020-03-23 Jakub Jelinek <jakub@redhat.com> PR c++/91993 diff --git a/gcc/testsuite/g++.dg/warn/Wnoexcept2.C b/gcc/testsuite/g++.dg/warn/Wnoexcept2.C new file mode 100644 index 00000000000..60541be3575 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wnoexcept2.C @@ -0,0 +1,15 @@ +// PR c++/93805 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-Wnoexcept" } + +struct B +{ + B() {} +}; + +struct C +{ + B b = B(); +}; + +C c; // { dg-bogus "noexcept-expression" }
diff --git a/gcc/cp/method.c b/gcc/cp/method.c index c131fd41536..41b9ff86bdd 100644 --- a/gcc/cp/method.c +++ b/gcc/cp/method.c @@ -1988,7 +1988,7 @@ walk_field_subobs (tree fields, special_function_kind sfk, tree fnname, if (nsdmi == error_mark_node) *spec_p = error_mark_node; else if (*spec_p != error_mark_node - && !expr_noexcept_p (nsdmi, complain)) + && !expr_noexcept_p (nsdmi, tf_none)) *spec_p = noexcept_false_spec; } /* Don't do the normal processing. */ diff --git a/gcc/testsuite/g++.dg/warn/Wnoexcept2.C b/gcc/testsuite/g++.dg/warn/Wnoexcept2.C new file mode 100644 index 00000000000..60541be3575 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wnoexcept2.C @@ -0,0 +1,15 @@ +// PR c++/93805 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-Wnoexcept" } + +struct B +{ + B() {} +}; + +struct C +{ + B b = B(); +}; + +C c; // { dg-bogus "noexcept-expression" }