| Message ID | 20200426224817.2656335-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Delegating constructor in constexpr init [PR94772] | expand |
On 4/26/20 6:48 PM, Patrick Palka wrote: > In the testcase below, the call to the target constructor foo{} from foo's > delegating constructor is encoded as the INIT_EXPR > > *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; > > During initialization of the variable 'bar', we prematurely set TREE_READONLY on > bar's CONSTRUCTOR in two places before the outer delegating constructor has > returned: first, at the end of cxx_eval_call_expression after evaluating the RHS > of the above INIT_EXPR, and second, at the end of cxx_eval_store_expression > after having finished evaluating the above INIT_EXPR. This then prevents the > rest of the outer delegating constructor from mutating 'bar'. > > This (hopefully minimally risky) patch makes cxx_eval_call_expression refrain > from setting TREE_READONLY when evaluating the target constructor of a > delegating constructor. It also makes cxx_eval_store_expression refrain from > setting TREE_READONLY when the object being initialized is "*this', on the basis > that it should be the responsibility of the routine that set 'this' in the first > place to set the object's TREE_READONLY appropriately. > > Passes 'make check-c++', does this look OK to commit after full > bootstrap/regtest? > > gcc/cp/ChangeLog: > > PR c++/94772 > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > evaluating the target constructor of a delegating constructor. > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > INIT_EXPR is '*this'. > > gcc/testsuite/ChangeLog: > > PR c++/94772 > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > --- > gcc/cp/constexpr.c | 29 +++++++++++++++---- > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ > 2 files changed, 45 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 6b3e514398b..a9ddd861195 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > /* In a constructor, it should be the first `this' argument. > At this point it has already been evaluated in the call > to cxx_bind_parameters_in_call. */ > - new_obj = TREE_VEC_ELT (new_call.bindings, 0); > - STRIP_NOPS (new_obj); > - if (TREE_CODE (new_obj) == ADDR_EXPR) > - new_obj = TREE_OPERAND (new_obj, 0); > + > + if (ctx->call && ctx->call->fundef > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) > + && (TREE_VEC_ELT (ctx->call->bindings, 0) > + == TREE_VEC_ELT (new_call.bindings, 0))) > + /* We're calling the target constructor of a delegating constructor, so > + there is no new object. */; > + else > + { > + new_obj = TREE_VEC_ELT (new_call.bindings, 0); > + STRIP_NOPS (new_obj); > + if (TREE_CODE (new_obj) == ADDR_EXPR) > + new_obj = TREE_OPERAND (new_obj, 0); > + } > } > > tree result = NULL_TREE; > @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > if (TREE_CODE (t) == INIT_EXPR > && TREE_CODE (*valp) == CONSTRUCTOR > && TYPE_READONLY (type)) > - TREE_READONLY (*valp) = true; > + { > + if (INDIRECT_REF_P (target) > + && (is_this_parameter > + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) > + /* We've just initialized '*this' (perhaps via the target constructor of > + a delegating constructor). Leave it up to the caller that set 'this' > + to set TREE_READONLY appropriately. */; Let's checking_assert that target and *this are same_type_ignoring_top_level_qualifiers_p. > + else > + TREE_READONLY (*valp) = true; > + } > > /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing > CONSTRUCTORs, if any. */ > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > new file mode 100644 > index 00000000000..266b62b852f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > @@ -0,0 +1,21 @@ > +// PR c++/94772 > +// { dg-do compile { target c++14 } } > + > +struct foo > +{ > + int x{}; > + > + constexpr foo() noexcept = default; > + > + constexpr foo(int a) : foo{} > + { x = -a; } > + > + constexpr foo(int a, int b) : foo{a} > + { x += a + b; } > +}; > + > +int main() > +{ > + constexpr foo bar{1, 2}; > + static_assert(bar.x == 2, ""); > +} >
On Mon, 27 Apr 2020, Jason Merrill wrote: > On 4/26/20 6:48 PM, Patrick Palka wrote: > > In the testcase below, the call to the target constructor foo{} from foo's > > delegating constructor is encoded as the INIT_EXPR > > > > *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; > > > > During initialization of the variable 'bar', we prematurely set > > TREE_READONLY on > > bar's CONSTRUCTOR in two places before the outer delegating constructor has > > returned: first, at the end of cxx_eval_call_expression after evaluating the > > RHS > > of the above INIT_EXPR, and second, at the end of cxx_eval_store_expression > > after having finished evaluating the above INIT_EXPR. This then prevents > > the > > rest of the outer delegating constructor from mutating 'bar'. > > > > This (hopefully minimally risky) patch makes cxx_eval_call_expression > > refrain > > from setting TREE_READONLY when evaluating the target constructor of a > > delegating constructor. It also makes cxx_eval_store_expression refrain > > from > > setting TREE_READONLY when the object being initialized is "*this', on the > > basis > > that it should be the responsibility of the routine that set 'this' in the > > first > > place to set the object's TREE_READONLY appropriately. > > > > Passes 'make check-c++', does this look OK to commit after full > > bootstrap/regtest? > > > > gcc/cp/ChangeLog: > > > > PR c++/94772 > > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > > evaluating the target constructor of a delegating constructor. > > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > > INIT_EXPR is '*this'. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/94772 > > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > > --- > > gcc/cp/constexpr.c | 29 +++++++++++++++---- > > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ > > 2 files changed, 45 insertions(+), 5 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > index 6b3e514398b..a9ddd861195 100644 > > --- a/gcc/cp/constexpr.c > > +++ b/gcc/cp/constexpr.c > > @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, > > tree t, > > /* In a constructor, it should be the first `this' argument. > > At this point it has already been evaluated in the call > > to cxx_bind_parameters_in_call. */ > > - new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > - STRIP_NOPS (new_obj); > > - if (TREE_CODE (new_obj) == ADDR_EXPR) > > - new_obj = TREE_OPERAND (new_obj, 0); > > + > > + if (ctx->call && ctx->call->fundef > > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) > > + && (TREE_VEC_ELT (ctx->call->bindings, 0) > > + == TREE_VEC_ELT (new_call.bindings, 0))) > > + /* We're calling the target constructor of a delegating constructor, > > so > > + there is no new object. */; > > + else > > + { > > + new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > + STRIP_NOPS (new_obj); > > + if (TREE_CODE (new_obj) == ADDR_EXPR) > > + new_obj = TREE_OPERAND (new_obj, 0); > > + } > > } > > tree result = NULL_TREE; > > @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, > > tree t, > > if (TREE_CODE (t) == INIT_EXPR > > && TREE_CODE (*valp) == CONSTRUCTOR > > && TYPE_READONLY (type)) > > - TREE_READONLY (*valp) = true; > > + { > > + if (INDIRECT_REF_P (target) > > + && (is_this_parameter > > + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) > > + /* We've just initialized '*this' (perhaps via the target constructor > > of > > + a delegating constructor). Leave it up to the caller that set > > 'this' > > + to set TREE_READONLY appropriately. */; > > Let's checking_assert that target and *this are > same_type_ignoring_top_level_qualifiers_p. Like this? Bootstrap and regtest in progress. -- >8 -- gcc/cp/ChangeLog: PR c++/94772 * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're evaluating the target constructor of a delegating constructor. (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the INIT_EXPR is '*this'. gcc/testsuite/ChangeLog: PR c++/94772 * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. --- gcc/cp/constexpr.c | 31 ++++++++++++++++--- .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 6b3e514398b..c7923897e23 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, /* In a constructor, it should be the first `this' argument. At this point it has already been evaluated in the call to cxx_bind_parameters_in_call. */ - new_obj = TREE_VEC_ELT (new_call.bindings, 0); - STRIP_NOPS (new_obj); - if (TREE_CODE (new_obj) == ADDR_EXPR) - new_obj = TREE_OPERAND (new_obj, 0); + + if (ctx->call && ctx->call->fundef + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) + && (TREE_VEC_ELT (ctx->call->bindings, 0) + == TREE_VEC_ELT (new_call.bindings, 0))) + /* We're calling the target constructor of a delegating constructor, so + there is no new object. */; + else + { + new_obj = TREE_VEC_ELT (new_call.bindings, 0); + STRIP_NOPS (new_obj); + if (TREE_CODE (new_obj) == ADDR_EXPR) + new_obj = TREE_OPERAND (new_obj, 0); + } } tree result = NULL_TREE; @@ -4950,7 +4960,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, if (TREE_CODE (t) == INIT_EXPR && TREE_CODE (*valp) == CONSTRUCTOR && TYPE_READONLY (type)) - TREE_READONLY (*valp) = true; + { + if (INDIRECT_REF_P (target) + && (is_this_parameter + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) + /* We've just initialized '*this' (perhaps via the target constructor of + a delegating constructor). Leave it up to the caller that set 'this' + to set TREE_READONLY appropriately. */ + gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (target), type)); + else + TREE_READONLY (*valp) = true; + } /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing CONSTRUCTORs, if any. */ diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C new file mode 100644 index 00000000000..266b62b852f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C @@ -0,0 +1,21 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +struct foo +{ + int x{}; + + constexpr foo() noexcept = default; + + constexpr foo(int a) : foo{} + { x = -a; } + + constexpr foo(int a, int b) : foo{a} + { x += a + b; } +}; + +int main() +{ + constexpr foo bar{1, 2}; + static_assert(bar.x == 2, ""); +}
On Mon, 27 Apr 2020, Patrick Palka wrote: > On Mon, 27 Apr 2020, Jason Merrill wrote: > > > On 4/26/20 6:48 PM, Patrick Palka wrote: > > > In the testcase below, the call to the target constructor foo{} from foo's > > > delegating constructor is encoded as the INIT_EXPR > > > > > > *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; > > > > > > During initialization of the variable 'bar', we prematurely set > > > TREE_READONLY on > > > bar's CONSTRUCTOR in two places before the outer delegating constructor has > > > returned: first, at the end of cxx_eval_call_expression after evaluating the > > > RHS > > > of the above INIT_EXPR, and second, at the end of cxx_eval_store_expression > > > after having finished evaluating the above INIT_EXPR. This then prevents > > > the > > > rest of the outer delegating constructor from mutating 'bar'. > > > > > > This (hopefully minimally risky) patch makes cxx_eval_call_expression > > > refrain > > > from setting TREE_READONLY when evaluating the target constructor of a > > > delegating constructor. It also makes cxx_eval_store_expression refrain > > > from > > > setting TREE_READONLY when the object being initialized is "*this', on the > > > basis > > > that it should be the responsibility of the routine that set 'this' in the > > > first > > > place to set the object's TREE_READONLY appropriately. > > > > > > Passes 'make check-c++', does this look OK to commit after full > > > bootstrap/regtest? > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/94772 > > > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > > > evaluating the target constructor of a delegating constructor. > > > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > > > INIT_EXPR is '*this'. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/94772 > > > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > > > --- > > > gcc/cp/constexpr.c | 29 +++++++++++++++---- > > > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ > > > 2 files changed, 45 insertions(+), 5 deletions(-) > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > > index 6b3e514398b..a9ddd861195 100644 > > > --- a/gcc/cp/constexpr.c > > > +++ b/gcc/cp/constexpr.c > > > @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, > > > tree t, > > > /* In a constructor, it should be the first `this' argument. > > > At this point it has already been evaluated in the call > > > to cxx_bind_parameters_in_call. */ > > > - new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > > - STRIP_NOPS (new_obj); > > > - if (TREE_CODE (new_obj) == ADDR_EXPR) > > > - new_obj = TREE_OPERAND (new_obj, 0); > > > + > > > + if (ctx->call && ctx->call->fundef > > > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) > > > + && (TREE_VEC_ELT (ctx->call->bindings, 0) > > > + == TREE_VEC_ELT (new_call.bindings, 0))) > > > + /* We're calling the target constructor of a delegating constructor, > > > so > > > + there is no new object. */; > > > + else > > > + { > > > + new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > > + STRIP_NOPS (new_obj); > > > + if (TREE_CODE (new_obj) == ADDR_EXPR) > > > + new_obj = TREE_OPERAND (new_obj, 0); > > > + } > > > } > > > tree result = NULL_TREE; > > > @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, > > > tree t, > > > if (TREE_CODE (t) == INIT_EXPR > > > && TREE_CODE (*valp) == CONSTRUCTOR > > > && TYPE_READONLY (type)) > > > - TREE_READONLY (*valp) = true; > > > + { > > > + if (INDIRECT_REF_P (target) > > > + && (is_this_parameter > > > + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) > > > + /* We've just initialized '*this' (perhaps via the target constructor > > > of > > > + a delegating constructor). Leave it up to the caller that set > > > 'this' > > > + to set TREE_READONLY appropriately. */; > > > > Let's checking_assert that target and *this are > > same_type_ignoring_top_level_qualifiers_p. > > Like this? Bootstrap and regtest in progress. > > -- >8 -- > > gcc/cp/ChangeLog: > > PR c++/94772 > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > evaluating the target constructor of a delegating constructor. > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > INIT_EXPR is '*this'. > > gcc/testsuite/ChangeLog: > > PR c++/94772 > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > --- > gcc/cp/constexpr.c | 31 ++++++++++++++++--- > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++ > 2 files changed, 47 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 6b3e514398b..c7923897e23 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > /* In a constructor, it should be the first `this' argument. > At this point it has already been evaluated in the call > to cxx_bind_parameters_in_call. */ > - new_obj = TREE_VEC_ELT (new_call.bindings, 0); > - STRIP_NOPS (new_obj); > - if (TREE_CODE (new_obj) == ADDR_EXPR) > - new_obj = TREE_OPERAND (new_obj, 0); > + > + if (ctx->call && ctx->call->fundef > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) > + && (TREE_VEC_ELT (ctx->call->bindings, 0) > + == TREE_VEC_ELT (new_call.bindings, 0))) > + /* We're calling the target constructor of a delegating constructor, so > + there is no new object. */; Further experimentation revealed that testing the 'this' arguments for pointer equality here is too strict because the target constructor could belong to a base class, in which case its 'this' argument would be (base *)&bar instead of (foo *)&bar, as in the new testcase below. Fixed by comparing the objects pointed to by the 'this' arguments more directly. Bootstrap and regtest is in progress.. -- >8 -- gcc/cp/ChangeLog: PR c++/94772 * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're evaluating the target constructor of a delegating constructor. (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the INIT_EXPR is '*this'. gcc/testsuite/ChangeLog: PR c++/94772 * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. * g++.dg/cpp1y/constexpr-tracking-const24.C: New test. --- gcc/cp/constexpr.c | 26 ++++++++++++++++++- .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++++ .../g++.dg/cpp1y/constexpr-tracking-const24.C | 26 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 6b3e514398b..5d9b10c63d4 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2371,6 +2371,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, STRIP_NOPS (new_obj); if (TREE_CODE (new_obj) == ADDR_EXPR) new_obj = TREE_OPERAND (new_obj, 0); + + if (ctx->call && ctx->call->fundef + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) + { + tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); + STRIP_NOPS (cur_obj); + if (TREE_CODE (cur_obj) == ADDR_EXPR) + cur_obj = TREE_OPERAND (cur_obj, 0); + if (new_obj == cur_obj) + /* We're calling the target constructor of a delegating constructor, + so there is no new object. */ + new_obj = NULL_TREE; + } } tree result = NULL_TREE; @@ -4950,7 +4963,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, if (TREE_CODE (t) == INIT_EXPR && TREE_CODE (*valp) == CONSTRUCTOR && TYPE_READONLY (type)) - TREE_READONLY (*valp) = true; + { + if (INDIRECT_REF_P (target) + && (is_this_parameter + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) + /* We've just initialized '*this' (perhaps via the target constructor of + a delegating constructor). Leave it up to the caller that set 'this' + to set TREE_READONLY appropriately. */ + gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (target), type)); + else + TREE_READONLY (*valp) = true; + } /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing CONSTRUCTORs, if any. */ diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C new file mode 100644 index 00000000000..c6643c78a6f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C @@ -0,0 +1,21 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +struct foo +{ + int x{}; + + constexpr foo() noexcept = default; + + constexpr foo(int a) : foo{} + { x = -a; } + + constexpr foo(int a, int b) : foo{a} + { x += a + b; } +}; + +int main() +{ + constexpr foo bar{1, 2}; + static_assert(bar.x == 2, ""); +} diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C new file mode 100644 index 00000000000..2c923f69cf4 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C @@ -0,0 +1,26 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +struct base +{ + base() = default; + + constexpr base(int) : base{} { } +}; + +struct foo : base +{ + int x{}; + + constexpr foo(int a) : base{a} + { x = -a; } + + constexpr foo(int a, int b) : foo{a} + { x += a + b; } +}; + +int main() +{ + constexpr foo bar{1, 2}; + static_assert(bar.x == 2, ""); +}
On 4/27/20 10:45 AM, Patrick Palka wrote: > On Mon, 27 Apr 2020, Patrick Palka wrote: > >> On Mon, 27 Apr 2020, Jason Merrill wrote: >> >>> On 4/26/20 6:48 PM, Patrick Palka wrote: >>>> In the testcase below, the call to the target constructor foo{} from foo's >>>> delegating constructor is encoded as the INIT_EXPR >>>> >>>> *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; >>>> >>>> During initialization of the variable 'bar', we prematurely set >>>> TREE_READONLY on >>>> bar's CONSTRUCTOR in two places before the outer delegating constructor has >>>> returned: first, at the end of cxx_eval_call_expression after evaluating the >>>> RHS >>>> of the above INIT_EXPR, and second, at the end of cxx_eval_store_expression >>>> after having finished evaluating the above INIT_EXPR. This then prevents >>>> the >>>> rest of the outer delegating constructor from mutating 'bar'. >>>> >>>> This (hopefully minimally risky) patch makes cxx_eval_call_expression >>>> refrain >>>> from setting TREE_READONLY when evaluating the target constructor of a >>>> delegating constructor. It also makes cxx_eval_store_expression refrain >>>> from >>>> setting TREE_READONLY when the object being initialized is "*this', on the >>>> basis >>>> that it should be the responsibility of the routine that set 'this' in the >>>> first >>>> place to set the object's TREE_READONLY appropriately. >>>> >>>> Passes 'make check-c++', does this look OK to commit after full >>>> bootstrap/regtest? >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/94772 >>>> * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're >>>> evaluating the target constructor of a delegating constructor. >>>> (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the >>>> INIT_EXPR is '*this'. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/94772 >>>> * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. >>>> --- >>>> gcc/cp/constexpr.c | 29 +++++++++++++++---- >>>> .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ >>>> 2 files changed, 45 insertions(+), 5 deletions(-) >>>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C >>>> >>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>>> index 6b3e514398b..a9ddd861195 100644 >>>> --- a/gcc/cp/constexpr.c >>>> +++ b/gcc/cp/constexpr.c >>>> @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, >>>> tree t, >>>> /* In a constructor, it should be the first `this' argument. >>>> At this point it has already been evaluated in the call >>>> to cxx_bind_parameters_in_call. */ >>>> - new_obj = TREE_VEC_ELT (new_call.bindings, 0); >>>> - STRIP_NOPS (new_obj); >>>> - if (TREE_CODE (new_obj) == ADDR_EXPR) >>>> - new_obj = TREE_OPERAND (new_obj, 0); >>>> + >>>> + if (ctx->call && ctx->call->fundef >>>> + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) >>>> + && (TREE_VEC_ELT (ctx->call->bindings, 0) >>>> + == TREE_VEC_ELT (new_call.bindings, 0))) >>>> + /* We're calling the target constructor of a delegating constructor, >>>> so >>>> + there is no new object. */; >>>> + else >>>> + { >>>> + new_obj = TREE_VEC_ELT (new_call.bindings, 0); >>>> + STRIP_NOPS (new_obj); >>>> + if (TREE_CODE (new_obj) == ADDR_EXPR) >>>> + new_obj = TREE_OPERAND (new_obj, 0); >>>> + } >>>> } >>>> tree result = NULL_TREE; >>>> @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, >>>> tree t, >>>> if (TREE_CODE (t) == INIT_EXPR >>>> && TREE_CODE (*valp) == CONSTRUCTOR >>>> && TYPE_READONLY (type)) >>>> - TREE_READONLY (*valp) = true; >>>> + { >>>> + if (INDIRECT_REF_P (target) >>>> + && (is_this_parameter >>>> + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) >>>> + /* We've just initialized '*this' (perhaps via the target constructor >>>> of >>>> + a delegating constructor). Leave it up to the caller that set >>>> 'this' >>>> + to set TREE_READONLY appropriately. */; >>> >>> Let's checking_assert that target and *this are >>> same_type_ignoring_top_level_qualifiers_p. >> >> Like this? Bootstrap and regtest in progress. >> >> -- >8 -- >> >> gcc/cp/ChangeLog: >> >> PR c++/94772 >> * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're >> evaluating the target constructor of a delegating constructor. >> (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the >> INIT_EXPR is '*this'. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/94772 >> * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. >> --- >> gcc/cp/constexpr.c | 31 ++++++++++++++++--- >> .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++ >> 2 files changed, 47 insertions(+), 5 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C >> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >> index 6b3e514398b..c7923897e23 100644 >> --- a/gcc/cp/constexpr.c >> +++ b/gcc/cp/constexpr.c >> @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, >> /* In a constructor, it should be the first `this' argument. >> At this point it has already been evaluated in the call >> to cxx_bind_parameters_in_call. */ >> - new_obj = TREE_VEC_ELT (new_call.bindings, 0); >> - STRIP_NOPS (new_obj); >> - if (TREE_CODE (new_obj) == ADDR_EXPR) >> - new_obj = TREE_OPERAND (new_obj, 0); >> + >> + if (ctx->call && ctx->call->fundef >> + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) >> + && (TREE_VEC_ELT (ctx->call->bindings, 0) >> + == TREE_VEC_ELT (new_call.bindings, 0))) >> + /* We're calling the target constructor of a delegating constructor, so >> + there is no new object. */; > > Further experimentation revealed that testing the 'this' arguments for > pointer equality here is too strict because the target constructor could > belong to a base class, in which case its 'this' argument would be > (base *)&bar instead of (foo *)&bar, as in the new testcase below. Well, in that case it's not a delegating constructor, it's normal base construction. But it's certainly true that we don't want to treat a base subobject as a whole new object. > Fixed by comparing the objects pointed to by the 'this' arguments more > directly. Bootstrap and regtest is in progress.. > > -- >8 -- > > gcc/cp/ChangeLog: > > PR c++/94772 > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > evaluating the target constructor of a delegating constructor. > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > INIT_EXPR is '*this'. > > gcc/testsuite/ChangeLog: > > PR c++/94772 > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > * g++.dg/cpp1y/constexpr-tracking-const24.C: New test. > --- > gcc/cp/constexpr.c | 26 ++++++++++++++++++- > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++++ > .../g++.dg/cpp1y/constexpr-tracking-const24.C | 26 +++++++++++++++++++ > 3 files changed, 72 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 6b3e514398b..5d9b10c63d4 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -2371,6 +2371,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > STRIP_NOPS (new_obj); > if (TREE_CODE (new_obj) == ADDR_EXPR) > new_obj = TREE_OPERAND (new_obj, 0); > + > + if (ctx->call && ctx->call->fundef > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) > + { > + tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); > + STRIP_NOPS (cur_obj); > + if (TREE_CODE (cur_obj) == ADDR_EXPR) > + cur_obj = TREE_OPERAND (cur_obj, 0); > + if (new_obj == cur_obj) > + /* We're calling the target constructor of a delegating constructor, > + so there is no new object. */ ...so you'll want to update this comment. What happens if we get to 'base' by COMPONENT_REF rather than NOP_EXPR? Jason
On Mon, 27 Apr 2020, Jason Merrill wrote: > On 4/27/20 10:45 AM, Patrick Palka wrote: > > On Mon, 27 Apr 2020, Patrick Palka wrote: > > > > > On Mon, 27 Apr 2020, Jason Merrill wrote: > > > > > > > On 4/26/20 6:48 PM, Patrick Palka wrote: > > > > > In the testcase below, the call to the target constructor foo{} from > > > > > foo's > > > > > delegating constructor is encoded as the INIT_EXPR > > > > > > > > > > *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; > > > > > > > > > > During initialization of the variable 'bar', we prematurely set > > > > > TREE_READONLY on > > > > > bar's CONSTRUCTOR in two places before the outer delegating > > > > > constructor has > > > > > returned: first, at the end of cxx_eval_call_expression after > > > > > evaluating the > > > > > RHS > > > > > of the above INIT_EXPR, and second, at the end of > > > > > cxx_eval_store_expression > > > > > after having finished evaluating the above INIT_EXPR. This then > > > > > prevents > > > > > the > > > > > rest of the outer delegating constructor from mutating 'bar'. > > > > > > > > > > This (hopefully minimally risky) patch makes cxx_eval_call_expression > > > > > refrain > > > > > from setting TREE_READONLY when evaluating the target constructor of a > > > > > delegating constructor. It also makes cxx_eval_store_expression > > > > > refrain > > > > > from > > > > > setting TREE_READONLY when the object being initialized is "*this', on > > > > > the > > > > > basis > > > > > that it should be the responsibility of the routine that set 'this' in > > > > > the > > > > > first > > > > > place to set the object's TREE_READONLY appropriately. > > > > > > > > > > Passes 'make check-c++', does this look OK to commit after full > > > > > bootstrap/regtest? > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > PR c++/94772 > > > > > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if > > > > > we're > > > > > evaluating the target constructor of a delegating constructor. > > > > > (cxx_eval_store_expression): Don't set TREE_READONLY if the > > > > > LHS of the > > > > > INIT_EXPR is '*this'. > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > PR c++/94772 > > > > > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > > > > > --- > > > > > gcc/cp/constexpr.c | 29 > > > > > +++++++++++++++---- > > > > > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ > > > > > 2 files changed, 45 insertions(+), 5 deletions(-) > > > > > create mode 100644 > > > > > gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > > > > > > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > > > > index 6b3e514398b..a9ddd861195 100644 > > > > > --- a/gcc/cp/constexpr.c > > > > > +++ b/gcc/cp/constexpr.c > > > > > @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx > > > > > *ctx, > > > > > tree t, > > > > > /* In a constructor, it should be the first `this' argument. > > > > > At this point it has already been evaluated in the call > > > > > to cxx_bind_parameters_in_call. */ > > > > > - new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > > > > - STRIP_NOPS (new_obj); > > > > > - if (TREE_CODE (new_obj) == ADDR_EXPR) > > > > > - new_obj = TREE_OPERAND (new_obj, 0); > > > > > + > > > > > + if (ctx->call && ctx->call->fundef > > > > > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) > > > > > + && (TREE_VEC_ELT (ctx->call->bindings, 0) > > > > > + == TREE_VEC_ELT (new_call.bindings, 0))) > > > > > + /* We're calling the target constructor of a delegating > > > > > constructor, > > > > > so > > > > > + there is no new object. */; > > > > > + else > > > > > + { > > > > > + new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > > > > + STRIP_NOPS (new_obj); > > > > > + if (TREE_CODE (new_obj) == ADDR_EXPR) > > > > > + new_obj = TREE_OPERAND (new_obj, 0); > > > > > + } > > > > > } > > > > > tree result = NULL_TREE; > > > > > @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx > > > > > *ctx, > > > > > tree t, > > > > > if (TREE_CODE (t) == INIT_EXPR > > > > > && TREE_CODE (*valp) == CONSTRUCTOR > > > > > && TYPE_READONLY (type)) > > > > > - TREE_READONLY (*valp) = true; > > > > > + { > > > > > + if (INDIRECT_REF_P (target) > > > > > + && (is_this_parameter > > > > > + (tree_strip_nop_conversions (TREE_OPERAND (target, > > > > > 0))))) > > > > > + /* We've just initialized '*this' (perhaps via the target > > > > > constructor > > > > > of > > > > > + a delegating constructor). Leave it up to the caller that > > > > > set > > > > > 'this' > > > > > + to set TREE_READONLY appropriately. */; > > > > > > > > Let's checking_assert that target and *this are > > > > same_type_ignoring_top_level_qualifiers_p. > > > > > > Like this? Bootstrap and regtest in progress. > > > > > > -- >8 -- > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/94772 > > > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > > > evaluating the target constructor of a delegating constructor. > > > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > > > INIT_EXPR is '*this'. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/94772 > > > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > > > --- > > > gcc/cp/constexpr.c | 31 ++++++++++++++++--- > > > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++ > > > 2 files changed, 47 insertions(+), 5 deletions(-) > > > create mode 100644 > > > gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > > index 6b3e514398b..c7923897e23 100644 > > > --- a/gcc/cp/constexpr.c > > > +++ b/gcc/cp/constexpr.c > > > @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx > > > *ctx, tree t, > > > /* In a constructor, it should be the first `this' argument. > > > At this point it has already been evaluated in the call > > > to cxx_bind_parameters_in_call. */ > > > - new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > > - STRIP_NOPS (new_obj); > > > - if (TREE_CODE (new_obj) == ADDR_EXPR) > > > - new_obj = TREE_OPERAND (new_obj, 0); > > > + > > > + if (ctx->call && ctx->call->fundef > > > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) > > > + && (TREE_VEC_ELT (ctx->call->bindings, 0) > > > + == TREE_VEC_ELT (new_call.bindings, 0))) > > > + /* We're calling the target constructor of a delegating constructor, > > > so > > > + there is no new object. */; > > > > Further experimentation revealed that testing the 'this' arguments for > > pointer equality here is too strict because the target constructor could > > belong to a base class, in which case its 'this' argument would be > > (base *)&bar instead of (foo *)&bar, as in the new testcase below. > > Well, in that case it's not a delegating constructor, it's normal base > construction. But it's certainly true that we don't want to treat a base > subobject as a whole new object. Ah okay, noted. > > > Fixed by comparing the objects pointed to by the 'this' arguments more > > directly. Bootstrap and regtest is in progress.. > > > > -- >8 -- > > > > gcc/cp/ChangeLog: > > > > PR c++/94772 > > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > > evaluating the target constructor of a delegating constructor. > > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > > INIT_EXPR is '*this'. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/94772 > > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > > * g++.dg/cpp1y/constexpr-tracking-const24.C: New test. > > --- > > gcc/cp/constexpr.c | 26 ++++++++++++++++++- > > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++++ > > .../g++.dg/cpp1y/constexpr-tracking-const24.C | 26 +++++++++++++++++++ > > 3 files changed, 72 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C > > > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > > index 6b3e514398b..5d9b10c63d4 100644 > > --- a/gcc/cp/constexpr.c > > +++ b/gcc/cp/constexpr.c > > @@ -2371,6 +2371,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, > > tree t, > > STRIP_NOPS (new_obj); > > if (TREE_CODE (new_obj) == ADDR_EXPR) > > new_obj = TREE_OPERAND (new_obj, 0); > > + > > + if (ctx->call && ctx->call->fundef > > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) > > + { > > + tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); > > + STRIP_NOPS (cur_obj); > > + if (TREE_CODE (cur_obj) == ADDR_EXPR) > > + cur_obj = TREE_OPERAND (cur_obj, 0); > > + if (new_obj == cur_obj) > > + /* We're calling the target constructor of a delegating > > constructor, > > + so there is no new object. */ > > ...so you'll want to update this comment. Done. > > What happens if we get to 'base' by COMPONENT_REF rather than NOP_EXPR? It looks like in this case the TREE_TYPE of the evaluated COMPONENT_REF is non-const regardless of the constness of the parent object, because in cxx_eval_component_reference the built COMPONENT_REF inherits the constness of the original tree ((struct foo *) this)->D.2414; that is passed as the 'this' argument to the base class constructor (and is evaluated by cxx_bind_parameters_in_call). So because its TREE_TYPE is non-const, we don't consider setting TREE_READONLY on the CONSTRUCTOR of the parent object at the end of cxx_eval_call_expression, and likewise in cxx_eval_store_expression. So when constructing a base subobject through a COMPONENT_REF it luckily seems we don't have this constness problem with or without this patch. Here's the updated patch, with the comment updated and a new test added. -- >8 -- gcc/cp/ChangeLog: PR c++/94772 * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're evaluating the target constructor of a delegating constructor. (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the INIT_EXPR is '*this'. gcc/testsuite/ChangeLog: PR c++/94772 * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. * g++.dg/cpp1y/constexpr-tracking-const24.C: New test. * g++.dg/cpp1y/constexpr-tracking-const25.C: New test. --- gcc/cp/constexpr.c | 28 +++++++- .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++ .../g++.dg/cpp1y/constexpr-tracking-const24.C | 26 ++++++++ .../g++.dg/cpp1y/constexpr-tracking-const25.C | 66 +++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 6b3e514398b..637cb746576 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2371,6 +2371,21 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, STRIP_NOPS (new_obj); if (TREE_CODE (new_obj) == ADDR_EXPR) new_obj = TREE_OPERAND (new_obj, 0); + + if (ctx->call && ctx->call->fundef + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) + { + tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); + STRIP_NOPS (cur_obj); + if (TREE_CODE (cur_obj) == ADDR_EXPR) + cur_obj = TREE_OPERAND (cur_obj, 0); + if (new_obj == cur_obj) + /* We're calling the target constructor of a delegating + constructor, or accessing a base subobject through a + NOP_EXPR as part of a call to a base constructor, so + there is no new (sub)object. */ + new_obj = NULL_TREE; + } } tree result = NULL_TREE; @@ -4950,7 +4965,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, if (TREE_CODE (t) == INIT_EXPR && TREE_CODE (*valp) == CONSTRUCTOR && TYPE_READONLY (type)) - TREE_READONLY (*valp) = true; + { + if (INDIRECT_REF_P (target) + && (is_this_parameter + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) + /* We've just initialized '*this' (perhaps via the target + constructor of a delegating constructor). Leave it up to the + caller that set 'this' to set TREE_READONLY appropriately. */ + gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (target), type)); + else + TREE_READONLY (*valp) = true; + } /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing CONSTRUCTORs, if any. */ diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C new file mode 100644 index 00000000000..c6643c78a6f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C @@ -0,0 +1,21 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +struct foo +{ + int x{}; + + constexpr foo() noexcept = default; + + constexpr foo(int a) : foo{} + { x = -a; } + + constexpr foo(int a, int b) : foo{a} + { x += a + b; } +}; + +int main() +{ + constexpr foo bar{1, 2}; + static_assert(bar.x == 2, ""); +} diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C new file mode 100644 index 00000000000..2c923f69cf4 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C @@ -0,0 +1,26 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +struct base +{ + base() = default; + + constexpr base(int) : base{} { } +}; + +struct foo : base +{ + int x{}; + + constexpr foo(int a) : base{a} + { x = -a; } + + constexpr foo(int a, int b) : foo{a} + { x += a + b; } +}; + +int main() +{ + constexpr foo bar{1, 2}; + static_assert(bar.x == 2, ""); +} diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C new file mode 100644 index 00000000000..662a6f93642 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C @@ -0,0 +1,66 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +template<int> +struct base +{ + int y{}; + + base() = default; + + constexpr base(int a) : base{} + { y = a; } +}; + +struct foo : base<1>, base<2> +{ + int x{}; + + constexpr foo() : base<2>{} + { + x = x; + ++base<1>::y; + ++base<2>::y; + } + + constexpr foo(int a) : base<2>{a} + { + x = -base<2>::y; + ++base<1>::y; + ++base<2>::y; + } + + constexpr foo(int a, int b) : foo{a} + { + x += a + b; + ++base<1>::y; + ++base<2>::y; + } + + constexpr foo(int a, int b, int c) : base<1>{a} + { + x += a + b + c; + ++base<1>::y; + ++base<2>::y; + } +}; + +#define SA(X) static_assert(X, #X) + +int main() +{ + constexpr foo bar1{1, 2}; + SA( bar1.x == 2 ); + SA( bar1.base<1>::y == 2 ); + SA( bar1.base<2>::y == 3 ); + + constexpr foo bar2{1, 2, 3}; + SA( bar2.x == 6 ); + SA( bar2.base<1>::y == 2 ); + SA( bar2.base<2>::y == 1 ); + + constexpr foo bar3{}; + SA( bar3.x == 0 ); + SA( bar3.base<1>::y == 1 ); + SA( bar3.base<2>::y == 1 ); +}
On 4/27/20 3:33 PM, Patrick Palka wrote: > On Mon, 27 Apr 2020, Jason Merrill wrote: > >> On 4/27/20 10:45 AM, Patrick Palka wrote: >>> On Mon, 27 Apr 2020, Patrick Palka wrote: >>> >>>> On Mon, 27 Apr 2020, Jason Merrill wrote: >>>> >>>>> On 4/26/20 6:48 PM, Patrick Palka wrote: >>>>>> In the testcase below, the call to the target constructor foo{} from >>>>>> foo's >>>>>> delegating constructor is encoded as the INIT_EXPR >>>>>> >>>>>> *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; >>>>>> >>>>>> During initialization of the variable 'bar', we prematurely set >>>>>> TREE_READONLY on >>>>>> bar's CONSTRUCTOR in two places before the outer delegating >>>>>> constructor has >>>>>> returned: first, at the end of cxx_eval_call_expression after >>>>>> evaluating the >>>>>> RHS >>>>>> of the above INIT_EXPR, and second, at the end of >>>>>> cxx_eval_store_expression >>>>>> after having finished evaluating the above INIT_EXPR. This then >>>>>> prevents >>>>>> the >>>>>> rest of the outer delegating constructor from mutating 'bar'. >>>>>> >>>>>> This (hopefully minimally risky) patch makes cxx_eval_call_expression >>>>>> refrain >>>>>> from setting TREE_READONLY when evaluating the target constructor of a >>>>>> delegating constructor. It also makes cxx_eval_store_expression >>>>>> refrain >>>>>> from >>>>>> setting TREE_READONLY when the object being initialized is "*this', on >>>>>> the >>>>>> basis >>>>>> that it should be the responsibility of the routine that set 'this' in >>>>>> the >>>>>> first >>>>>> place to set the object's TREE_READONLY appropriately. >>>>>> >>>>>> Passes 'make check-c++', does this look OK to commit after full >>>>>> bootstrap/regtest? >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> PR c++/94772 >>>>>> * constexpr.c (cxx_eval_call_expression): Don't set new_obj if >>>>>> we're >>>>>> evaluating the target constructor of a delegating constructor. >>>>>> (cxx_eval_store_expression): Don't set TREE_READONLY if the >>>>>> LHS of the >>>>>> INIT_EXPR is '*this'. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> PR c++/94772 >>>>>> * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. >>>>>> --- >>>>>> gcc/cp/constexpr.c | 29 >>>>>> +++++++++++++++---- >>>>>> .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ >>>>>> 2 files changed, 45 insertions(+), 5 deletions(-) >>>>>> create mode 100644 >>>>>> gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C >>>>>> >>>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>>>>> index 6b3e514398b..a9ddd861195 100644 >>>>>> --- a/gcc/cp/constexpr.c >>>>>> +++ b/gcc/cp/constexpr.c >>>>>> @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx >>>>>> *ctx, >>>>>> tree t, >>>>>> /* In a constructor, it should be the first `this' argument. >>>>>> At this point it has already been evaluated in the call >>>>>> to cxx_bind_parameters_in_call. */ >>>>>> - new_obj = TREE_VEC_ELT (new_call.bindings, 0); >>>>>> - STRIP_NOPS (new_obj); >>>>>> - if (TREE_CODE (new_obj) == ADDR_EXPR) >>>>>> - new_obj = TREE_OPERAND (new_obj, 0); >>>>>> + >>>>>> + if (ctx->call && ctx->call->fundef >>>>>> + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) >>>>>> + && (TREE_VEC_ELT (ctx->call->bindings, 0) >>>>>> + == TREE_VEC_ELT (new_call.bindings, 0))) >>>>>> + /* We're calling the target constructor of a delegating >>>>>> constructor, >>>>>> so >>>>>> + there is no new object. */; >>>>>> + else >>>>>> + { >>>>>> + new_obj = TREE_VEC_ELT (new_call.bindings, 0); >>>>>> + STRIP_NOPS (new_obj); >>>>>> + if (TREE_CODE (new_obj) == ADDR_EXPR) >>>>>> + new_obj = TREE_OPERAND (new_obj, 0); >>>>>> + } >>>>>> } >>>>>> tree result = NULL_TREE; >>>>>> @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx >>>>>> *ctx, >>>>>> tree t, >>>>>> if (TREE_CODE (t) == INIT_EXPR >>>>>> && TREE_CODE (*valp) == CONSTRUCTOR >>>>>> && TYPE_READONLY (type)) >>>>>> - TREE_READONLY (*valp) = true; >>>>>> + { >>>>>> + if (INDIRECT_REF_P (target) >>>>>> + && (is_this_parameter >>>>>> + (tree_strip_nop_conversions (TREE_OPERAND (target, >>>>>> 0))))) >>>>>> + /* We've just initialized '*this' (perhaps via the target >>>>>> constructor >>>>>> of >>>>>> + a delegating constructor). Leave it up to the caller that >>>>>> set >>>>>> 'this' >>>>>> + to set TREE_READONLY appropriately. */; >>>>> >>>>> Let's checking_assert that target and *this are >>>>> same_type_ignoring_top_level_qualifiers_p. >>>> >>>> Like this? Bootstrap and regtest in progress. >>>> >>>> -- >8 -- >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/94772 >>>> * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're >>>> evaluating the target constructor of a delegating constructor. >>>> (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the >>>> INIT_EXPR is '*this'. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/94772 >>>> * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. >>>> --- >>>> gcc/cp/constexpr.c | 31 ++++++++++++++++--- >>>> .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++ >>>> 2 files changed, 47 insertions(+), 5 deletions(-) >>>> create mode 100644 >>>> gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C >>>> >>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>>> index 6b3e514398b..c7923897e23 100644 >>>> --- a/gcc/cp/constexpr.c >>>> +++ b/gcc/cp/constexpr.c >>>> @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx >>>> *ctx, tree t, >>>> /* In a constructor, it should be the first `this' argument. >>>> At this point it has already been evaluated in the call >>>> to cxx_bind_parameters_in_call. */ >>>> - new_obj = TREE_VEC_ELT (new_call.bindings, 0); >>>> - STRIP_NOPS (new_obj); >>>> - if (TREE_CODE (new_obj) == ADDR_EXPR) >>>> - new_obj = TREE_OPERAND (new_obj, 0); >>>> + >>>> + if (ctx->call && ctx->call->fundef >>>> + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) >>>> + && (TREE_VEC_ELT (ctx->call->bindings, 0) >>>> + == TREE_VEC_ELT (new_call.bindings, 0))) >>>> + /* We're calling the target constructor of a delegating constructor, >>>> so >>>> + there is no new object. */; >>> >>> Further experimentation revealed that testing the 'this' arguments for >>> pointer equality here is too strict because the target constructor could >>> belong to a base class, in which case its 'this' argument would be >>> (base *)&bar instead of (foo *)&bar, as in the new testcase below. >> >> Well, in that case it's not a delegating constructor, it's normal base >> construction. But it's certainly true that we don't want to treat a base >> subobject as a whole new object. > > Ah okay, noted. > >> >>> Fixed by comparing the objects pointed to by the 'this' arguments more >>> directly. Bootstrap and regtest is in progress.. >>> >>> -- >8 -- >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/94772 >>> * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're >>> evaluating the target constructor of a delegating constructor. >>> (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the >>> INIT_EXPR is '*this'. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/94772 >>> * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. >>> * g++.dg/cpp1y/constexpr-tracking-const24.C: New test. >>> --- >>> gcc/cp/constexpr.c | 26 ++++++++++++++++++- >>> .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 +++++++++++++++ >>> .../g++.dg/cpp1y/constexpr-tracking-const24.C | 26 +++++++++++++++++++ >>> 3 files changed, 72 insertions(+), 1 deletion(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C >>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C >>> >>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>> index 6b3e514398b..5d9b10c63d4 100644 >>> --- a/gcc/cp/constexpr.c >>> +++ b/gcc/cp/constexpr.c >>> @@ -2371,6 +2371,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, >>> tree t, >>> STRIP_NOPS (new_obj); >>> if (TREE_CODE (new_obj) == ADDR_EXPR) >>> new_obj = TREE_OPERAND (new_obj, 0); >>> + >>> + if (ctx->call && ctx->call->fundef >>> + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) >>> + { >>> + tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); >>> + STRIP_NOPS (cur_obj); >>> + if (TREE_CODE (cur_obj) == ADDR_EXPR) >>> + cur_obj = TREE_OPERAND (cur_obj, 0); >>> + if (new_obj == cur_obj) >>> + /* We're calling the target constructor of a delegating >>> constructor, >>> + so there is no new object. */ >> >> ...so you'll want to update this comment. > > Done. > >> >> What happens if we get to 'base' by COMPONENT_REF rather than NOP_EXPR? > > It looks like in this case the TREE_TYPE of the evaluated COMPONENT_REF > is non-const regardless of the constness of the parent object, because > in cxx_eval_component_reference the built COMPONENT_REF inherits the > constness of the original tree > ((struct foo *) this)->D.2414; > that is passed as the 'this' argument to the base class constructor (and > is evaluated by cxx_bind_parameters_in_call). > > So because its TREE_TYPE is non-const, we don't consider setting > TREE_READONLY on the CONSTRUCTOR of the parent object at the end of > cxx_eval_call_expression, and likewise in cxx_eval_store_expression. So > when constructing a base subobject through a COMPONENT_REF it luckily > seems we don't have this constness problem with or without this patch. > > Here's the updated patch, with the comment updated and a new test added. OK, thanks. > -- >8 -- > > gcc/cp/ChangeLog: > > PR c++/94772 > * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're > evaluating the target constructor of a delegating constructor. > (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the > INIT_EXPR is '*this'. > > gcc/testsuite/ChangeLog: > > PR c++/94772 > * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. > * g++.dg/cpp1y/constexpr-tracking-const24.C: New test. > * g++.dg/cpp1y/constexpr-tracking-const25.C: New test. > --- > gcc/cp/constexpr.c | 28 +++++++- > .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++ > .../g++.dg/cpp1y/constexpr-tracking-const24.C | 26 ++++++++ > .../g++.dg/cpp1y/constexpr-tracking-const25.C | 66 +++++++++++++++++++ > 4 files changed, 140 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 6b3e514398b..637cb746576 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -2371,6 +2371,21 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > STRIP_NOPS (new_obj); > if (TREE_CODE (new_obj) == ADDR_EXPR) > new_obj = TREE_OPERAND (new_obj, 0); > + > + if (ctx->call && ctx->call->fundef > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) > + { > + tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); > + STRIP_NOPS (cur_obj); > + if (TREE_CODE (cur_obj) == ADDR_EXPR) > + cur_obj = TREE_OPERAND (cur_obj, 0); > + if (new_obj == cur_obj) > + /* We're calling the target constructor of a delegating > + constructor, or accessing a base subobject through a > + NOP_EXPR as part of a call to a base constructor, so > + there is no new (sub)object. */ > + new_obj = NULL_TREE; > + } > } > > tree result = NULL_TREE; > @@ -4950,7 +4965,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > if (TREE_CODE (t) == INIT_EXPR > && TREE_CODE (*valp) == CONSTRUCTOR > && TYPE_READONLY (type)) > - TREE_READONLY (*valp) = true; > + { > + if (INDIRECT_REF_P (target) > + && (is_this_parameter > + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) > + /* We've just initialized '*this' (perhaps via the target > + constructor of a delegating constructor). Leave it up to the > + caller that set 'this' to set TREE_READONLY appropriately. */ > + gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p > + (TREE_TYPE (target), type)); > + else > + TREE_READONLY (*valp) = true; > + } > > /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing > CONSTRUCTORs, if any. */ > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > new file mode 100644 > index 00000000000..c6643c78a6f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C > @@ -0,0 +1,21 @@ > +// PR c++/94772 > +// { dg-do compile { target c++14 } } > + > +struct foo > +{ > + int x{}; > + > + constexpr foo() noexcept = default; > + > + constexpr foo(int a) : foo{} > + { x = -a; } > + > + constexpr foo(int a, int b) : foo{a} > + { x += a + b; } > +}; > + > +int main() > +{ > + constexpr foo bar{1, 2}; > + static_assert(bar.x == 2, ""); > +} > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C > new file mode 100644 > index 00000000000..2c923f69cf4 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const24.C > @@ -0,0 +1,26 @@ > +// PR c++/94772 > +// { dg-do compile { target c++14 } } > + > +struct base > +{ > + base() = default; > + > + constexpr base(int) : base{} { } > +}; > + > +struct foo : base > +{ > + int x{}; > + > + constexpr foo(int a) : base{a} > + { x = -a; } > + > + constexpr foo(int a, int b) : foo{a} > + { x += a + b; } > +}; > + > +int main() > +{ > + constexpr foo bar{1, 2}; > + static_assert(bar.x == 2, ""); > +} > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C > new file mode 100644 > index 00000000000..662a6f93642 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const25.C > @@ -0,0 +1,66 @@ > +// PR c++/94772 > +// { dg-do compile { target c++14 } } > + > +template<int> > +struct base > +{ > + int y{}; > + > + base() = default; > + > + constexpr base(int a) : base{} > + { y = a; } > +}; > + > +struct foo : base<1>, base<2> > +{ > + int x{}; > + > + constexpr foo() : base<2>{} > + { > + x = x; > + ++base<1>::y; > + ++base<2>::y; > + } > + > + constexpr foo(int a) : base<2>{a} > + { > + x = -base<2>::y; > + ++base<1>::y; > + ++base<2>::y; > + } > + > + constexpr foo(int a, int b) : foo{a} > + { > + x += a + b; > + ++base<1>::y; > + ++base<2>::y; > + } > + > + constexpr foo(int a, int b, int c) : base<1>{a} > + { > + x += a + b + c; > + ++base<1>::y; > + ++base<2>::y; > + } > +}; > + > +#define SA(X) static_assert(X, #X) > + > +int main() > +{ > + constexpr foo bar1{1, 2}; > + SA( bar1.x == 2 ); > + SA( bar1.base<1>::y == 2 ); > + SA( bar1.base<2>::y == 3 ); > + > + constexpr foo bar2{1, 2, 3}; > + SA( bar2.x == 6 ); > + SA( bar2.base<1>::y == 2 ); > + SA( bar2.base<2>::y == 1 ); > + > + constexpr foo bar3{}; > + SA( bar3.x == 0 ); > + SA( bar3.base<1>::y == 1 ); > + SA( bar3.base<2>::y == 1 ); > +} >
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 6b3e514398b..a9ddd861195 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -2367,10 +2367,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, /* In a constructor, it should be the first `this' argument. At this point it has already been evaluated in the call to cxx_bind_parameters_in_call. */ - new_obj = TREE_VEC_ELT (new_call.bindings, 0); - STRIP_NOPS (new_obj); - if (TREE_CODE (new_obj) == ADDR_EXPR) - new_obj = TREE_OPERAND (new_obj, 0); + + if (ctx->call && ctx->call->fundef + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl) + && (TREE_VEC_ELT (ctx->call->bindings, 0) + == TREE_VEC_ELT (new_call.bindings, 0))) + /* We're calling the target constructor of a delegating constructor, so + there is no new object. */; + else + { + new_obj = TREE_VEC_ELT (new_call.bindings, 0); + STRIP_NOPS (new_obj); + if (TREE_CODE (new_obj) == ADDR_EXPR) + new_obj = TREE_OPERAND (new_obj, 0); + } } tree result = NULL_TREE; @@ -4950,7 +4960,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, if (TREE_CODE (t) == INIT_EXPR && TREE_CODE (*valp) == CONSTRUCTOR && TYPE_READONLY (type)) - TREE_READONLY (*valp) = true; + { + if (INDIRECT_REF_P (target) + && (is_this_parameter + (tree_strip_nop_conversions (TREE_OPERAND (target, 0))))) + /* We've just initialized '*this' (perhaps via the target constructor of + a delegating constructor). Leave it up to the caller that set 'this' + to set TREE_READONLY appropriately. */; + else + TREE_READONLY (*valp) = true; + } /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing CONSTRUCTORs, if any. */ diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C new file mode 100644 index 00000000000..266b62b852f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C @@ -0,0 +1,21 @@ +// PR c++/94772 +// { dg-do compile { target c++14 } } + +struct foo +{ + int x{}; + + constexpr foo() noexcept = default; + + constexpr foo(int a) : foo{} + { x = -a; } + + constexpr foo(int a, int b) : foo{a} + { x += a + b; } +}; + +int main() +{ + constexpr foo bar{1, 2}; + static_assert(bar.x == 2, ""); +}
In the testcase below, the call to the target constructor foo{} from foo's delegating constructor is encoded as the INIT_EXPR *(struct foo *) this = AGGR_INIT_EXPR <4, __ct_comp, D.2140, ...>; During initialization of the variable 'bar', we prematurely set TREE_READONLY on bar's CONSTRUCTOR in two places before the outer delegating constructor has returned: first, at the end of cxx_eval_call_expression after evaluating the RHS of the above INIT_EXPR, and second, at the end of cxx_eval_store_expression after having finished evaluating the above INIT_EXPR. This then prevents the rest of the outer delegating constructor from mutating 'bar'. This (hopefully minimally risky) patch makes cxx_eval_call_expression refrain from setting TREE_READONLY when evaluating the target constructor of a delegating constructor. It also makes cxx_eval_store_expression refrain from setting TREE_READONLY when the object being initialized is "*this', on the basis that it should be the responsibility of the routine that set 'this' in the first place to set the object's TREE_READONLY appropriately. Passes 'make check-c++', does this look OK to commit after full bootstrap/regtest? gcc/cp/ChangeLog: PR c++/94772 * constexpr.c (cxx_eval_call_expression): Don't set new_obj if we're evaluating the target constructor of a delegating constructor. (cxx_eval_store_expression): Don't set TREE_READONLY if the LHS of the INIT_EXPR is '*this'. gcc/testsuite/ChangeLog: PR c++/94772 * g++.dg/cpp1y/constexpr-tracking-const23.C: New test. --- gcc/cp/constexpr.c | 29 +++++++++++++++---- .../g++.dg/cpp1y/constexpr-tracking-const23.C | 21 ++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const23.C