Message ID | 20210708014058.854624-1-polacek@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: Fix noexcept with unevaluated operand [PR101087] | expand |
On 7/7/21 9:40 PM, Marek Polacek wrote: > It sounds plausible that this assert > > int f(); > static_assert(noexcept(sizeof(f()))); > > should pass: sizeof produces a std::size_t and its operand is not > evaluated, so it can't throw. noexcept should only evaluate to > false for potentially evaluated operands. Therefore I think that > check_noexcept_r shouldn't walk into operands of sizeof/decltype/ > alignof/typeof. Only checking cp_unevaluated_operand therein does > not work, because expr_noexcept_p can be called in an unevaluated > context, so I resorted to the following cp_evaluated hack. Does > that seem acceptable? I suppose, but why not check for SIZEOF_EXPR/ALIGNOF_EXPR/NOEXCEPT_EXPR directly? > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR c++/101087 > > gcc/cp/ChangeLog: > > * except.c (check_noexcept_r): Don't walk into unevaluated > operands. > (expr_noexcept_p): Use cp_evaluated. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/noexcept70.C: New test. > --- > gcc/cp/except.c | 14 +++++++++++--- > gcc/testsuite/g++.dg/cpp0x/noexcept70.C | 5 +++++ > 2 files changed, 16 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept70.C > > diff --git a/gcc/cp/except.c b/gcc/cp/except.c > index a8cea53cf91..6f97ac40b4b 100644 > --- a/gcc/cp/except.c > +++ b/gcc/cp/except.c > @@ -1033,12 +1033,15 @@ check_handlers (tree handlers) > expression whose type is a polymorphic class type (10.3). */ > > static tree > -check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void * /*data*/) > +check_noexcept_r (tree *tp, int *walk_subtrees, void *) > { > tree t = *tp; > enum tree_code code = TREE_CODE (t); > - if ((code == CALL_EXPR && CALL_EXPR_FN (t)) > - || code == AGGR_INIT_EXPR) > + > + if (cp_unevaluated_operand) > + *walk_subtrees = false; > + else if ((code == CALL_EXPR && CALL_EXPR_FN (t)) > + || code == AGGR_INIT_EXPR) > { > /* We can only use the exception specification of the called function > for determining the value of a noexcept expression; we can't use > @@ -1155,6 +1158,11 @@ expr_noexcept_p (tree expr, tsubst_flags_t complain) > if (expr == error_mark_node) > return false; > > + /* Even though the operand of noexcept is an _unevaluated_ operand, > + temporarily clearing cp_unevaluated_operand allows us to check it > + in check_noexcept_r, to handle noexcept(sizeof(f())). It could be > + set when we are called in the context of synthesized_method_walk. */ > + cp_evaluated ev; > fn = cp_walk_tree_without_duplicates (&expr, check_noexcept_r, 0); > if (fn) > { > diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept70.C b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C > new file mode 100644 > index 00000000000..45a6137dd6f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C > @@ -0,0 +1,5 @@ > +// PR c++/101087 > +// { dg-do compile { target c++11 } } > + > +int f(); > +static_assert(noexcept(sizeof(f())), ""); > > base-commit: a110855667782dac7b674d3e328b253b3b3c919b >
On Thu, Jul 08, 2021 at 09:30:27AM -0400, Jason Merrill wrote: > On 7/7/21 9:40 PM, Marek Polacek wrote: > > It sounds plausible that this assert > > > > int f(); > > static_assert(noexcept(sizeof(f()))); > > > > should pass: sizeof produces a std::size_t and its operand is not > > evaluated, so it can't throw. noexcept should only evaluate to > > false for potentially evaluated operands. Therefore I think that > > check_noexcept_r shouldn't walk into operands of sizeof/decltype/ > > alignof/typeof. Only checking cp_unevaluated_operand therein does > > not work, because expr_noexcept_p can be called in an unevaluated > > context, so I resorted to the following cp_evaluated hack. Does > > that seem acceptable? > > I suppose, but why not check for SIZEOF_EXPR/ALIGNOF_EXPR/NOEXCEPT_EXPR > directly? I thought I would, but then it occurred to me that it might be better to rely on cp_walk_subtrees which ++/--s cp_unevaluated_operand for those codes. I'd be happy to change the patch to check those codes directly; maybe I'm overthinking things here. -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
On Thu, Jul 08, 2021 at 09:35:02AM -0400, Marek Polacek wrote: > On Thu, Jul 08, 2021 at 09:30:27AM -0400, Jason Merrill wrote: > > On 7/7/21 9:40 PM, Marek Polacek wrote: > > > It sounds plausible that this assert > > > > > > int f(); > > > static_assert(noexcept(sizeof(f()))); > > > > > > should pass: sizeof produces a std::size_t and its operand is not > > > evaluated, so it can't throw. noexcept should only evaluate to > > > false for potentially evaluated operands. Therefore I think that > > > check_noexcept_r shouldn't walk into operands of sizeof/decltype/ > > > alignof/typeof. Only checking cp_unevaluated_operand therein does > > > not work, because expr_noexcept_p can be called in an unevaluated > > > context, so I resorted to the following cp_evaluated hack. Does > > > that seem acceptable? > > > > I suppose, but why not check for SIZEOF_EXPR/ALIGNOF_EXPR/NOEXCEPT_EXPR > > directly? > > I thought I would, but then it occurred to me that it might be better to > rely on cp_walk_subtrees which ++/--s cp_unevaluated_operand for those > codes. I'd be happy to change the patch to check those codes directly; > maybe I'm overthinking things here. So here's v2 which checks the codes directly, via a new inline: Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- It sounds plausible that this assert int f(); static_assert(noexcept(sizeof(f()))); should pass: sizeof produces a std::size_t and its operand is not evaluated, so it can't throw. noexcept should only evaluate to false for potentially evaluated operands. Therefore I think that check_noexcept_r shouldn't walk into operands of sizeof/decltype/ alignof/typeof. PR c++/101087 gcc/cp/ChangeLog: * cp-tree.h (unevaluated_p): New. * except.c (check_noexcept_r): Use it. Don't walk into unevaluated operands. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/noexcept70.C: New test. --- gcc/cp/cp-tree.h | 13 +++++++++++++ gcc/cp/except.c | 9 ++++++--- gcc/testsuite/g++.dg/cpp0x/noexcept70.C | 5 +++++ 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept70.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index b4501576b26..d4810c0c986 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -8465,6 +8465,19 @@ is_constrained_auto (const_tree t) return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t); } +/* True if CODE, a tree code, denotes a tree whose operand is not evaluated + as per [expr.context], i.e., an operand to sizeof, typeof, decltype, or + alignof. */ + +inline bool +unevaluated_p (tree_code code) +{ + return (code == DECLTYPE_TYPE + || code == ALIGNOF_EXPR + || code == SIZEOF_EXPR + || code == NOEXCEPT_EXPR); +} + /* RAII class to push/pop the access scope for T. */ struct push_access_scope_guard diff --git a/gcc/cp/except.c b/gcc/cp/except.c index a8cea53cf91..a8acbc4b7b2 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1033,12 +1033,15 @@ check_handlers (tree handlers) expression whose type is a polymorphic class type (10.3). */ static tree -check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void * /*data*/) +check_noexcept_r (tree *tp, int *walk_subtrees, void *) { tree t = *tp; enum tree_code code = TREE_CODE (t); - if ((code == CALL_EXPR && CALL_EXPR_FN (t)) - || code == AGGR_INIT_EXPR) + + if (unevaluated_p (code)) + *walk_subtrees = false; + else if ((code == CALL_EXPR && CALL_EXPR_FN (t)) + || code == AGGR_INIT_EXPR) { /* We can only use the exception specification of the called function for determining the value of a noexcept expression; we can't use diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept70.C b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C new file mode 100644 index 00000000000..45a6137dd6f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C @@ -0,0 +1,5 @@ +// PR c++/101087 +// { dg-do compile { target c++11 } } + +int f(); +static_assert(noexcept(sizeof(f())), ""); base-commit: 763121ccd908f52bc666f277ea2cf42110b3aad9
On 7/8/21 4:26 PM, Marek Polacek wrote: > On Thu, Jul 08, 2021 at 09:35:02AM -0400, Marek Polacek wrote: >> On Thu, Jul 08, 2021 at 09:30:27AM -0400, Jason Merrill wrote: >>> On 7/7/21 9:40 PM, Marek Polacek wrote: >>>> It sounds plausible that this assert >>>> >>>> int f(); >>>> static_assert(noexcept(sizeof(f()))); >>>> >>>> should pass: sizeof produces a std::size_t and its operand is not >>>> evaluated, so it can't throw. noexcept should only evaluate to >>>> false for potentially evaluated operands. Therefore I think that >>>> check_noexcept_r shouldn't walk into operands of sizeof/decltype/ >>>> alignof/typeof. Only checking cp_unevaluated_operand therein does >>>> not work, because expr_noexcept_p can be called in an unevaluated >>>> context, so I resorted to the following cp_evaluated hack. Does >>>> that seem acceptable? >>> >>> I suppose, but why not check for SIZEOF_EXPR/ALIGNOF_EXPR/NOEXCEPT_EXPR >>> directly? >> >> I thought I would, but then it occurred to me that it might be better to >> rely on cp_walk_subtrees which ++/--s cp_unevaluated_operand for those >> codes. I'd be happy to change the patch to check those codes directly; >> maybe I'm overthinking things here. > > So here's v2 which checks the codes directly, via a new inline: > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK for trunk and 11, at least. I lean toward putting it on older release branches as well, but it doesn't seem urgent. > -- >8 -- > It sounds plausible that this assert > > int f(); > static_assert(noexcept(sizeof(f()))); > > should pass: sizeof produces a std::size_t and its operand is not > evaluated, so it can't throw. noexcept should only evaluate to > false for potentially evaluated operands. Therefore I think that > check_noexcept_r shouldn't walk into operands of sizeof/decltype/ > alignof/typeof. > > PR c++/101087 > > gcc/cp/ChangeLog: > > * cp-tree.h (unevaluated_p): New. > * except.c (check_noexcept_r): Use it. Don't walk into > unevaluated operands. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/noexcept70.C: New test. > --- > gcc/cp/cp-tree.h | 13 +++++++++++++ > gcc/cp/except.c | 9 ++++++--- > gcc/testsuite/g++.dg/cpp0x/noexcept70.C | 5 +++++ > 3 files changed, 24 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept70.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index b4501576b26..d4810c0c986 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -8465,6 +8465,19 @@ is_constrained_auto (const_tree t) > return is_auto (t) && PLACEHOLDER_TYPE_CONSTRAINTS_INFO (t); > } > > +/* True if CODE, a tree code, denotes a tree whose operand is not evaluated > + as per [expr.context], i.e., an operand to sizeof, typeof, decltype, or > + alignof. */ > + > +inline bool > +unevaluated_p (tree_code code) > +{ > + return (code == DECLTYPE_TYPE > + || code == ALIGNOF_EXPR > + || code == SIZEOF_EXPR > + || code == NOEXCEPT_EXPR); > +} > + > /* RAII class to push/pop the access scope for T. */ > > struct push_access_scope_guard > diff --git a/gcc/cp/except.c b/gcc/cp/except.c > index a8cea53cf91..a8acbc4b7b2 100644 > --- a/gcc/cp/except.c > +++ b/gcc/cp/except.c > @@ -1033,12 +1033,15 @@ check_handlers (tree handlers) > expression whose type is a polymorphic class type (10.3). */ > > static tree > -check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void * /*data*/) > +check_noexcept_r (tree *tp, int *walk_subtrees, void *) > { > tree t = *tp; > enum tree_code code = TREE_CODE (t); > - if ((code == CALL_EXPR && CALL_EXPR_FN (t)) > - || code == AGGR_INIT_EXPR) > + > + if (unevaluated_p (code)) > + *walk_subtrees = false; > + else if ((code == CALL_EXPR && CALL_EXPR_FN (t)) > + || code == AGGR_INIT_EXPR) > { > /* We can only use the exception specification of the called function > for determining the value of a noexcept expression; we can't use > diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept70.C b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C > new file mode 100644 > index 00000000000..45a6137dd6f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C > @@ -0,0 +1,5 @@ > +// PR c++/101087 > +// { dg-do compile { target c++11 } } > + > +int f(); > +static_assert(noexcept(sizeof(f())), ""); > > base-commit: 763121ccd908f52bc666f277ea2cf42110b3aad9 >
On Thu, Jul 08, 2021 at 05:34:24PM -0400, Jason Merrill wrote: > On 7/8/21 4:26 PM, Marek Polacek wrote: > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > OK for trunk and 11, at least. I lean toward putting it on older release > branches as well, but it doesn't seem urgent. Ok, I'll backport to 11 and 10, it seems very safe. Thanks, Marek
diff --git a/gcc/cp/except.c b/gcc/cp/except.c index a8cea53cf91..6f97ac40b4b 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1033,12 +1033,15 @@ check_handlers (tree handlers) expression whose type is a polymorphic class type (10.3). */ static tree -check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void * /*data*/) +check_noexcept_r (tree *tp, int *walk_subtrees, void *) { tree t = *tp; enum tree_code code = TREE_CODE (t); - if ((code == CALL_EXPR && CALL_EXPR_FN (t)) - || code == AGGR_INIT_EXPR) + + if (cp_unevaluated_operand) + *walk_subtrees = false; + else if ((code == CALL_EXPR && CALL_EXPR_FN (t)) + || code == AGGR_INIT_EXPR) { /* We can only use the exception specification of the called function for determining the value of a noexcept expression; we can't use @@ -1155,6 +1158,11 @@ expr_noexcept_p (tree expr, tsubst_flags_t complain) if (expr == error_mark_node) return false; + /* Even though the operand of noexcept is an _unevaluated_ operand, + temporarily clearing cp_unevaluated_operand allows us to check it + in check_noexcept_r, to handle noexcept(sizeof(f())). It could be + set when we are called in the context of synthesized_method_walk. */ + cp_evaluated ev; fn = cp_walk_tree_without_duplicates (&expr, check_noexcept_r, 0); if (fn) { diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept70.C b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C new file mode 100644 index 00000000000..45a6137dd6f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept70.C @@ -0,0 +1,5 @@ +// PR c++/101087 +// { dg-do compile { target c++11 } } + +int f(); +static_assert(noexcept(sizeof(f())), "");