* C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) @ 2018-01-25 21:58 Marek Polacek 2018-01-25 22:37 ` Marek Polacek 2018-02-02 19:11 ` Jason Merrill 0 siblings, 2 replies; 8+ messages in thread From: Marek Polacek @ 2018-01-25 21:58 UTC (permalink / raw) To: GCC Patches, Jason Merrill This is a similar problem to 83116: we'd cached a constexpr call, but after a store the result had become invalid, yet we used the wrong result again when encountering the same call later. This resulted in evaluating a THROW_EXPR which doesn't work. Details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 The fix for 83116 didn't work here, because when evaluating the body of the ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do cache. It seems that we may no longer rely on the constexpr call table when we do cxx_eval_store_expression, because that just rewrites *valp, i.e. the value of an object. Might be too big a hammer again, but I couldn't think of how I could guard the caching of a constexpr call. This doesn't manifest in C++14 because build_special_member_call in C++17 is more aggressive with copy elisions (as required by P0135 which changed how we view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so expand_default_init calls maybe_constant_init, for which STRICT is false, so we avoid caching as per 83116. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-01-25 Marek Polacek <polacek@redhat.com> PR c++/83692 * constexpr.c (cxx_eval_store_expression): Clear constexpr_call_table. * g++.dg/cpp1y/constexpr-83692.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 4d2ee4a28fc..0202d22f320 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -3663,6 +3663,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, else *valp = init; + /* We've rewritten a value of a temporary in this constexpr + context which might invalide a cached call. */ + constexpr_call_table = NULL; + /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing CONSTRUCTORs, if any. */ tree elt; diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-83692.C gcc/testsuite/g++.dg/cpp1y/constexpr-83692.C index e69de29bb2d..292ba7c22e9 100644 --- gcc/testsuite/g++.dg/cpp1y/constexpr-83692.C +++ gcc/testsuite/g++.dg/cpp1y/constexpr-83692.C @@ -0,0 +1,21 @@ +// PR c++/83692 +// { dg-do compile { target c++14 } } + +struct integer { + constexpr int value() const { return m_value; } + int m_value; +}; + +struct outer { + integer m_x{0}; + constexpr outer() + { + if (m_x.value() != 0) + throw 0; + m_x.m_value = integer{1}.value(); + if (m_x.value() != 1) + throw 0; + } +}; + +constexpr outer o{}; Marek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-01-25 21:58 C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) Marek Polacek @ 2018-01-25 22:37 ` Marek Polacek 2018-02-02 19:11 ` Jason Merrill 1 sibling, 0 replies; 8+ messages in thread From: Marek Polacek @ 2018-01-25 22:37 UTC (permalink / raw) To: GCC Patches, Jason Merrill On Thu, Jan 25, 2018 at 10:16:39PM +0100, Marek Polacek wrote: > This is a similar problem to 83116: we'd cached a constexpr call, but after a > store the result had become invalid, yet we used the wrong result again when > encountering the same call later. This resulted in evaluating a THROW_EXPR > which doesn't work. Details in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 > > The fix for 83116 didn't work here, because when evaluating the body of the > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do > cache. > > It seems that we may no longer rely on the constexpr call table when we > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the > value of an object. Might be too big a hammer again, but I couldn't think > of how I could guard the caching of a constexpr call. > > This doesn't manifest in C++14 because build_special_member_call in C++17 is > more aggressive with copy elisions (as required by P0135 which changed how we > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so > expand_default_init calls maybe_constant_init, for which STRICT is false, so > we avoid caching as per 83116. ...so the testcase should actually test c++17. Consider that fixed. Marek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-01-25 21:58 C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) Marek Polacek 2018-01-25 22:37 ` Marek Polacek @ 2018-02-02 19:11 ` Jason Merrill 2018-02-05 13:38 ` Marek Polacek 1 sibling, 1 reply; 8+ messages in thread From: Jason Merrill @ 2018-02-02 19:11 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek <polacek@redhat.com> wrote: > This is a similar problem to 83116: we'd cached a constexpr call, but after a > store the result had become invalid, yet we used the wrong result again when > encountering the same call later. This resulted in evaluating a THROW_EXPR > which doesn't work. Details in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 > > The fix for 83116 didn't work here, because when evaluating the body of the > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do > cache. > It seems that we may no longer rely on the constexpr call table when we > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the > value of an object. Might be too big a hammer again, but I couldn't think > of how I could guard the caching of a constexpr call. > This doesn't manifest in C++14 because build_special_member_call in C++17 is > more aggressive with copy elisions (as required by P0135 which changed how we > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so > expand_default_init calls maybe_constant_init, for which STRICT is false, so > we avoid caching as per 83116. So it sounds like the problem is using cxx_constant_value for the diagnostic when it has different semantics from the maybe_constant_init that follows right after. I guess we want a cxx_constant_init function that is a hybrid of the two. Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-02-02 19:11 ` Jason Merrill @ 2018-02-05 13:38 ` Marek Polacek 2018-02-05 18:45 ` Jason Merrill 0 siblings, 1 reply; 8+ messages in thread From: Marek Polacek @ 2018-02-05 13:38 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Fri, Feb 02, 2018 at 02:11:27PM -0500, Jason Merrill wrote: > On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek <polacek@redhat.com> wrote: > > This is a similar problem to 83116: we'd cached a constexpr call, but after a > > store the result had become invalid, yet we used the wrong result again when > > encountering the same call later. This resulted in evaluating a THROW_EXPR > > which doesn't work. Details in > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 > > > > The fix for 83116 didn't work here, because when evaluating the body of the > > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do > > cache. > > > It seems that we may no longer rely on the constexpr call table when we > > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the > > value of an object. Might be too big a hammer again, but I couldn't think > > of how I could guard the caching of a constexpr call. > > > This doesn't manifest in C++14 because build_special_member_call in C++17 is > > more aggressive with copy elisions (as required by P0135 which changed how we > > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so > > expand_default_init calls maybe_constant_init, for which STRICT is false, so > > we avoid caching as per 83116. > > So it sounds like the problem is using cxx_constant_value for the > diagnostic when it has different semantics from the > maybe_constant_init that follows right after. I guess we want a > cxx_constant_init function that is a hybrid of the two. So like the following? Thanks, Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-02-04 Marek Polacek <polacek@redhat.com> PR c++/83692 * constexpr.c (cxx_constant_init): New function. * cp-tree.h (cxx_constant_init): Declare. * typeck2.c (store_init_value): Call cxx_constant_init instead of cxx_constant_value. * g++.dg/cpp1z/constexpr-83692.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 93dd8ae049c..f95aacf2580 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -4902,6 +4902,14 @@ cxx_constant_value (tree t, tree decl) return cxx_eval_outermost_constant_expr (t, false, true, decl); } +/* Like cxx_constant_value, but non-strict mode. */ + +tree +cxx_constant_init (tree t, tree decl) +{ + return cxx_eval_outermost_constant_expr (t, false, false, decl); +} + /* Helper routine for fold_simple function. Either return simplified expression T, otherwise NULL_TREE. In contrast to cp_fully_fold, and to maybe_constant_value, we try to fold diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h index a53f4fd9c03..9f973305fbb 100644 --- gcc/cp/cp-tree.h +++ gcc/cp/cp-tree.h @@ -7417,6 +7417,7 @@ extern bool require_potential_constant_expression (tree); extern bool require_constant_expression (tree); extern bool require_potential_rvalue_constant_expression (tree); extern tree cxx_constant_value (tree, tree = NULL_TREE); +extern tree cxx_constant_init (tree, tree = NULL_TREE); extern tree maybe_constant_value (tree, tree = NULL_TREE); extern tree maybe_constant_init (tree, tree = NULL_TREE); extern tree fold_non_dependent_expr (tree); diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c index 899d60e8535..b4abc54f537 100644 --- gcc/cp/typeck2.c +++ gcc/cp/typeck2.c @@ -830,7 +830,7 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags) if (!require_constant_expression (value)) value = error_mark_node; else - value = cxx_constant_value (value, decl); + value = cxx_constant_init (value, decl); } value = maybe_constant_init (value, decl); if (TREE_CODE (value) == CONSTRUCTOR && cp_has_mutable_p (type)) diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C index e69de29bb2d..f6b61eeab85 100644 --- gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C +++ gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C @@ -0,0 +1,21 @@ +// PR c++/83692 +// { dg-options -std=c++17 } + +struct integer { + constexpr int value() const { return m_value; } + int m_value; +}; + +struct outer { + integer m_x{0}; + constexpr outer() + { + if (m_x.value() != 0) + throw 0; + m_x.m_value = integer{1}.value(); + if (m_x.value() != 1) + throw 0; + } +}; + +constexpr outer o{}; Marek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-02-05 13:38 ` Marek Polacek @ 2018-02-05 18:45 ` Jason Merrill 2018-02-16 21:10 ` Jason Merrill 0 siblings, 1 reply; 8+ messages in thread From: Jason Merrill @ 2018-02-05 18:45 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On Mon, Feb 5, 2018 at 8:37 AM, Marek Polacek <polacek@redhat.com> wrote: > On Fri, Feb 02, 2018 at 02:11:27PM -0500, Jason Merrill wrote: >> On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek <polacek@redhat.com> wrote: >> > This is a similar problem to 83116: we'd cached a constexpr call, but after a >> > store the result had become invalid, yet we used the wrong result again when >> > encountering the same call later. This resulted in evaluating a THROW_EXPR >> > which doesn't work. Details in >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 >> > >> > The fix for 83116 didn't work here, because when evaluating the body of the >> > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do >> > cache. >> >> > It seems that we may no longer rely on the constexpr call table when we >> > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the >> > value of an object. Might be too big a hammer again, but I couldn't think >> > of how I could guard the caching of a constexpr call. >> >> > This doesn't manifest in C++14 because build_special_member_call in C++17 is >> > more aggressive with copy elisions (as required by P0135 which changed how we >> > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so >> > expand_default_init calls maybe_constant_init, for which STRICT is false, so >> > we avoid caching as per 83116. >> >> So it sounds like the problem is using cxx_constant_value for the >> diagnostic when it has different semantics from the >> maybe_constant_init that follows right after. I guess we want a >> cxx_constant_init function that is a hybrid of the two. > > So like the following? Thanks, > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-02-04 Marek Polacek <polacek@redhat.com> > > PR c++/83692 > * constexpr.c (cxx_constant_init): New function. > * cp-tree.h (cxx_constant_init): Declare. > * typeck2.c (store_init_value): Call cxx_constant_init instead of > cxx_constant_value. > > +/* Like cxx_constant_value, but non-strict mode. */ > + > +tree > +cxx_constant_init (tree t, tree decl) > +{ > + return cxx_eval_outermost_constant_expr (t, false, false, decl); > +} Hmm, that doesn't do the TARGET_EXPR stripping that maybe_constant_init does. I was thinking of a version of maybe_constant_init that passes false to allow_non_constant. Probably by making "maybe_constant_init" and cxx_constant_init both call the current function with an additional parameter. And then the existing call to maybe_constant_init can move under an 'else' to avoid redundant constexpr evaluation. Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-02-05 18:45 ` Jason Merrill @ 2018-02-16 21:10 ` Jason Merrill 2018-02-23 14:30 ` Marek Polacek 0 siblings, 1 reply; 8+ messages in thread From: Jason Merrill @ 2018-02-16 21:10 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches On Mon, Feb 5, 2018 at 1:45 PM, Jason Merrill <jason@redhat.com> wrote: > On Mon, Feb 5, 2018 at 8:37 AM, Marek Polacek <polacek@redhat.com> wrote: >> On Fri, Feb 02, 2018 at 02:11:27PM -0500, Jason Merrill wrote: >>> On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek <polacek@redhat.com> wrote: >>> > This is a similar problem to 83116: we'd cached a constexpr call, but after a >>> > store the result had become invalid, yet we used the wrong result again when >>> > encountering the same call later. This resulted in evaluating a THROW_EXPR >>> > which doesn't work. Details in >>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 >>> > >>> > The fix for 83116 didn't work here, because when evaluating the body of the >>> > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do >>> > cache. >>> >>> > It seems that we may no longer rely on the constexpr call table when we >>> > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the >>> > value of an object. Might be too big a hammer again, but I couldn't think >>> > of how I could guard the caching of a constexpr call. >>> >>> > This doesn't manifest in C++14 because build_special_member_call in C++17 is >>> > more aggressive with copy elisions (as required by P0135 which changed how we >>> > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so >>> > expand_default_init calls maybe_constant_init, for which STRICT is false, so >>> > we avoid caching as per 83116. >>> >>> So it sounds like the problem is using cxx_constant_value for the >>> diagnostic when it has different semantics from the >>> maybe_constant_init that follows right after. I guess we want a >>> cxx_constant_init function that is a hybrid of the two. >> >> So like the following? Thanks, >> >> Bootstrapped/regtested on x86_64-linux, ok for trunk? >> >> 2018-02-04 Marek Polacek <polacek@redhat.com> >> >> PR c++/83692 >> * constexpr.c (cxx_constant_init): New function. >> * cp-tree.h (cxx_constant_init): Declare. >> * typeck2.c (store_init_value): Call cxx_constant_init instead of >> cxx_constant_value. >> >> +/* Like cxx_constant_value, but non-strict mode. */ >> + >> +tree >> +cxx_constant_init (tree t, tree decl) >> +{ >> + return cxx_eval_outermost_constant_expr (t, false, false, decl); >> +} > > Hmm, that doesn't do the TARGET_EXPR stripping that > maybe_constant_init does. I was thinking of a version of > maybe_constant_init that passes false to allow_non_constant. Probably > by making "maybe_constant_init" and cxx_constant_init both call the > current function with an additional parameter. And then the existing > call to maybe_constant_init can move under an 'else' to avoid > redundant constexpr evaluation. Want me to take this over? Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-02-16 21:10 ` Jason Merrill @ 2018-02-23 14:30 ` Marek Polacek 2018-02-24 1:55 ` Jason Merrill 0 siblings, 1 reply; 8+ messages in thread From: Marek Polacek @ 2018-02-23 14:30 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches On Fri, Feb 16, 2018 at 04:10:20PM -0500, Jason Merrill wrote: > On Mon, Feb 5, 2018 at 1:45 PM, Jason Merrill <jason@redhat.com> wrote: > > On Mon, Feb 5, 2018 at 8:37 AM, Marek Polacek <polacek@redhat.com> wrote: > >> On Fri, Feb 02, 2018 at 02:11:27PM -0500, Jason Merrill wrote: > >>> On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek <polacek@redhat.com> wrote: > >>> > This is a similar problem to 83116: we'd cached a constexpr call, but after a > >>> > store the result had become invalid, yet we used the wrong result again when > >>> > encountering the same call later. This resulted in evaluating a THROW_EXPR > >>> > which doesn't work. Details in > >>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 > >>> > > >>> > The fix for 83116 didn't work here, because when evaluating the body of the > >>> > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do > >>> > cache. > >>> > >>> > It seems that we may no longer rely on the constexpr call table when we > >>> > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the > >>> > value of an object. Might be too big a hammer again, but I couldn't think > >>> > of how I could guard the caching of a constexpr call. > >>> > >>> > This doesn't manifest in C++14 because build_special_member_call in C++17 is > >>> > more aggressive with copy elisions (as required by P0135 which changed how we > >>> > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so > >>> > expand_default_init calls maybe_constant_init, for which STRICT is false, so > >>> > we avoid caching as per 83116. > >>> > >>> So it sounds like the problem is using cxx_constant_value for the > >>> diagnostic when it has different semantics from the > >>> maybe_constant_init that follows right after. I guess we want a > >>> cxx_constant_init function that is a hybrid of the two. > >> > >> So like the following? Thanks, > >> > >> Bootstrapped/regtested on x86_64-linux, ok for trunk? > >> > >> 2018-02-04 Marek Polacek <polacek@redhat.com> > >> > >> PR c++/83692 > >> * constexpr.c (cxx_constant_init): New function. > >> * cp-tree.h (cxx_constant_init): Declare. > >> * typeck2.c (store_init_value): Call cxx_constant_init instead of > >> cxx_constant_value. > >> > >> +/* Like cxx_constant_value, but non-strict mode. */ > >> + > >> +tree > >> +cxx_constant_init (tree t, tree decl) > >> +{ > >> + return cxx_eval_outermost_constant_expr (t, false, false, decl); > >> +} > > > > Hmm, that doesn't do the TARGET_EXPR stripping that > > maybe_constant_init does. I was thinking of a version of > > maybe_constant_init that passes false to allow_non_constant. Probably > > by making "maybe_constant_init" and cxx_constant_init both call the > > current function with an additional parameter. And then the existing > > call to maybe_constant_init can move under an 'else' to avoid > > redundant constexpr evaluation. > > Want me to take this over? Sorry again for the delay. I tried to do what you suggested. There was one twist: it regressed constexpr-nullptr-2.C, in particular we lost diagnostics for constexpr int* pj0 = &((S*)0)->j; // { dg-error "not a constant expression" } constexpr int* pj1 = &((S*)nullptr)->j; // { dg-error "not a constant expression" } because when maybe_constant_init_1 saw a constant: 5142 else if (CONSTANT_CLASS_P (t)) 5143 /* No evaluation needed. */; so it didn't call cxx_eval_outermost_constant_expr which is supposed to give the error. I fixed it by adding "&& allow_non_constant" so now it gives the proper diagnostics. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-02-23 Marek Polacek <polacek@redhat.com> PR c++/83692 * constexpr.c (maybe_constant_init_1): New function. (maybe_constant_init): Make it a wrapper around maybe_constant_init_1. (cxx_constant_init): New function. * cp-tree.h (cxx_constant_init): Declare. * typeck2.c (store_init_value): Call cxx_constant_init instead of cxx_constant_value. Move the maybe_constant_init call under an 'else'. * g++.dg/cpp1z/constexpr-83692.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 47ff90cb055..26d0d099a05 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -5123,8 +5123,8 @@ fold_non_dependent_expr (tree t) /* Like maybe_constant_value, but returns a CONSTRUCTOR directly, rather than wrapped in a TARGET_EXPR. */ -tree -maybe_constant_init (tree t, tree decl) +static tree +maybe_constant_init_1 (tree t, tree decl, bool allow_non_constant) { if (!t) return t; @@ -5139,10 +5139,10 @@ maybe_constant_init (tree t, tree decl) t = TARGET_EXPR_INITIAL (t); if (!is_nondependent_static_init_expression (t)) /* Don't try to evaluate it. */; - else if (CONSTANT_CLASS_P (t)) + else if (CONSTANT_CLASS_P (t) && allow_non_constant) /* No evaluation needed. */; else - t = cxx_eval_outermost_constant_expr (t, true, false, decl); + t = cxx_eval_outermost_constant_expr (t, allow_non_constant, false, decl); if (TREE_CODE (t) == TARGET_EXPR) { tree init = TARGET_EXPR_INITIAL (t); @@ -5152,6 +5152,22 @@ maybe_constant_init (tree t, tree decl) return t; } +/* Wrapper for maybe_constant_init_1 which permits non constants. */ + +tree +maybe_constant_init (tree t, tree decl) +{ + return maybe_constant_init_1 (t, decl, true); +} + +/* Wrapper for maybe_constant_init_1 which does not permit non constants. */ + +tree +cxx_constant_init (tree t, tree decl) +{ + return maybe_constant_init_1 (t, decl, false); +} + #if 0 /* FIXME see ADDR_EXPR section in potential_constant_expression_1. */ /* Return true if the object referred to by REF has automatic or thread diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h index 9038d677b2d..04c7b7ce3a9 100644 --- gcc/cp/cp-tree.h +++ gcc/cp/cp-tree.h @@ -7411,6 +7411,7 @@ extern bool require_potential_constant_expression (tree); extern bool require_constant_expression (tree); extern bool require_potential_rvalue_constant_expression (tree); extern tree cxx_constant_value (tree, tree = NULL_TREE); +extern tree cxx_constant_init (tree, tree = NULL_TREE); extern tree maybe_constant_value (tree, tree = NULL_TREE); extern tree maybe_constant_init (tree, tree = NULL_TREE); extern tree fold_non_dependent_expr (tree); diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c index 899d60e8535..153b46cca77 100644 --- gcc/cp/typeck2.c +++ gcc/cp/typeck2.c @@ -830,9 +830,10 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags) if (!require_constant_expression (value)) value = error_mark_node; else - value = cxx_constant_value (value, decl); + value = cxx_constant_init (value, decl); } - value = maybe_constant_init (value, decl); + else + value = maybe_constant_init (value, decl); if (TREE_CODE (value) == CONSTRUCTOR && cp_has_mutable_p (type)) /* Poison this CONSTRUCTOR so it can't be copied to another constexpr variable. */ diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C index e69de29bb2d..f6b61eeab85 100644 --- gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C +++ gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C @@ -0,0 +1,21 @@ +// PR c++/83692 +// { dg-options -std=c++17 } + +struct integer { + constexpr int value() const { return m_value; } + int m_value; +}; + +struct outer { + integer m_x{0}; + constexpr outer() + { + if (m_x.value() != 0) + throw 0; + m_x.m_value = integer{1}.value(); + if (m_x.value() != 1) + throw 0; + } +}; + +constexpr outer o{}; Marek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) 2018-02-23 14:30 ` Marek Polacek @ 2018-02-24 1:55 ` Jason Merrill 0 siblings, 0 replies; 8+ messages in thread From: Jason Merrill @ 2018-02-24 1:55 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches OK, thanks. On Fri, Feb 23, 2018 at 9:29 AM, Marek Polacek <polacek@redhat.com> wrote: > On Fri, Feb 16, 2018 at 04:10:20PM -0500, Jason Merrill wrote: >> On Mon, Feb 5, 2018 at 1:45 PM, Jason Merrill <jason@redhat.com> wrote: >> > On Mon, Feb 5, 2018 at 8:37 AM, Marek Polacek <polacek@redhat.com> wrote: >> >> On Fri, Feb 02, 2018 at 02:11:27PM -0500, Jason Merrill wrote: >> >>> On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek <polacek@redhat.com> wrote: >> >>> > This is a similar problem to 83116: we'd cached a constexpr call, but after a >> >>> > store the result had become invalid, yet we used the wrong result again when >> >>> > encountering the same call later. This resulted in evaluating a THROW_EXPR >> >>> > which doesn't work. Details in >> >>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 >> >>> > >> >>> > The fix for 83116 didn't work here, because when evaluating the body of the >> >>> > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do >> >>> > cache. >> >>> >> >>> > It seems that we may no longer rely on the constexpr call table when we >> >>> > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the >> >>> > value of an object. Might be too big a hammer again, but I couldn't think >> >>> > of how I could guard the caching of a constexpr call. >> >>> >> >>> > This doesn't manifest in C++14 because build_special_member_call in C++17 is >> >>> > more aggressive with copy elisions (as required by P0135 which changed how we >> >>> > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so >> >>> > expand_default_init calls maybe_constant_init, for which STRICT is false, so >> >>> > we avoid caching as per 83116. >> >>> >> >>> So it sounds like the problem is using cxx_constant_value for the >> >>> diagnostic when it has different semantics from the >> >>> maybe_constant_init that follows right after. I guess we want a >> >>> cxx_constant_init function that is a hybrid of the two. >> >> >> >> So like the following? Thanks, >> >> >> >> Bootstrapped/regtested on x86_64-linux, ok for trunk? >> >> >> >> 2018-02-04 Marek Polacek <polacek@redhat.com> >> >> >> >> PR c++/83692 >> >> * constexpr.c (cxx_constant_init): New function. >> >> * cp-tree.h (cxx_constant_init): Declare. >> >> * typeck2.c (store_init_value): Call cxx_constant_init instead of >> >> cxx_constant_value. >> >> >> >> +/* Like cxx_constant_value, but non-strict mode. */ >> >> + >> >> +tree >> >> +cxx_constant_init (tree t, tree decl) >> >> +{ >> >> + return cxx_eval_outermost_constant_expr (t, false, false, decl); >> >> +} >> > >> > Hmm, that doesn't do the TARGET_EXPR stripping that >> > maybe_constant_init does. I was thinking of a version of >> > maybe_constant_init that passes false to allow_non_constant. Probably >> > by making "maybe_constant_init" and cxx_constant_init both call the >> > current function with an additional parameter. And then the existing >> > call to maybe_constant_init can move under an 'else' to avoid >> > redundant constexpr evaluation. >> >> Want me to take this over? > > Sorry again for the delay. > > I tried to do what you suggested. There was one twist: it regressed > constexpr-nullptr-2.C, in particular we lost diagnostics for > > constexpr int* pj0 = &((S*)0)->j; // { dg-error "not a constant expression" } > constexpr int* pj1 = &((S*)nullptr)->j; // { dg-error "not a constant expression" } > > because when maybe_constant_init_1 saw a constant: > > 5142 else if (CONSTANT_CLASS_P (t)) > 5143 /* No evaluation needed. */; > > so it didn't call cxx_eval_outermost_constant_expr which is supposed to give > the error. I fixed it by adding "&& allow_non_constant" so now it gives the > proper diagnostics. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-02-23 Marek Polacek <polacek@redhat.com> > > PR c++/83692 > * constexpr.c (maybe_constant_init_1): New function. > (maybe_constant_init): Make it a wrapper around maybe_constant_init_1. > (cxx_constant_init): New function. > * cp-tree.h (cxx_constant_init): Declare. > * typeck2.c (store_init_value): Call cxx_constant_init instead of > cxx_constant_value. Move the maybe_constant_init call under an 'else'. > > * g++.dg/cpp1z/constexpr-83692.C: New test. > > diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c > index 47ff90cb055..26d0d099a05 100644 > --- gcc/cp/constexpr.c > +++ gcc/cp/constexpr.c > @@ -5123,8 +5123,8 @@ fold_non_dependent_expr (tree t) > /* Like maybe_constant_value, but returns a CONSTRUCTOR directly, rather > than wrapped in a TARGET_EXPR. */ > > -tree > -maybe_constant_init (tree t, tree decl) > +static tree > +maybe_constant_init_1 (tree t, tree decl, bool allow_non_constant) > { > if (!t) > return t; > @@ -5139,10 +5139,10 @@ maybe_constant_init (tree t, tree decl) > t = TARGET_EXPR_INITIAL (t); > if (!is_nondependent_static_init_expression (t)) > /* Don't try to evaluate it. */; > - else if (CONSTANT_CLASS_P (t)) > + else if (CONSTANT_CLASS_P (t) && allow_non_constant) > /* No evaluation needed. */; > else > - t = cxx_eval_outermost_constant_expr (t, true, false, decl); > + t = cxx_eval_outermost_constant_expr (t, allow_non_constant, false, decl); > if (TREE_CODE (t) == TARGET_EXPR) > { > tree init = TARGET_EXPR_INITIAL (t); > @@ -5152,6 +5152,22 @@ maybe_constant_init (tree t, tree decl) > return t; > } > > +/* Wrapper for maybe_constant_init_1 which permits non constants. */ > + > +tree > +maybe_constant_init (tree t, tree decl) > +{ > + return maybe_constant_init_1 (t, decl, true); > +} > + > +/* Wrapper for maybe_constant_init_1 which does not permit non constants. */ > + > +tree > +cxx_constant_init (tree t, tree decl) > +{ > + return maybe_constant_init_1 (t, decl, false); > +} > + > #if 0 > /* FIXME see ADDR_EXPR section in potential_constant_expression_1. */ > /* Return true if the object referred to by REF has automatic or thread > diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h > index 9038d677b2d..04c7b7ce3a9 100644 > --- gcc/cp/cp-tree.h > +++ gcc/cp/cp-tree.h > @@ -7411,6 +7411,7 @@ extern bool require_potential_constant_expression (tree); > extern bool require_constant_expression (tree); > extern bool require_potential_rvalue_constant_expression (tree); > extern tree cxx_constant_value (tree, tree = NULL_TREE); > +extern tree cxx_constant_init (tree, tree = NULL_TREE); > extern tree maybe_constant_value (tree, tree = NULL_TREE); > extern tree maybe_constant_init (tree, tree = NULL_TREE); > extern tree fold_non_dependent_expr (tree); > diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c > index 899d60e8535..153b46cca77 100644 > --- gcc/cp/typeck2.c > +++ gcc/cp/typeck2.c > @@ -830,9 +830,10 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags) > if (!require_constant_expression (value)) > value = error_mark_node; > else > - value = cxx_constant_value (value, decl); > + value = cxx_constant_init (value, decl); > } > - value = maybe_constant_init (value, decl); > + else > + value = maybe_constant_init (value, decl); > if (TREE_CODE (value) == CONSTRUCTOR && cp_has_mutable_p (type)) > /* Poison this CONSTRUCTOR so it can't be copied to another > constexpr variable. */ > diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C > index e69de29bb2d..f6b61eeab85 100644 > --- gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C > +++ gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C > @@ -0,0 +1,21 @@ > +// PR c++/83692 > +// { dg-options -std=c++17 } > + > +struct integer { > + constexpr int value() const { return m_value; } > + int m_value; > +}; > + > +struct outer { > + integer m_x{0}; > + constexpr outer() > + { > + if (m_x.value() != 0) > + throw 0; > + m_x.m_value = integer{1}.value(); > + if (m_x.value() != 1) > + throw 0; > + } > +}; > + > +constexpr outer o{}; > > Marek ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-24 1:55 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-25 21:58 C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) Marek Polacek 2018-01-25 22:37 ` Marek Polacek 2018-02-02 19:11 ` Jason Merrill 2018-02-05 13:38 ` Marek Polacek 2018-02-05 18:45 ` Jason Merrill 2018-02-16 21:10 ` Jason Merrill 2018-02-23 14:30 ` Marek Polacek 2018-02-24 1:55 ` 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).