* [PATCH] c++: Fix noexcept with unevaluated operand [PR101087]
@ 2021-07-08 1:40 Marek Polacek
2021-07-08 13:30 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2021-07-08 1:40 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
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?
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
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix noexcept with unevaluated operand [PR101087]
2021-07-08 1:40 [PATCH] c++: Fix noexcept with unevaluated operand [PR101087] Marek Polacek
@ 2021-07-08 13:30 ` Jason Merrill
2021-07-08 13:35 ` Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2021-07-08 13:30 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix noexcept with unevaluated operand [PR101087]
2021-07-08 13:30 ` Jason Merrill
@ 2021-07-08 13:35 ` Marek Polacek
2021-07-08 20:26 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2021-07-08 13:35 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Fix noexcept with unevaluated operand [PR101087]
2021-07-08 13:35 ` Marek Polacek
@ 2021-07-08 20:26 ` Marek Polacek
2021-07-08 21:34 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2021-07-08 20:26 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Fix noexcept with unevaluated operand [PR101087]
2021-07-08 20:26 ` [PATCH v2] " Marek Polacek
@ 2021-07-08 21:34 ` Jason Merrill
2021-07-08 21:37 ` Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2021-07-08 21:34 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Fix noexcept with unevaluated operand [PR101087]
2021-07-08 21:34 ` Jason Merrill
@ 2021-07-08 21:37 ` Marek Polacek
0 siblings, 0 replies; 6+ messages in thread
From: Marek Polacek @ 2021-07-08 21:37 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-08 21:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08 1:40 [PATCH] c++: Fix noexcept with unevaluated operand [PR101087] Marek Polacek
2021-07-08 13:30 ` Jason Merrill
2021-07-08 13:35 ` Marek Polacek
2021-07-08 20:26 ` [PATCH v2] " Marek Polacek
2021-07-08 21:34 ` Jason Merrill
2021-07-08 21:37 ` Marek Polacek
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).