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
