* [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
@ 2023-11-03 1:18 Nathaniel Shead
2023-11-03 1:34 ` Nathaniel Shead
2023-12-09 20:12 ` Jason Merrill
0 siblings, 2 replies; 29+ messages in thread
From: Nathaniel Shead @ 2023-11-03 1:18 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
Bootstrapped and regtested on x86-64_pc_linux_gnu.
I'm not entirely sure if the change I made to have destructors clobber with
CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to have
broken by doing this and I wasn't able to find anything else that really
depended on this distinction other than a warning pass. Otherwise I could
experiment with a new clobber kind for destructor calls.
-- >8 --
This patch adds checks for using objects after they've been manually
destroyed via explicit destructor call. Currently this is only
implemented for 'top-level' objects; FIELD_DECLs and individual elements
of arrays will need a lot more work to track correctly and are left for
a future patch.
The other limitation is that destruction of parameter objects is checked
too 'early', happening at the end of the function call rather than the
end of the owning full-expression as they should be for consistency;
see cpp2a/constexpr-lifetime2.C. This is because I wasn't able to find a
good way to link the constructed parameter declarations with the
variable declarations that are actually destroyed later on to propagate
their lifetime status, so I'm leaving this for a later patch.
PR c++/71093
gcc/cp/ChangeLog:
* call.cc (build_trivial_dtor_call): Mark pseudo-destructors as
ending lifetime.
* constexpr.cc (constexpr_global_ctx::get_value_ptr): Don't
return NULL_TREE for objects we're initializing.
(constexpr_global_ctx::destroy_value): Rename from remove_value.
Only mark real variables as outside lifetime.
(constexpr_global_ctx::clear_value): New function.
(destroy_value_checked): New function.
(cxx_eval_call_expression): Defer complaining about non-constant
arg0 for operator delete. Use remove_value_safe.
(cxx_fold_indirect_ref_1): Handle conversion to 'as base' type.
(outside_lifetime_error): Include name of object we're
accessing.
(cxx_eval_store_expression): Handle clobbers. Improve error
messages.
(cxx_eval_constant_expression): Use remove_value_safe. Clear
bind variables before entering body.
* decl.cc (build_clobber_this): Mark destructors as ending
lifetime.
(start_preparsed_function): Pass false to build_clobber_this.
(begin_destructor_body): Pass true to build_clobber_this.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1y/constexpr-lifetime1.C: Improve error message.
* g++.dg/cpp1y/constexpr-lifetime2.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime3.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime4.C: Likewise.
* g++.dg/cpp2a/bitfield2.C: Likewise.
* g++.dg/cpp2a/constexpr-new3.C: Likewise. New check.
* g++.dg/cpp1y/constexpr-lifetime7.C: New test.
* g++.dg/cpp2a/constexpr-lifetime1.C: New test.
* g++.dg/cpp2a/constexpr-lifetime2.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/call.cc | 2 +-
gcc/cp/constexpr.cc | 149 +++++++++++++++---
gcc/cp/decl.cc | 10 +-
.../g++.dg/cpp1y/constexpr-lifetime1.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime2.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime3.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime4.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime7.C | 93 +++++++++++
gcc/testsuite/g++.dg/cpp2a/bitfield2.C | 2 +-
.../g++.dg/cpp2a/constexpr-lifetime1.C | 21 +++
.../g++.dg/cpp2a/constexpr-lifetime2.C | 23 +++
gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C | 17 +-
12 files changed, 292 insertions(+), 33 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 2eb54b5b6ed..e5e9c6c44f8 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9682,7 +9682,7 @@ build_trivial_dtor_call (tree instance, bool no_ptr_deref)
}
/* A trivial destructor should still clobber the object. */
- tree clobber = build_clobber (TREE_TYPE (instance));
+ tree clobber = build_clobber (TREE_TYPE (instance), CLOBBER_EOL);
return build2 (MODIFY_EXPR, void_type_node,
instance, clobber);
}
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index c05760e6789..4f0f590c38a 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1193,13 +1193,20 @@ public:
return *p;
return NULL_TREE;
}
- tree *get_value_ptr (tree t)
+ tree *get_value_ptr (tree t, bool initializing)
{
if (modifiable && !modifiable->contains (t))
return nullptr;
if (tree *p = values.get (t))
- if (*p != void_node)
- return p;
+ {
+ if (*p != void_node)
+ return p;
+ else if (initializing)
+ {
+ *p = NULL_TREE;
+ return p;
+ }
+ }
return nullptr;
}
void put_value (tree t, tree v)
@@ -1208,13 +1215,20 @@ public:
if (!already_in_map && modifiable)
modifiable->add (t);
}
- void remove_value (tree t)
+ void destroy_value (tree t)
{
- if (DECL_P (t))
+ if ((TREE_CODE (t) == VAR_DECL
+ || TREE_CODE (t) == PARM_DECL
+ || TREE_CODE (t) == RESULT_DECL)
+ && DECL_NAME (t) != in_charge_identifier)
values.put (t, void_node);
else
values.remove (t);
}
+ void clear_value (tree t)
+ {
+ values.remove (t);
+ }
};
/* Helper class for constexpr_global_ctx. In some cases we want to avoid
@@ -1238,7 +1252,7 @@ public:
~modifiable_tracker ()
{
for (tree t: set)
- global->remove_value (t);
+ global->clear_value (t);
global->modifiable = nullptr;
}
};
@@ -1278,6 +1292,40 @@ struct constexpr_ctx {
mce_value manifestly_const_eval;
};
+/* Remove T from the global values map, checking for attempts to destroy
+ a value that has already finished its lifetime. */
+
+static void
+destroy_value_checked (const constexpr_ctx* ctx, tree t, bool *non_constant_p)
+{
+ if (t == error_mark_node || TREE_TYPE (t) == error_mark_node)
+ return;
+
+ /* Don't error again here if we've already reported a problem. */
+ if (!*non_constant_p
+ && DECL_P (t)
+ /* Non-trivial destructors have their lifetimes ended explicitly
+ with a clobber, so don't worry about it here. */
+ && (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (t))
+ /* ...except parameters are remapped in cxx_eval_call_expression,
+ and the destructor call during cleanup won't be able to tell that
+ this value has already been destroyed, so complain now. This is
+ not quite unobservable, but is extremely unlikely to crop up in
+ practice; see g++.dg/cpp2a/constexpr-lifetime2.C. */
+ || TREE_CODE (t) == PARM_DECL)
+ && ctx->global->is_outside_lifetime (t))
+ {
+ if (!ctx->quiet)
+ {
+ auto_diagnostic_group d;
+ error ("destroying %qE outside its lifetime", t);
+ inform (DECL_SOURCE_LOCATION (t), "declared here");
+ }
+ *non_constant_p = true;
+ }
+ ctx->global->destroy_value (t);
+}
+
/* This internal flag controls whether we should avoid doing anything during
constexpr evaluation that would cause extra DECL_UID generation, such as
template instantiation and function body copying. */
@@ -2806,6 +2854,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
&& (CALL_FROM_NEW_OR_DELETE_P (t)
|| is_std_allocator_allocate (ctx->call)))
{
+ const bool new_op_p = IDENTIFIER_NEW_OP_P (DECL_NAME (fun));
const int nargs = call_expr_nargs (t);
tree arg0 = NULL_TREE;
for (int i = 0; i < nargs; ++i)
@@ -2813,12 +2862,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
tree arg = CALL_EXPR_ARG (t, i);
arg = cxx_eval_constant_expression (ctx, arg, vc_prvalue,
non_constant_p, overflow_p);
- VERIFY_CONSTANT (arg);
+ /* Deleting a non-constant pointer has a better error message
+ below. */
+ if (new_op_p || i != 0)
+ VERIFY_CONSTANT (arg);
if (i == 0)
arg0 = arg;
}
gcc_assert (arg0);
- if (IDENTIFIER_NEW_OP_P (DECL_NAME (fun)))
+ if (new_op_p)
{
tree type = build_array_type_nelts (char_type_node,
tree_to_uhwi (arg0));
@@ -2867,7 +2919,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
return t;
}
DECL_NAME (var) = heap_deleted_identifier;
- ctx->global->remove_value (var);
+ ctx->global->destroy_value (var);
ctx->global->heap_dealloc_count++;
return void_node;
}
@@ -2890,7 +2942,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
return t;
}
DECL_NAME (var) = heap_deleted_identifier;
- ctx->global->remove_value (var);
+ ctx->global->destroy_value (var);
ctx->global->heap_dealloc_count++;
return void_node;
}
@@ -3242,9 +3294,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
non_constant_p, overflow_p);
/* Remove the parms/result from the values map. */
- ctx->global->remove_value (res);
+ destroy_value_checked (ctx, res, non_constant_p);
for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
- ctx->global->remove_value (parm);
+ destroy_value_checked (ctx, parm, non_constant_p);
/* Free any parameter CONSTRUCTORs we aren't returning directly. */
while (!ctors->is_empty ())
@@ -5644,6 +5696,10 @@ cxx_fold_indirect_ref_1 (const constexpr_ctx *ctx, location_t loc, tree type,
}
}
+ /* Handle conversion to "as base" type. */
+ if (CLASSTYPE_AS_BASE (optype) == type)
+ return op;
+
/* Handle conversion to an empty base class, which is represented with a
NOP_EXPR. Do this before spelunking into the non-empty subobjects,
which is likely to be a waste of time (109678). */
@@ -5895,7 +5951,7 @@ outside_lifetime_error (location_t loc, tree r)
}
else
{
- error_at (loc, "accessing object outside its lifetime");
+ error_at (loc, "accessing %qE outside its lifetime", r);
inform (DECL_SOURCE_LOCATION (r), "declared here");
}
}
@@ -6112,8 +6168,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
constexpr_ctx new_ctx = *ctx;
tree init = TREE_OPERAND (t, 1);
- if (TREE_CLOBBER_P (init))
- /* Just ignore clobbers. */
+
+ if (TREE_CLOBBER_P (init)
+ && CLOBBER_KIND (init) != CLOBBER_EOL)
+ /* Only handle clobbers ending the lifetime of storage. */
return void_node;
/* First we figure out where we're storing to. */
@@ -6123,7 +6181,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
tree type = TREE_TYPE (target);
bool preeval = SCALAR_TYPE_P (type) || TREE_CODE (t) == MODIFY_EXPR;
- if (preeval)
+ if (preeval && !TREE_CLOBBER_P (init))
{
/* Evaluate the value to be stored without knowing what object it will be
stored in, so that any side-effects happen first. */
@@ -6231,11 +6289,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
&& const_object_being_modified == NULL_TREE)
const_object_being_modified = object;
+ if (DECL_P (object)
+ && TREE_CLOBBER_P (init)
+ && DECL_NAME (object) == heap_deleted_identifier)
+ /* Ignore clobbers of deleted allocations for now; we'll get a better error
+ message later when operator delete is called. */
+ return void_node;
+
/* And then find/build up our initializer for the path to the subobject
we're initializing. */
tree *valp;
if (DECL_P (object))
- valp = ctx->global->get_value_ptr (object);
+ valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
else
valp = NULL;
if (!valp)
@@ -6243,10 +6308,45 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
/* A constant-expression cannot modify objects from outside the
constant-expression. */
if (!ctx->quiet)
- error ("modification of %qE is not a constant expression", object);
+ {
+ auto_diagnostic_group d;
+ if (DECL_P (object) && DECL_NAME (object) == heap_deleted_identifier)
+ {
+ error ("modification of allocated storage after deallocation "
+ "is not a constant expression");
+ inform (DECL_SOURCE_LOCATION (object), "allocated here");
+ }
+ else if (DECL_P (object) && ctx->global->is_outside_lifetime (object))
+ {
+ if (TREE_CLOBBER_P (init))
+ error ("destroying %qE outside its lifetime", object);
+ else
+ error ("modification of %qE outside its lifetime "
+ "is not a constant expression", object);
+ inform (DECL_SOURCE_LOCATION (object), "declared here");
+ }
+ else
+ {
+ if (TREE_CLOBBER_P (init))
+ error ("destroying %qE from outside current evaluation "
+ "is not a constant expression", object);
+ else
+ error ("modification of %qE from outside current evaluation "
+ "is not a constant expression", object);
+ }
+ }
*non_constant_p = true;
return t;
}
+
+ /* Handle explicit end-of-lifetime. */
+ if (TREE_CLOBBER_P (init))
+ {
+ if (refs->is_empty ())
+ ctx->global->destroy_value (object);
+ return void_node;
+ }
+
type = TREE_TYPE (object);
bool no_zero_init = true;
@@ -6520,7 +6620,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
/* The hash table might have moved since the get earlier, and the
initializer might have mutated the underlying CONSTRUCTORs, so we must
recompute VALP. */
- valp = ctx->global->get_value_ptr (object);
+ valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
for (unsigned i = 0; i < vec_safe_length (indexes); i++)
{
ctors[i] = valp;
@@ -7631,7 +7731,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
/* Forget SAVE_EXPRs and TARGET_EXPRs created by this
full-expression. */
for (tree save_expr : save_exprs)
- ctx->global->remove_value (save_expr);
+ destroy_value_checked (ctx, save_expr, non_constant_p);
}
break;
@@ -8184,13 +8284,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
non_constant_p, overflow_p, jump_target);
case BIND_EXPR:
+ /* Pre-emptively clear the vars declared by this BIND_EXPR from the value
+ map, so that when checking whether they're already destroyed later we
+ don't get confused by remnants of previous calls. */
+ for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
+ ctx->global->clear_value (decl);
r = cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
lval,
non_constant_p, overflow_p,
jump_target);
for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
- ctx->global->remove_value (decl);
- return r;
+ destroy_value_checked (ctx, decl, non_constant_p);
+ break;
case PREINCREMENT_EXPR:
case POSTINCREMENT_EXPR:
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 16af59de696..43f6379befb 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17313,7 +17313,7 @@ implicit_default_ctor_p (tree fn)
storage is dead when we enter the constructor or leave the destructor. */
static tree
-build_clobber_this ()
+build_clobber_this (bool is_destructor)
{
/* Clobbering an empty base is pointless, and harmful if its one byte
TYPE_SIZE overlays real data. */
@@ -17329,7 +17329,8 @@ build_clobber_this ()
if (!vbases)
ctype = CLASSTYPE_AS_BASE (ctype);
- tree clobber = build_clobber (ctype);
+ enum clobber_kind kind = is_destructor ? CLOBBER_EOL : CLOBBER_UNDEF;
+ tree clobber = build_clobber (ctype, kind);
tree thisref = current_class_ref;
if (ctype != current_class_type)
@@ -17750,7 +17751,7 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
because part of the initialization might happen before we enter the
constructor, via AGGR_INIT_ZERO_FIRST (c++/68006). */
&& !implicit_default_ctor_p (decl1))
- finish_expr_stmt (build_clobber_this ());
+ finish_expr_stmt (build_clobber_this (/*is_destructor=*/false));
if (!processing_template_decl
&& DECL_CONSTRUCTOR_P (decl1)
@@ -17973,7 +17974,8 @@ begin_destructor_body (void)
finish_decl_cleanup (NULL_TREE, stmt);
}
else
- finish_decl_cleanup (NULL_TREE, build_clobber_this ());
+ finish_decl_cleanup (NULL_TREE,
+ build_clobber_this (/*is_destructor=*/true));
}
/* And insert cleanups for our bases and members so that they
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
index 43aa7c974c1..3fda29e0cc2 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
@@ -10,4 +10,4 @@ constexpr const int& test() {
auto local = S{}; // { dg-message "note: declared here" }
return local.get();
}
-constexpr int x = test(); // { dg-error "accessing object outside its lifetime" }
+constexpr int x = test(); // { dg-error "accessing .local. outside its lifetime" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
index 2f5ae8db6d5..d82ba5c8b73 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
@@ -8,7 +8,7 @@ struct S {
constexpr int error() {
const auto& local = S{}.get(); // { dg-message "note: declared here" }
- return local; // { dg-error "accessing object outside its lifetime" }
+ return local; // { dg-error "accessing '\[^'\]+' outside its lifetime" }
}
constexpr int x = error(); // { dg-message "in .constexpr. expansion" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
index 53785521d05..67e9b91c723 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
@@ -7,7 +7,7 @@ constexpr int f(int i) {
int j = 123; // { dg-message "note: declared here" }
p = &j;
}
- return *p; // { dg-error "accessing object outside its lifetime" }
+ return *p; // { dg-error "accessing 'j' outside its lifetime" }
}
constexpr int i = f(0); // { dg-message "in .constexpr. expansion" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
index 181a1201663..6f0d749dcf2 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
@@ -5,7 +5,7 @@ constexpr const double& test() {
return local;
}
-static_assert(test() == 3.0, ""); // { dg-error "constant|accessing object outside its lifetime" }
+static_assert(test() == 3.0, ""); // { dg-error "constant|accessing '\[^'\]+' outside its lifetime" }
// no deference, shouldn't error
static_assert((test(), true), "");
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
new file mode 100644
index 00000000000..4148f42f7be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
@@ -0,0 +1,93 @@
+// PR c++/71093
+// { dg-do compile { target c++14 } }
+
+constexpr int f (const int *p)
+{
+ typedef int T;
+ p->~T (); // { dg-error "destroying" }
+ return *p;
+}
+
+constexpr int i = 0;
+constexpr int j = f (&i);
+
+
+template <typename T>
+constexpr bool test_access() {
+ T x {};
+ x.~T();
+ T y = x; // { dg-error "lifetime" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_modification() {
+ T x {};
+ x.~T();
+ x = T(); // { dg-error "lifetime" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_scope() {
+ {
+ T x {};
+ x.~T();
+ } // { dg-error "destroying" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_destroy_temp() {
+ T{}.~T(); // { dg-error "destroying" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_parameter(T t) {
+ // note: error message occurs at point of call
+ t.~T();
+ return true;
+}
+
+template <typename T>
+constexpr void test_bindings_impl(int n) {
+ if (n == 0) return;
+ T a {};
+ if (n == 1) return;
+ T b {};
+}
+
+template <typename T>
+constexpr bool test_bindings() {
+ test_bindings_impl<T>(1);
+ test_bindings_impl<T>(0);
+ test_bindings_impl<T>(2);
+ return true;
+}
+
+constexpr bool i1 = test_access<int>(); // { dg-message "in .constexpr." }
+constexpr bool i2 = test_modification<int>(); // { dg-message "in .constexpr." }
+constexpr bool i3 = test_scope<int>(); // { dg-message "in .constexpr." }
+constexpr bool i4 = test_destroy_temp<int>(); // { dg-message "in .constexpr." "" { xfail *-*-* } }
+constexpr bool i5 = test_parameter(int{}); // { dg-error "destroying" }
+constexpr bool i6 = test_bindings<int>();
+
+struct Trivial { int x; };
+constexpr bool t1 = test_access<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t2 = test_modification<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t3 = test_scope<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t4 = test_destroy_temp<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t5 = test_parameter(Trivial{}); // { dg-error "destroying" }
+constexpr bool t6 = test_bindings<Trivial>();
+
+#if __cplusplus >= 202002L
+struct NonTrivial { int x; constexpr ~NonTrivial() {} }; // { dg-error "destroying" "" { target c++20 } }
+constexpr bool n1 = test_access<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n2 = test_modification<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n3 = test_scope<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n4 = test_destroy_temp<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n5 = test_parameter(NonTrivial{}); // { dg-error "destroying" "" { target c++20 } }
+constexpr bool n6 = test_bindings<NonTrivial>();
+#endif
+
diff --git a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
index dcb424fc8f6..885d4f0e26d 100644
--- a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
+++ b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
@@ -13,7 +13,7 @@ template <bool V, int W>
struct U {
int j : W = 7; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
int k : W { 8 }; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
- int l : V ? 7 : a = 3; // { dg-error "modification of .a. is not a constant expression" }
+ int l : V ? 7 : a = 3; // { dg-error "modification of .a. from outside current evaluation is not a constant expression" }
// { dg-error "width not an integer constant" "" { target *-*-* } .-1 }
int m : (V ? W : b) = 9; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
// { dg-error "zero width for bit-field" "" { target *-*-* } .-1 }
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
new file mode 100644
index 00000000000..36163844eca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
@@ -0,0 +1,21 @@
+// { dg-do compile { target c++20 } }
+
+#include "construct_at.h"
+
+struct S { int x; };
+constexpr int f() {
+ S s;
+ s.~S();
+ std::construct_at(&s, 5);
+ return s.x;
+}
+static_assert(f() == 5);
+
+struct T { int x; constexpr ~T() {} };
+constexpr int g() {
+ T t;
+ t.~T();
+ std::construct_at(&t, 12);
+ return t.x;
+}
+static_assert(g() == 12);
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
new file mode 100644
index 00000000000..56cc9e3c1c8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
@@ -0,0 +1,23 @@
+// { dg-do compile { target c++20 } }
+
+#include "construct_at.h"
+
+struct S { int x; };
+
+constexpr bool foo(S s, S*& p) {
+ p = &s;
+ s.~S();
+ return true;
+}
+
+constexpr bool bar() {
+ // This is, strictly speaking, implementation-defined behaviour;
+ // see [expr.call] p6. However, in all other cases we destroy
+ // at the end of the full-expression, so the below should be fixed.
+ S* p;
+ foo(S{}, p), std::construct_at(p); // { dg-bogus "destroying" "" { xfail *-*-* } }
+
+ return true;
+}
+
+constexpr bool x = bar();
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
index 3ba440fec53..5d9f192507b 100644
--- a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
@@ -34,7 +34,7 @@ constexpr auto v3 = f3 (); // { dg-message "in 'constexpr' expansion of" }
constexpr bool
f4 (int *p)
{
- delete p; // { dg-error "deallocation of storage that was not previously allocated" }
+ delete p; // { dg-error "destroying 'q' from outside current evaluation" }
return false;
}
@@ -70,3 +70,18 @@ f7 ()
}
constexpr auto v7 = f7 ();
+
+constexpr bool
+f8_impl (int *p)
+{
+ delete p; // { dg-error "deallocation of storage that was not previously allocated" }
+ return false;
+}
+
+constexpr bool
+f8 ()
+{
+ int q = 0;
+ return f8_impl (&q);
+}
+constexpr auto v8 = f8 (); // { dg-message "in 'constexpr' expansion of" }
--
2.42.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-11-03 1:18 [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093] Nathaniel Shead
@ 2023-11-03 1:34 ` Nathaniel Shead
2023-11-27 11:08 ` Nathaniel Shead
2023-12-09 20:12 ` Jason Merrill
1 sibling, 1 reply; 29+ messages in thread
From: Nathaniel Shead @ 2023-11-03 1:34 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
Oh, this also fixes PR102284 and its other linked PRs (apart from
fields); I forgot to note that in the commit.
On Fri, Nov 03, 2023 at 12:18:29PM +1100, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86-64_pc_linux_gnu.
>
> I'm not entirely sure if the change I made to have destructors clobber with
> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to have
> broken by doing this and I wasn't able to find anything else that really
> depended on this distinction other than a warning pass. Otherwise I could
> experiment with a new clobber kind for destructor calls.
>
> -- >8 --
>
> This patch adds checks for using objects after they've been manually
> destroyed via explicit destructor call. Currently this is only
> implemented for 'top-level' objects; FIELD_DECLs and individual elements
> of arrays will need a lot more work to track correctly and are left for
> a future patch.
>
> The other limitation is that destruction of parameter objects is checked
> too 'early', happening at the end of the function call rather than the
> end of the owning full-expression as they should be for consistency;
> see cpp2a/constexpr-lifetime2.C. This is because I wasn't able to find a
> good way to link the constructed parameter declarations with the
> variable declarations that are actually destroyed later on to propagate
> their lifetime status, so I'm leaving this for a later patch.
>
> PR c++/71093
>
> gcc/cp/ChangeLog:
>
> * call.cc (build_trivial_dtor_call): Mark pseudo-destructors as
> ending lifetime.
> * constexpr.cc (constexpr_global_ctx::get_value_ptr): Don't
> return NULL_TREE for objects we're initializing.
> (constexpr_global_ctx::destroy_value): Rename from remove_value.
> Only mark real variables as outside lifetime.
> (constexpr_global_ctx::clear_value): New function.
> (destroy_value_checked): New function.
> (cxx_eval_call_expression): Defer complaining about non-constant
> arg0 for operator delete. Use remove_value_safe.
> (cxx_fold_indirect_ref_1): Handle conversion to 'as base' type.
> (outside_lifetime_error): Include name of object we're
> accessing.
> (cxx_eval_store_expression): Handle clobbers. Improve error
> messages.
> (cxx_eval_constant_expression): Use remove_value_safe. Clear
> bind variables before entering body.
> * decl.cc (build_clobber_this): Mark destructors as ending
> lifetime.
> (start_preparsed_function): Pass false to build_clobber_this.
> (begin_destructor_body): Pass true to build_clobber_this.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp1y/constexpr-lifetime1.C: Improve error message.
> * g++.dg/cpp1y/constexpr-lifetime2.C: Likewise.
> * g++.dg/cpp1y/constexpr-lifetime3.C: Likewise.
> * g++.dg/cpp1y/constexpr-lifetime4.C: Likewise.
> * g++.dg/cpp2a/bitfield2.C: Likewise.
> * g++.dg/cpp2a/constexpr-new3.C: Likewise. New check.
> * g++.dg/cpp1y/constexpr-lifetime7.C: New test.
> * g++.dg/cpp2a/constexpr-lifetime1.C: New test.
> * g++.dg/cpp2a/constexpr-lifetime2.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
> gcc/cp/call.cc | 2 +-
> gcc/cp/constexpr.cc | 149 +++++++++++++++---
> gcc/cp/decl.cc | 10 +-
> .../g++.dg/cpp1y/constexpr-lifetime1.C | 2 +-
> .../g++.dg/cpp1y/constexpr-lifetime2.C | 2 +-
> .../g++.dg/cpp1y/constexpr-lifetime3.C | 2 +-
> .../g++.dg/cpp1y/constexpr-lifetime4.C | 2 +-
> .../g++.dg/cpp1y/constexpr-lifetime7.C | 93 +++++++++++
> gcc/testsuite/g++.dg/cpp2a/bitfield2.C | 2 +-
> .../g++.dg/cpp2a/constexpr-lifetime1.C | 21 +++
> .../g++.dg/cpp2a/constexpr-lifetime2.C | 23 +++
> gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C | 17 +-
> 12 files changed, 292 insertions(+), 33 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 2eb54b5b6ed..e5e9c6c44f8 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -9682,7 +9682,7 @@ build_trivial_dtor_call (tree instance, bool no_ptr_deref)
> }
>
> /* A trivial destructor should still clobber the object. */
> - tree clobber = build_clobber (TREE_TYPE (instance));
> + tree clobber = build_clobber (TREE_TYPE (instance), CLOBBER_EOL);
> return build2 (MODIFY_EXPR, void_type_node,
> instance, clobber);
> }
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index c05760e6789..4f0f590c38a 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -1193,13 +1193,20 @@ public:
> return *p;
> return NULL_TREE;
> }
> - tree *get_value_ptr (tree t)
> + tree *get_value_ptr (tree t, bool initializing)
> {
> if (modifiable && !modifiable->contains (t))
> return nullptr;
> if (tree *p = values.get (t))
> - if (*p != void_node)
> - return p;
> + {
> + if (*p != void_node)
> + return p;
> + else if (initializing)
> + {
> + *p = NULL_TREE;
> + return p;
> + }
> + }
> return nullptr;
> }
> void put_value (tree t, tree v)
> @@ -1208,13 +1215,20 @@ public:
> if (!already_in_map && modifiable)
> modifiable->add (t);
> }
> - void remove_value (tree t)
> + void destroy_value (tree t)
> {
> - if (DECL_P (t))
> + if ((TREE_CODE (t) == VAR_DECL
> + || TREE_CODE (t) == PARM_DECL
> + || TREE_CODE (t) == RESULT_DECL)
> + && DECL_NAME (t) != in_charge_identifier)
> values.put (t, void_node);
> else
> values.remove (t);
> }
> + void clear_value (tree t)
> + {
> + values.remove (t);
> + }
> };
>
> /* Helper class for constexpr_global_ctx. In some cases we want to avoid
> @@ -1238,7 +1252,7 @@ public:
> ~modifiable_tracker ()
> {
> for (tree t: set)
> - global->remove_value (t);
> + global->clear_value (t);
> global->modifiable = nullptr;
> }
> };
> @@ -1278,6 +1292,40 @@ struct constexpr_ctx {
> mce_value manifestly_const_eval;
> };
>
> +/* Remove T from the global values map, checking for attempts to destroy
> + a value that has already finished its lifetime. */
> +
> +static void
> +destroy_value_checked (const constexpr_ctx* ctx, tree t, bool *non_constant_p)
> +{
> + if (t == error_mark_node || TREE_TYPE (t) == error_mark_node)
> + return;
> +
> + /* Don't error again here if we've already reported a problem. */
> + if (!*non_constant_p
> + && DECL_P (t)
> + /* Non-trivial destructors have their lifetimes ended explicitly
> + with a clobber, so don't worry about it here. */
> + && (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (t))
> + /* ...except parameters are remapped in cxx_eval_call_expression,
> + and the destructor call during cleanup won't be able to tell that
> + this value has already been destroyed, so complain now. This is
> + not quite unobservable, but is extremely unlikely to crop up in
> + practice; see g++.dg/cpp2a/constexpr-lifetime2.C. */
> + || TREE_CODE (t) == PARM_DECL)
> + && ctx->global->is_outside_lifetime (t))
> + {
> + if (!ctx->quiet)
> + {
> + auto_diagnostic_group d;
> + error ("destroying %qE outside its lifetime", t);
> + inform (DECL_SOURCE_LOCATION (t), "declared here");
> + }
> + *non_constant_p = true;
> + }
> + ctx->global->destroy_value (t);
> +}
> +
> /* This internal flag controls whether we should avoid doing anything during
> constexpr evaluation that would cause extra DECL_UID generation, such as
> template instantiation and function body copying. */
> @@ -2806,6 +2854,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> && (CALL_FROM_NEW_OR_DELETE_P (t)
> || is_std_allocator_allocate (ctx->call)))
> {
> + const bool new_op_p = IDENTIFIER_NEW_OP_P (DECL_NAME (fun));
> const int nargs = call_expr_nargs (t);
> tree arg0 = NULL_TREE;
> for (int i = 0; i < nargs; ++i)
> @@ -2813,12 +2862,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> tree arg = CALL_EXPR_ARG (t, i);
> arg = cxx_eval_constant_expression (ctx, arg, vc_prvalue,
> non_constant_p, overflow_p);
> - VERIFY_CONSTANT (arg);
> + /* Deleting a non-constant pointer has a better error message
> + below. */
> + if (new_op_p || i != 0)
> + VERIFY_CONSTANT (arg);
> if (i == 0)
> arg0 = arg;
> }
> gcc_assert (arg0);
> - if (IDENTIFIER_NEW_OP_P (DECL_NAME (fun)))
> + if (new_op_p)
> {
> tree type = build_array_type_nelts (char_type_node,
> tree_to_uhwi (arg0));
> @@ -2867,7 +2919,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> return t;
> }
> DECL_NAME (var) = heap_deleted_identifier;
> - ctx->global->remove_value (var);
> + ctx->global->destroy_value (var);
> ctx->global->heap_dealloc_count++;
> return void_node;
> }
> @@ -2890,7 +2942,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> return t;
> }
> DECL_NAME (var) = heap_deleted_identifier;
> - ctx->global->remove_value (var);
> + ctx->global->destroy_value (var);
> ctx->global->heap_dealloc_count++;
> return void_node;
> }
> @@ -3242,9 +3294,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> non_constant_p, overflow_p);
>
> /* Remove the parms/result from the values map. */
> - ctx->global->remove_value (res);
> + destroy_value_checked (ctx, res, non_constant_p);
> for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
> - ctx->global->remove_value (parm);
> + destroy_value_checked (ctx, parm, non_constant_p);
>
> /* Free any parameter CONSTRUCTORs we aren't returning directly. */
> while (!ctors->is_empty ())
> @@ -5644,6 +5696,10 @@ cxx_fold_indirect_ref_1 (const constexpr_ctx *ctx, location_t loc, tree type,
> }
> }
>
> + /* Handle conversion to "as base" type. */
> + if (CLASSTYPE_AS_BASE (optype) == type)
> + return op;
> +
> /* Handle conversion to an empty base class, which is represented with a
> NOP_EXPR. Do this before spelunking into the non-empty subobjects,
> which is likely to be a waste of time (109678). */
> @@ -5895,7 +5951,7 @@ outside_lifetime_error (location_t loc, tree r)
> }
> else
> {
> - error_at (loc, "accessing object outside its lifetime");
> + error_at (loc, "accessing %qE outside its lifetime", r);
> inform (DECL_SOURCE_LOCATION (r), "declared here");
> }
> }
> @@ -6112,8 +6168,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> constexpr_ctx new_ctx = *ctx;
>
> tree init = TREE_OPERAND (t, 1);
> - if (TREE_CLOBBER_P (init))
> - /* Just ignore clobbers. */
> +
> + if (TREE_CLOBBER_P (init)
> + && CLOBBER_KIND (init) != CLOBBER_EOL)
> + /* Only handle clobbers ending the lifetime of storage. */
> return void_node;
>
> /* First we figure out where we're storing to. */
> @@ -6123,7 +6181,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>
> tree type = TREE_TYPE (target);
> bool preeval = SCALAR_TYPE_P (type) || TREE_CODE (t) == MODIFY_EXPR;
> - if (preeval)
> + if (preeval && !TREE_CLOBBER_P (init))
> {
> /* Evaluate the value to be stored without knowing what object it will be
> stored in, so that any side-effects happen first. */
> @@ -6231,11 +6289,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> && const_object_being_modified == NULL_TREE)
> const_object_being_modified = object;
>
> + if (DECL_P (object)
> + && TREE_CLOBBER_P (init)
> + && DECL_NAME (object) == heap_deleted_identifier)
> + /* Ignore clobbers of deleted allocations for now; we'll get a better error
> + message later when operator delete is called. */
> + return void_node;
> +
> /* And then find/build up our initializer for the path to the subobject
> we're initializing. */
> tree *valp;
> if (DECL_P (object))
> - valp = ctx->global->get_value_ptr (object);
> + valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
> else
> valp = NULL;
> if (!valp)
> @@ -6243,10 +6308,45 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> /* A constant-expression cannot modify objects from outside the
> constant-expression. */
> if (!ctx->quiet)
> - error ("modification of %qE is not a constant expression", object);
> + {
> + auto_diagnostic_group d;
> + if (DECL_P (object) && DECL_NAME (object) == heap_deleted_identifier)
> + {
> + error ("modification of allocated storage after deallocation "
> + "is not a constant expression");
> + inform (DECL_SOURCE_LOCATION (object), "allocated here");
> + }
> + else if (DECL_P (object) && ctx->global->is_outside_lifetime (object))
> + {
> + if (TREE_CLOBBER_P (init))
> + error ("destroying %qE outside its lifetime", object);
> + else
> + error ("modification of %qE outside its lifetime "
> + "is not a constant expression", object);
> + inform (DECL_SOURCE_LOCATION (object), "declared here");
> + }
> + else
> + {
> + if (TREE_CLOBBER_P (init))
> + error ("destroying %qE from outside current evaluation "
> + "is not a constant expression", object);
> + else
> + error ("modification of %qE from outside current evaluation "
> + "is not a constant expression", object);
> + }
> + }
> *non_constant_p = true;
> return t;
> }
> +
> + /* Handle explicit end-of-lifetime. */
> + if (TREE_CLOBBER_P (init))
> + {
> + if (refs->is_empty ())
> + ctx->global->destroy_value (object);
> + return void_node;
> + }
> +
> type = TREE_TYPE (object);
> bool no_zero_init = true;
>
> @@ -6520,7 +6620,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> /* The hash table might have moved since the get earlier, and the
> initializer might have mutated the underlying CONSTRUCTORs, so we must
> recompute VALP. */
> - valp = ctx->global->get_value_ptr (object);
> + valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
> for (unsigned i = 0; i < vec_safe_length (indexes); i++)
> {
> ctors[i] = valp;
> @@ -7631,7 +7731,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> /* Forget SAVE_EXPRs and TARGET_EXPRs created by this
> full-expression. */
> for (tree save_expr : save_exprs)
> - ctx->global->remove_value (save_expr);
> + destroy_value_checked (ctx, save_expr, non_constant_p);
> }
> break;
>
> @@ -8184,13 +8284,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> non_constant_p, overflow_p, jump_target);
>
> case BIND_EXPR:
> + /* Pre-emptively clear the vars declared by this BIND_EXPR from the value
> + map, so that when checking whether they're already destroyed later we
> + don't get confused by remnants of previous calls. */
> + for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
> + ctx->global->clear_value (decl);
> r = cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
> lval,
> non_constant_p, overflow_p,
> jump_target);
> for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
> - ctx->global->remove_value (decl);
> - return r;
> + destroy_value_checked (ctx, decl, non_constant_p);
> + break;
>
> case PREINCREMENT_EXPR:
> case POSTINCREMENT_EXPR:
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 16af59de696..43f6379befb 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -17313,7 +17313,7 @@ implicit_default_ctor_p (tree fn)
> storage is dead when we enter the constructor or leave the destructor. */
>
> static tree
> -build_clobber_this ()
> +build_clobber_this (bool is_destructor)
> {
> /* Clobbering an empty base is pointless, and harmful if its one byte
> TYPE_SIZE overlays real data. */
> @@ -17329,7 +17329,8 @@ build_clobber_this ()
> if (!vbases)
> ctype = CLASSTYPE_AS_BASE (ctype);
>
> - tree clobber = build_clobber (ctype);
> + enum clobber_kind kind = is_destructor ? CLOBBER_EOL : CLOBBER_UNDEF;
> + tree clobber = build_clobber (ctype, kind);
>
> tree thisref = current_class_ref;
> if (ctype != current_class_type)
> @@ -17750,7 +17751,7 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
> because part of the initialization might happen before we enter the
> constructor, via AGGR_INIT_ZERO_FIRST (c++/68006). */
> && !implicit_default_ctor_p (decl1))
> - finish_expr_stmt (build_clobber_this ());
> + finish_expr_stmt (build_clobber_this (/*is_destructor=*/false));
>
> if (!processing_template_decl
> && DECL_CONSTRUCTOR_P (decl1)
> @@ -17973,7 +17974,8 @@ begin_destructor_body (void)
> finish_decl_cleanup (NULL_TREE, stmt);
> }
> else
> - finish_decl_cleanup (NULL_TREE, build_clobber_this ());
> + finish_decl_cleanup (NULL_TREE,
> + build_clobber_this (/*is_destructor=*/true));
> }
>
> /* And insert cleanups for our bases and members so that they
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
> index 43aa7c974c1..3fda29e0cc2 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
> @@ -10,4 +10,4 @@ constexpr const int& test() {
> auto local = S{}; // { dg-message "note: declared here" }
> return local.get();
> }
> -constexpr int x = test(); // { dg-error "accessing object outside its lifetime" }
> +constexpr int x = test(); // { dg-error "accessing .local. outside its lifetime" }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
> index 2f5ae8db6d5..d82ba5c8b73 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
> @@ -8,7 +8,7 @@ struct S {
>
> constexpr int error() {
> const auto& local = S{}.get(); // { dg-message "note: declared here" }
> - return local; // { dg-error "accessing object outside its lifetime" }
> + return local; // { dg-error "accessing '\[^'\]+' outside its lifetime" }
> }
> constexpr int x = error(); // { dg-message "in .constexpr. expansion" }
>
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
> index 53785521d05..67e9b91c723 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
> @@ -7,7 +7,7 @@ constexpr int f(int i) {
> int j = 123; // { dg-message "note: declared here" }
> p = &j;
> }
> - return *p; // { dg-error "accessing object outside its lifetime" }
> + return *p; // { dg-error "accessing 'j' outside its lifetime" }
> }
>
> constexpr int i = f(0); // { dg-message "in .constexpr. expansion" }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
> index 181a1201663..6f0d749dcf2 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
> @@ -5,7 +5,7 @@ constexpr const double& test() {
> return local;
> }
>
> -static_assert(test() == 3.0, ""); // { dg-error "constant|accessing object outside its lifetime" }
> +static_assert(test() == 3.0, ""); // { dg-error "constant|accessing '\[^'\]+' outside its lifetime" }
>
> // no deference, shouldn't error
> static_assert((test(), true), "");
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
> new file mode 100644
> index 00000000000..4148f42f7be
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
> @@ -0,0 +1,93 @@
> +// PR c++/71093
> +// { dg-do compile { target c++14 } }
> +
> +constexpr int f (const int *p)
> +{
> + typedef int T;
> + p->~T (); // { dg-error "destroying" }
> + return *p;
> +}
> +
> +constexpr int i = 0;
> +constexpr int j = f (&i);
> +
> +
> +template <typename T>
> +constexpr bool test_access() {
> + T x {};
> + x.~T();
> + T y = x; // { dg-error "lifetime" }
> + return true;
> +}
> +
> +template <typename T>
> +constexpr bool test_modification() {
> + T x {};
> + x.~T();
> + x = T(); // { dg-error "lifetime" }
> + return true;
> +}
> +
> +template <typename T>
> +constexpr bool test_scope() {
> + {
> + T x {};
> + x.~T();
> + } // { dg-error "destroying" }
> + return true;
> +}
> +
> +template <typename T>
> +constexpr bool test_destroy_temp() {
> + T{}.~T(); // { dg-error "destroying" }
> + return true;
> +}
> +
> +template <typename T>
> +constexpr bool test_parameter(T t) {
> + // note: error message occurs at point of call
> + t.~T();
> + return true;
> +}
> +
> +template <typename T>
> +constexpr void test_bindings_impl(int n) {
> + if (n == 0) return;
> + T a {};
> + if (n == 1) return;
> + T b {};
> +}
> +
> +template <typename T>
> +constexpr bool test_bindings() {
> + test_bindings_impl<T>(1);
> + test_bindings_impl<T>(0);
> + test_bindings_impl<T>(2);
> + return true;
> +}
> +
> +constexpr bool i1 = test_access<int>(); // { dg-message "in .constexpr." }
> +constexpr bool i2 = test_modification<int>(); // { dg-message "in .constexpr." }
> +constexpr bool i3 = test_scope<int>(); // { dg-message "in .constexpr." }
> +constexpr bool i4 = test_destroy_temp<int>(); // { dg-message "in .constexpr." "" { xfail *-*-* } }
> +constexpr bool i5 = test_parameter(int{}); // { dg-error "destroying" }
> +constexpr bool i6 = test_bindings<int>();
> +
> +struct Trivial { int x; };
> +constexpr bool t1 = test_access<Trivial>(); // { dg-message "in .constexpr." }
> +constexpr bool t2 = test_modification<Trivial>(); // { dg-message "in .constexpr." }
> +constexpr bool t3 = test_scope<Trivial>(); // { dg-message "in .constexpr." }
> +constexpr bool t4 = test_destroy_temp<Trivial>(); // { dg-message "in .constexpr." }
> +constexpr bool t5 = test_parameter(Trivial{}); // { dg-error "destroying" }
> +constexpr bool t6 = test_bindings<Trivial>();
> +
> +#if __cplusplus >= 202002L
> +struct NonTrivial { int x; constexpr ~NonTrivial() {} }; // { dg-error "destroying" "" { target c++20 } }
> +constexpr bool n1 = test_access<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> +constexpr bool n2 = test_modification<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> +constexpr bool n3 = test_scope<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> +constexpr bool n4 = test_destroy_temp<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> +constexpr bool n5 = test_parameter(NonTrivial{}); // { dg-error "destroying" "" { target c++20 } }
> +constexpr bool n6 = test_bindings<NonTrivial>();
> +#endif
> +
> diff --git a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
> index dcb424fc8f6..885d4f0e26d 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
> @@ -13,7 +13,7 @@ template <bool V, int W>
> struct U {
> int j : W = 7; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
> int k : W { 8 }; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
> - int l : V ? 7 : a = 3; // { dg-error "modification of .a. is not a constant expression" }
> + int l : V ? 7 : a = 3; // { dg-error "modification of .a. from outside current evaluation is not a constant expression" }
> // { dg-error "width not an integer constant" "" { target *-*-* } .-1 }
> int m : (V ? W : b) = 9; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
> // { dg-error "zero width for bit-field" "" { target *-*-* } .-1 }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
> new file mode 100644
> index 00000000000..36163844eca
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
> @@ -0,0 +1,21 @@
> +// { dg-do compile { target c++20 } }
> +
> +#include "construct_at.h"
> +
> +struct S { int x; };
> +constexpr int f() {
> + S s;
> + s.~S();
> + std::construct_at(&s, 5);
> + return s.x;
> +}
> +static_assert(f() == 5);
> +
> +struct T { int x; constexpr ~T() {} };
> +constexpr int g() {
> + T t;
> + t.~T();
> + std::construct_at(&t, 12);
> + return t.x;
> +}
> +static_assert(g() == 12);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
> new file mode 100644
> index 00000000000..56cc9e3c1c8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
> @@ -0,0 +1,23 @@
> +// { dg-do compile { target c++20 } }
> +
> +#include "construct_at.h"
> +
> +struct S { int x; };
> +
> +constexpr bool foo(S s, S*& p) {
> + p = &s;
> + s.~S();
> + return true;
> +}
> +
> +constexpr bool bar() {
> + // This is, strictly speaking, implementation-defined behaviour;
> + // see [expr.call] p6. However, in all other cases we destroy
> + // at the end of the full-expression, so the below should be fixed.
> + S* p;
> + foo(S{}, p), std::construct_at(p); // { dg-bogus "destroying" "" { xfail *-*-* } }
> +
> + return true;
> +}
> +
> +constexpr bool x = bar();
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
> index 3ba440fec53..5d9f192507b 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
> @@ -34,7 +34,7 @@ constexpr auto v3 = f3 (); // { dg-message "in 'constexpr' expansion of" }
> constexpr bool
> f4 (int *p)
> {
> - delete p; // { dg-error "deallocation of storage that was not previously allocated" }
> + delete p; // { dg-error "destroying 'q' from outside current evaluation" }
> return false;
> }
>
> @@ -70,3 +70,18 @@ f7 ()
> }
>
> constexpr auto v7 = f7 ();
> +
> +constexpr bool
> +f8_impl (int *p)
> +{
> + delete p; // { dg-error "deallocation of storage that was not previously allocated" }
> + return false;
> +}
> +
> +constexpr bool
> +f8 ()
> +{
> + int q = 0;
> + return f8_impl (&q);
> +}
> +constexpr auto v8 = f8 (); // { dg-message "in 'constexpr' expansion of" }
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-11-03 1:34 ` Nathaniel Shead
@ 2023-11-27 11:08 ` Nathaniel Shead
0 siblings, 0 replies; 29+ messages in thread
From: Nathaniel Shead @ 2023-11-27 11:08 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635071.html.
On Fri, Nov 03, 2023 at 12:34:06PM +1100, Nathaniel Shead wrote:
> Oh, this also fixes PR102284 and its other linked PRs (apart from
> fields); I forgot to note that in the commit.
>
> On Fri, Nov 03, 2023 at 12:18:29PM +1100, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86-64_pc_linux_gnu.
> >
> > I'm not entirely sure if the change I made to have destructors clobber with
> > CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to have
> > broken by doing this and I wasn't able to find anything else that really
> > depended on this distinction other than a warning pass. Otherwise I could
> > experiment with a new clobber kind for destructor calls.
> >
> > -- >8 --
> >
> > This patch adds checks for using objects after they've been manually
> > destroyed via explicit destructor call. Currently this is only
> > implemented for 'top-level' objects; FIELD_DECLs and individual elements
> > of arrays will need a lot more work to track correctly and are left for
> > a future patch.
> >
> > The other limitation is that destruction of parameter objects is checked
> > too 'early', happening at the end of the function call rather than the
> > end of the owning full-expression as they should be for consistency;
> > see cpp2a/constexpr-lifetime2.C. This is because I wasn't able to find a
> > good way to link the constructed parameter declarations with the
> > variable declarations that are actually destroyed later on to propagate
> > their lifetime status, so I'm leaving this for a later patch.
> >
> > PR c++/71093
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (build_trivial_dtor_call): Mark pseudo-destructors as
> > ending lifetime.
> > * constexpr.cc (constexpr_global_ctx::get_value_ptr): Don't
> > return NULL_TREE for objects we're initializing.
> > (constexpr_global_ctx::destroy_value): Rename from remove_value.
> > Only mark real variables as outside lifetime.
> > (constexpr_global_ctx::clear_value): New function.
> > (destroy_value_checked): New function.
> > (cxx_eval_call_expression): Defer complaining about non-constant
> > arg0 for operator delete. Use remove_value_safe.
> > (cxx_fold_indirect_ref_1): Handle conversion to 'as base' type.
> > (outside_lifetime_error): Include name of object we're
> > accessing.
> > (cxx_eval_store_expression): Handle clobbers. Improve error
> > messages.
> > (cxx_eval_constant_expression): Use remove_value_safe. Clear
> > bind variables before entering body.
> > * decl.cc (build_clobber_this): Mark destructors as ending
> > lifetime.
> > (start_preparsed_function): Pass false to build_clobber_this.
> > (begin_destructor_body): Pass true to build_clobber_this.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp1y/constexpr-lifetime1.C: Improve error message.
> > * g++.dg/cpp1y/constexpr-lifetime2.C: Likewise.
> > * g++.dg/cpp1y/constexpr-lifetime3.C: Likewise.
> > * g++.dg/cpp1y/constexpr-lifetime4.C: Likewise.
> > * g++.dg/cpp2a/bitfield2.C: Likewise.
> > * g++.dg/cpp2a/constexpr-new3.C: Likewise. New check.
> > * g++.dg/cpp1y/constexpr-lifetime7.C: New test.
> > * g++.dg/cpp2a/constexpr-lifetime1.C: New test.
> > * g++.dg/cpp2a/constexpr-lifetime2.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> > gcc/cp/call.cc | 2 +-
> > gcc/cp/constexpr.cc | 149 +++++++++++++++---
> > gcc/cp/decl.cc | 10 +-
> > .../g++.dg/cpp1y/constexpr-lifetime1.C | 2 +-
> > .../g++.dg/cpp1y/constexpr-lifetime2.C | 2 +-
> > .../g++.dg/cpp1y/constexpr-lifetime3.C | 2 +-
> > .../g++.dg/cpp1y/constexpr-lifetime4.C | 2 +-
> > .../g++.dg/cpp1y/constexpr-lifetime7.C | 93 +++++++++++
> > gcc/testsuite/g++.dg/cpp2a/bitfield2.C | 2 +-
> > .../g++.dg/cpp2a/constexpr-lifetime1.C | 21 +++
> > .../g++.dg/cpp2a/constexpr-lifetime2.C | 23 +++
> > gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C | 17 +-
> > 12 files changed, 292 insertions(+), 33 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
> >
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index 2eb54b5b6ed..e5e9c6c44f8 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -9682,7 +9682,7 @@ build_trivial_dtor_call (tree instance, bool no_ptr_deref)
> > }
> >
> > /* A trivial destructor should still clobber the object. */
> > - tree clobber = build_clobber (TREE_TYPE (instance));
> > + tree clobber = build_clobber (TREE_TYPE (instance), CLOBBER_EOL);
> > return build2 (MODIFY_EXPR, void_type_node,
> > instance, clobber);
> > }
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index c05760e6789..4f0f590c38a 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -1193,13 +1193,20 @@ public:
> > return *p;
> > return NULL_TREE;
> > }
> > - tree *get_value_ptr (tree t)
> > + tree *get_value_ptr (tree t, bool initializing)
> > {
> > if (modifiable && !modifiable->contains (t))
> > return nullptr;
> > if (tree *p = values.get (t))
> > - if (*p != void_node)
> > - return p;
> > + {
> > + if (*p != void_node)
> > + return p;
> > + else if (initializing)
> > + {
> > + *p = NULL_TREE;
> > + return p;
> > + }
> > + }
> > return nullptr;
> > }
> > void put_value (tree t, tree v)
> > @@ -1208,13 +1215,20 @@ public:
> > if (!already_in_map && modifiable)
> > modifiable->add (t);
> > }
> > - void remove_value (tree t)
> > + void destroy_value (tree t)
> > {
> > - if (DECL_P (t))
> > + if ((TREE_CODE (t) == VAR_DECL
> > + || TREE_CODE (t) == PARM_DECL
> > + || TREE_CODE (t) == RESULT_DECL)
> > + && DECL_NAME (t) != in_charge_identifier)
> > values.put (t, void_node);
> > else
> > values.remove (t);
> > }
> > + void clear_value (tree t)
> > + {
> > + values.remove (t);
> > + }
> > };
> >
> > /* Helper class for constexpr_global_ctx. In some cases we want to avoid
> > @@ -1238,7 +1252,7 @@ public:
> > ~modifiable_tracker ()
> > {
> > for (tree t: set)
> > - global->remove_value (t);
> > + global->clear_value (t);
> > global->modifiable = nullptr;
> > }
> > };
> > @@ -1278,6 +1292,40 @@ struct constexpr_ctx {
> > mce_value manifestly_const_eval;
> > };
> >
> > +/* Remove T from the global values map, checking for attempts to destroy
> > + a value that has already finished its lifetime. */
> > +
> > +static void
> > +destroy_value_checked (const constexpr_ctx* ctx, tree t, bool *non_constant_p)
> > +{
> > + if (t == error_mark_node || TREE_TYPE (t) == error_mark_node)
> > + return;
> > +
> > + /* Don't error again here if we've already reported a problem. */
> > + if (!*non_constant_p
> > + && DECL_P (t)
> > + /* Non-trivial destructors have their lifetimes ended explicitly
> > + with a clobber, so don't worry about it here. */
> > + && (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (t))
> > + /* ...except parameters are remapped in cxx_eval_call_expression,
> > + and the destructor call during cleanup won't be able to tell that
> > + this value has already been destroyed, so complain now. This is
> > + not quite unobservable, but is extremely unlikely to crop up in
> > + practice; see g++.dg/cpp2a/constexpr-lifetime2.C. */
> > + || TREE_CODE (t) == PARM_DECL)
> > + && ctx->global->is_outside_lifetime (t))
> > + {
> > + if (!ctx->quiet)
> > + {
> > + auto_diagnostic_group d;
> > + error ("destroying %qE outside its lifetime", t);
> > + inform (DECL_SOURCE_LOCATION (t), "declared here");
> > + }
> > + *non_constant_p = true;
> > + }
> > + ctx->global->destroy_value (t);
> > +}
> > +
> > /* This internal flag controls whether we should avoid doing anything during
> > constexpr evaluation that would cause extra DECL_UID generation, such as
> > template instantiation and function body copying. */
> > @@ -2806,6 +2854,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> > && (CALL_FROM_NEW_OR_DELETE_P (t)
> > || is_std_allocator_allocate (ctx->call)))
> > {
> > + const bool new_op_p = IDENTIFIER_NEW_OP_P (DECL_NAME (fun));
> > const int nargs = call_expr_nargs (t);
> > tree arg0 = NULL_TREE;
> > for (int i = 0; i < nargs; ++i)
> > @@ -2813,12 +2862,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> > tree arg = CALL_EXPR_ARG (t, i);
> > arg = cxx_eval_constant_expression (ctx, arg, vc_prvalue,
> > non_constant_p, overflow_p);
> > - VERIFY_CONSTANT (arg);
> > + /* Deleting a non-constant pointer has a better error message
> > + below. */
> > + if (new_op_p || i != 0)
> > + VERIFY_CONSTANT (arg);
> > if (i == 0)
> > arg0 = arg;
> > }
> > gcc_assert (arg0);
> > - if (IDENTIFIER_NEW_OP_P (DECL_NAME (fun)))
> > + if (new_op_p)
> > {
> > tree type = build_array_type_nelts (char_type_node,
> > tree_to_uhwi (arg0));
> > @@ -2867,7 +2919,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> > return t;
> > }
> > DECL_NAME (var) = heap_deleted_identifier;
> > - ctx->global->remove_value (var);
> > + ctx->global->destroy_value (var);
> > ctx->global->heap_dealloc_count++;
> > return void_node;
> > }
> > @@ -2890,7 +2942,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> > return t;
> > }
> > DECL_NAME (var) = heap_deleted_identifier;
> > - ctx->global->remove_value (var);
> > + ctx->global->destroy_value (var);
> > ctx->global->heap_dealloc_count++;
> > return void_node;
> > }
> > @@ -3242,9 +3294,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
> > non_constant_p, overflow_p);
> >
> > /* Remove the parms/result from the values map. */
> > - ctx->global->remove_value (res);
> > + destroy_value_checked (ctx, res, non_constant_p);
> > for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
> > - ctx->global->remove_value (parm);
> > + destroy_value_checked (ctx, parm, non_constant_p);
> >
> > /* Free any parameter CONSTRUCTORs we aren't returning directly. */
> > while (!ctors->is_empty ())
> > @@ -5644,6 +5696,10 @@ cxx_fold_indirect_ref_1 (const constexpr_ctx *ctx, location_t loc, tree type,
> > }
> > }
> >
> > + /* Handle conversion to "as base" type. */
> > + if (CLASSTYPE_AS_BASE (optype) == type)
> > + return op;
> > +
> > /* Handle conversion to an empty base class, which is represented with a
> > NOP_EXPR. Do this before spelunking into the non-empty subobjects,
> > which is likely to be a waste of time (109678). */
> > @@ -5895,7 +5951,7 @@ outside_lifetime_error (location_t loc, tree r)
> > }
> > else
> > {
> > - error_at (loc, "accessing object outside its lifetime");
> > + error_at (loc, "accessing %qE outside its lifetime", r);
> > inform (DECL_SOURCE_LOCATION (r), "declared here");
> > }
> > }
> > @@ -6112,8 +6168,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> > constexpr_ctx new_ctx = *ctx;
> >
> > tree init = TREE_OPERAND (t, 1);
> > - if (TREE_CLOBBER_P (init))
> > - /* Just ignore clobbers. */
> > +
> > + if (TREE_CLOBBER_P (init)
> > + && CLOBBER_KIND (init) != CLOBBER_EOL)
> > + /* Only handle clobbers ending the lifetime of storage. */
> > return void_node;
> >
> > /* First we figure out where we're storing to. */
> > @@ -6123,7 +6181,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> >
> > tree type = TREE_TYPE (target);
> > bool preeval = SCALAR_TYPE_P (type) || TREE_CODE (t) == MODIFY_EXPR;
> > - if (preeval)
> > + if (preeval && !TREE_CLOBBER_P (init))
> > {
> > /* Evaluate the value to be stored without knowing what object it will be
> > stored in, so that any side-effects happen first. */
> > @@ -6231,11 +6289,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> > && const_object_being_modified == NULL_TREE)
> > const_object_being_modified = object;
> >
> > + if (DECL_P (object)
> > + && TREE_CLOBBER_P (init)
> > + && DECL_NAME (object) == heap_deleted_identifier)
> > + /* Ignore clobbers of deleted allocations for now; we'll get a better error
> > + message later when operator delete is called. */
> > + return void_node;
> > +
> > /* And then find/build up our initializer for the path to the subobject
> > we're initializing. */
> > tree *valp;
> > if (DECL_P (object))
> > - valp = ctx->global->get_value_ptr (object);
> > + valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
> > else
> > valp = NULL;
> > if (!valp)
> > @@ -6243,10 +6308,45 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> > /* A constant-expression cannot modify objects from outside the
> > constant-expression. */
> > if (!ctx->quiet)
> > - error ("modification of %qE is not a constant expression", object);
> > + {
> > + auto_diagnostic_group d;
> > + if (DECL_P (object) && DECL_NAME (object) == heap_deleted_identifier)
> > + {
> > + error ("modification of allocated storage after deallocation "
> > + "is not a constant expression");
> > + inform (DECL_SOURCE_LOCATION (object), "allocated here");
> > + }
> > + else if (DECL_P (object) && ctx->global->is_outside_lifetime (object))
> > + {
> > + if (TREE_CLOBBER_P (init))
> > + error ("destroying %qE outside its lifetime", object);
> > + else
> > + error ("modification of %qE outside its lifetime "
> > + "is not a constant expression", object);
> > + inform (DECL_SOURCE_LOCATION (object), "declared here");
> > + }
> > + else
> > + {
> > + if (TREE_CLOBBER_P (init))
> > + error ("destroying %qE from outside current evaluation "
> > + "is not a constant expression", object);
> > + else
> > + error ("modification of %qE from outside current evaluation "
> > + "is not a constant expression", object);
> > + }
> > + }
> > *non_constant_p = true;
> > return t;
> > }
> > +
> > + /* Handle explicit end-of-lifetime. */
> > + if (TREE_CLOBBER_P (init))
> > + {
> > + if (refs->is_empty ())
> > + ctx->global->destroy_value (object);
> > + return void_node;
> > + }
> > +
> > type = TREE_TYPE (object);
> > bool no_zero_init = true;
> >
> > @@ -6520,7 +6620,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
> > /* The hash table might have moved since the get earlier, and the
> > initializer might have mutated the underlying CONSTRUCTORs, so we must
> > recompute VALP. */
> > - valp = ctx->global->get_value_ptr (object);
> > + valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
> > for (unsigned i = 0; i < vec_safe_length (indexes); i++)
> > {
> > ctors[i] = valp;
> > @@ -7631,7 +7731,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> > /* Forget SAVE_EXPRs and TARGET_EXPRs created by this
> > full-expression. */
> > for (tree save_expr : save_exprs)
> > - ctx->global->remove_value (save_expr);
> > + destroy_value_checked (ctx, save_expr, non_constant_p);
> > }
> > break;
> >
> > @@ -8184,13 +8284,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> > non_constant_p, overflow_p, jump_target);
> >
> > case BIND_EXPR:
> > + /* Pre-emptively clear the vars declared by this BIND_EXPR from the value
> > + map, so that when checking whether they're already destroyed later we
> > + don't get confused by remnants of previous calls. */
> > + for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
> > + ctx->global->clear_value (decl);
> > r = cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
> > lval,
> > non_constant_p, overflow_p,
> > jump_target);
> > for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
> > - ctx->global->remove_value (decl);
> > - return r;
> > + destroy_value_checked (ctx, decl, non_constant_p);
> > + break;
> >
> > case PREINCREMENT_EXPR:
> > case POSTINCREMENT_EXPR:
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 16af59de696..43f6379befb 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -17313,7 +17313,7 @@ implicit_default_ctor_p (tree fn)
> > storage is dead when we enter the constructor or leave the destructor. */
> >
> > static tree
> > -build_clobber_this ()
> > +build_clobber_this (bool is_destructor)
> > {
> > /* Clobbering an empty base is pointless, and harmful if its one byte
> > TYPE_SIZE overlays real data. */
> > @@ -17329,7 +17329,8 @@ build_clobber_this ()
> > if (!vbases)
> > ctype = CLASSTYPE_AS_BASE (ctype);
> >
> > - tree clobber = build_clobber (ctype);
> > + enum clobber_kind kind = is_destructor ? CLOBBER_EOL : CLOBBER_UNDEF;
> > + tree clobber = build_clobber (ctype, kind);
> >
> > tree thisref = current_class_ref;
> > if (ctype != current_class_type)
> > @@ -17750,7 +17751,7 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
> > because part of the initialization might happen before we enter the
> > constructor, via AGGR_INIT_ZERO_FIRST (c++/68006). */
> > && !implicit_default_ctor_p (decl1))
> > - finish_expr_stmt (build_clobber_this ());
> > + finish_expr_stmt (build_clobber_this (/*is_destructor=*/false));
> >
> > if (!processing_template_decl
> > && DECL_CONSTRUCTOR_P (decl1)
> > @@ -17973,7 +17974,8 @@ begin_destructor_body (void)
> > finish_decl_cleanup (NULL_TREE, stmt);
> > }
> > else
> > - finish_decl_cleanup (NULL_TREE, build_clobber_this ());
> > + finish_decl_cleanup (NULL_TREE,
> > + build_clobber_this (/*is_destructor=*/true));
> > }
> >
> > /* And insert cleanups for our bases and members so that they
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
> > index 43aa7c974c1..3fda29e0cc2 100644
> > --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
> > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
> > @@ -10,4 +10,4 @@ constexpr const int& test() {
> > auto local = S{}; // { dg-message "note: declared here" }
> > return local.get();
> > }
> > -constexpr int x = test(); // { dg-error "accessing object outside its lifetime" }
> > +constexpr int x = test(); // { dg-error "accessing .local. outside its lifetime" }
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
> > index 2f5ae8db6d5..d82ba5c8b73 100644
> > --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
> > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
> > @@ -8,7 +8,7 @@ struct S {
> >
> > constexpr int error() {
> > const auto& local = S{}.get(); // { dg-message "note: declared here" }
> > - return local; // { dg-error "accessing object outside its lifetime" }
> > + return local; // { dg-error "accessing '\[^'\]+' outside its lifetime" }
> > }
> > constexpr int x = error(); // { dg-message "in .constexpr. expansion" }
> >
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
> > index 53785521d05..67e9b91c723 100644
> > --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
> > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
> > @@ -7,7 +7,7 @@ constexpr int f(int i) {
> > int j = 123; // { dg-message "note: declared here" }
> > p = &j;
> > }
> > - return *p; // { dg-error "accessing object outside its lifetime" }
> > + return *p; // { dg-error "accessing 'j' outside its lifetime" }
> > }
> >
> > constexpr int i = f(0); // { dg-message "in .constexpr. expansion" }
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
> > index 181a1201663..6f0d749dcf2 100644
> > --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
> > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
> > @@ -5,7 +5,7 @@ constexpr const double& test() {
> > return local;
> > }
> >
> > -static_assert(test() == 3.0, ""); // { dg-error "constant|accessing object outside its lifetime" }
> > +static_assert(test() == 3.0, ""); // { dg-error "constant|accessing '\[^'\]+' outside its lifetime" }
> >
> > // no deference, shouldn't error
> > static_assert((test(), true), "");
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
> > new file mode 100644
> > index 00000000000..4148f42f7be
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
> > @@ -0,0 +1,93 @@
> > +// PR c++/71093
> > +// { dg-do compile { target c++14 } }
> > +
> > +constexpr int f (const int *p)
> > +{
> > + typedef int T;
> > + p->~T (); // { dg-error "destroying" }
> > + return *p;
> > +}
> > +
> > +constexpr int i = 0;
> > +constexpr int j = f (&i);
> > +
> > +
> > +template <typename T>
> > +constexpr bool test_access() {
> > + T x {};
> > + x.~T();
> > + T y = x; // { dg-error "lifetime" }
> > + return true;
> > +}
> > +
> > +template <typename T>
> > +constexpr bool test_modification() {
> > + T x {};
> > + x.~T();
> > + x = T(); // { dg-error "lifetime" }
> > + return true;
> > +}
> > +
> > +template <typename T>
> > +constexpr bool test_scope() {
> > + {
> > + T x {};
> > + x.~T();
> > + } // { dg-error "destroying" }
> > + return true;
> > +}
> > +
> > +template <typename T>
> > +constexpr bool test_destroy_temp() {
> > + T{}.~T(); // { dg-error "destroying" }
> > + return true;
> > +}
> > +
> > +template <typename T>
> > +constexpr bool test_parameter(T t) {
> > + // note: error message occurs at point of call
> > + t.~T();
> > + return true;
> > +}
> > +
> > +template <typename T>
> > +constexpr void test_bindings_impl(int n) {
> > + if (n == 0) return;
> > + T a {};
> > + if (n == 1) return;
> > + T b {};
> > +}
> > +
> > +template <typename T>
> > +constexpr bool test_bindings() {
> > + test_bindings_impl<T>(1);
> > + test_bindings_impl<T>(0);
> > + test_bindings_impl<T>(2);
> > + return true;
> > +}
> > +
> > +constexpr bool i1 = test_access<int>(); // { dg-message "in .constexpr." }
> > +constexpr bool i2 = test_modification<int>(); // { dg-message "in .constexpr." }
> > +constexpr bool i3 = test_scope<int>(); // { dg-message "in .constexpr." }
> > +constexpr bool i4 = test_destroy_temp<int>(); // { dg-message "in .constexpr." "" { xfail *-*-* } }
> > +constexpr bool i5 = test_parameter(int{}); // { dg-error "destroying" }
> > +constexpr bool i6 = test_bindings<int>();
> > +
> > +struct Trivial { int x; };
> > +constexpr bool t1 = test_access<Trivial>(); // { dg-message "in .constexpr." }
> > +constexpr bool t2 = test_modification<Trivial>(); // { dg-message "in .constexpr." }
> > +constexpr bool t3 = test_scope<Trivial>(); // { dg-message "in .constexpr." }
> > +constexpr bool t4 = test_destroy_temp<Trivial>(); // { dg-message "in .constexpr." }
> > +constexpr bool t5 = test_parameter(Trivial{}); // { dg-error "destroying" }
> > +constexpr bool t6 = test_bindings<Trivial>();
> > +
> > +#if __cplusplus >= 202002L
> > +struct NonTrivial { int x; constexpr ~NonTrivial() {} }; // { dg-error "destroying" "" { target c++20 } }
> > +constexpr bool n1 = test_access<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> > +constexpr bool n2 = test_modification<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> > +constexpr bool n3 = test_scope<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> > +constexpr bool n4 = test_destroy_temp<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
> > +constexpr bool n5 = test_parameter(NonTrivial{}); // { dg-error "destroying" "" { target c++20 } }
> > +constexpr bool n6 = test_bindings<NonTrivial>();
> > +#endif
> > +
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
> > index dcb424fc8f6..885d4f0e26d 100644
> > --- a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
> > +++ b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
> > @@ -13,7 +13,7 @@ template <bool V, int W>
> > struct U {
> > int j : W = 7; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
> > int k : W { 8 }; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
> > - int l : V ? 7 : a = 3; // { dg-error "modification of .a. is not a constant expression" }
> > + int l : V ? 7 : a = 3; // { dg-error "modification of .a. from outside current evaluation is not a constant expression" }
> > // { dg-error "width not an integer constant" "" { target *-*-* } .-1 }
> > int m : (V ? W : b) = 9; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
> > // { dg-error "zero width for bit-field" "" { target *-*-* } .-1 }
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
> > new file mode 100644
> > index 00000000000..36163844eca
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
> > @@ -0,0 +1,21 @@
> > +// { dg-do compile { target c++20 } }
> > +
> > +#include "construct_at.h"
> > +
> > +struct S { int x; };
> > +constexpr int f() {
> > + S s;
> > + s.~S();
> > + std::construct_at(&s, 5);
> > + return s.x;
> > +}
> > +static_assert(f() == 5);
> > +
> > +struct T { int x; constexpr ~T() {} };
> > +constexpr int g() {
> > + T t;
> > + t.~T();
> > + std::construct_at(&t, 12);
> > + return t.x;
> > +}
> > +static_assert(g() == 12);
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
> > new file mode 100644
> > index 00000000000..56cc9e3c1c8
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
> > @@ -0,0 +1,23 @@
> > +// { dg-do compile { target c++20 } }
> > +
> > +#include "construct_at.h"
> > +
> > +struct S { int x; };
> > +
> > +constexpr bool foo(S s, S*& p) {
> > + p = &s;
> > + s.~S();
> > + return true;
> > +}
> > +
> > +constexpr bool bar() {
> > + // This is, strictly speaking, implementation-defined behaviour;
> > + // see [expr.call] p6. However, in all other cases we destroy
> > + // at the end of the full-expression, so the below should be fixed.
> > + S* p;
> > + foo(S{}, p), std::construct_at(p); // { dg-bogus "destroying" "" { xfail *-*-* } }
> > +
> > + return true;
> > +}
> > +
> > +constexpr bool x = bar();
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
> > index 3ba440fec53..5d9f192507b 100644
> > --- a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
> > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
> > @@ -34,7 +34,7 @@ constexpr auto v3 = f3 (); // { dg-message "in 'constexpr' expansion of" }
> > constexpr bool
> > f4 (int *p)
> > {
> > - delete p; // { dg-error "deallocation of storage that was not previously allocated" }
> > + delete p; // { dg-error "destroying 'q' from outside current evaluation" }
> > return false;
> > }
> >
> > @@ -70,3 +70,18 @@ f7 ()
> > }
> >
> > constexpr auto v7 = f7 ();
> > +
> > +constexpr bool
> > +f8_impl (int *p)
> > +{
> > + delete p; // { dg-error "deallocation of storage that was not previously allocated" }
> > + return false;
> > +}
> > +
> > +constexpr bool
> > +f8 ()
> > +{
> > + int q = 0;
> > + return f8_impl (&q);
> > +}
> > +constexpr auto v8 = f8 (); // { dg-message "in 'constexpr' expansion of" }
> > --
> > 2.42.0
> >
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-11-03 1:18 [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093] Nathaniel Shead
2023-11-03 1:34 ` Nathaniel Shead
@ 2023-12-09 20:12 ` Jason Merrill
2023-12-10 10:22 ` Richard Biener
1 sibling, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-09 20:12 UTC (permalink / raw)
To: Nathaniel Shead, gcc-patches; +Cc: Richard Biener
On 11/2/23 21:18, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86-64_pc_linux_gnu.
>
> I'm not entirely sure if the change I made to have destructors clobber with
> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to have
> broken by doing this and I wasn't able to find anything else that really
> depended on this distinction other than a warning pass. Otherwise I could
> experiment with a new clobber kind for destructor calls.
It seems wrong to me: CLOBBER_EOL is documented to mean that the storage
is expiring at that point as well, which a (pseudo-)destructor does not
imply; it's perfectly valid to destroy an object and then create another
in the same storage.
We probably do want another clobber kind for end of object lifetime.
And/or one for beginning of object lifetime.
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-09 20:12 ` Jason Merrill
@ 2023-12-10 10:22 ` Richard Biener
2023-12-10 11:21 ` Alexander Monakov
2023-12-10 18:34 ` Jason Merrill
0 siblings, 2 replies; 29+ messages in thread
From: Richard Biener @ 2023-12-10 10:22 UTC (permalink / raw)
To: Jason Merrill; +Cc: Nathaniel Shead, gcc-patches
> Am 09.12.2023 um 21:13 schrieb Jason Merrill <jason@redhat.com>:
>
> On 11/2/23 21:18, Nathaniel Shead wrote:
>> Bootstrapped and regtested on x86-64_pc_linux_gnu.
>> I'm not entirely sure if the change I made to have destructors clobber with
>> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to have
>> broken by doing this and I wasn't able to find anything else that really
>> depended on this distinction other than a warning pass. Otherwise I could
>> experiment with a new clobber kind for destructor calls.
>
> It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is expiring at that point as well, which a (pseudo-)destructor does not imply; it's perfectly valid to destroy an object and then create another in the same storage.
>
> We probably do want another clobber kind for end of object lifetime. And/or one for beginning of object lifetime.
There’s not much semantically different between UNDEF and end of object but not storage lifetime? At least for what middle-end optimizations do.
EOL is used by stack slot sharing and that operates on the underlying storage, not individual objects live in it.
Richard
> Jason
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-10 10:22 ` Richard Biener
@ 2023-12-10 11:21 ` Alexander Monakov
2023-12-10 15:58 ` Richard Biener
2023-12-10 18:34 ` Jason Merrill
1 sibling, 1 reply; 29+ messages in thread
From: Alexander Monakov @ 2023-12-10 11:21 UTC (permalink / raw)
To: Richard Biener; +Cc: Jason Merrill, Nathaniel Shead, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
On Sun, 10 Dec 2023, Richard Biener wrote:
> > It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is
> > expiring at that point as well, which a (pseudo-)destructor does not imply;
> > it's perfectly valid to destroy an object and then create another in the
> > same storage.
> >
> > We probably do want another clobber kind for end of object lifetime. And/or
> > one for beginning of object lifetime.
>
> There’s not much semantically different between UNDEF and end of object but
> not storage lifetime? At least for what middle-end optimizations do.
>
> EOL is used by stack slot sharing and that operates on the underlying storage,
> not individual objects live in it.
I thought EOL implies that ASan may poison underlying memory. In the respin
of the Valgrind interop patch we instrument CLOBBER_UNDEF, but not CLOBBER_EOL.
Alexander
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-10 11:21 ` Alexander Monakov
@ 2023-12-10 15:58 ` Richard Biener
0 siblings, 0 replies; 29+ messages in thread
From: Richard Biener @ 2023-12-10 15:58 UTC (permalink / raw)
To: Alexander Monakov; +Cc: Jason Merrill, Nathaniel Shead, gcc-patches
> Am 10.12.2023 um 12:21 schrieb Alexander Monakov <amonakov@ispras.ru>:
>
>
> On Sun, 10 Dec 2023, Richard Biener wrote:
>
>>> It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is
>>> expiring at that point as well, which a (pseudo-)destructor does not imply;
>>> it's perfectly valid to destroy an object and then create another in the
>>> same storage.
>>>
>>> We probably do want another clobber kind for end of object lifetime. And/or
>>> one for beginning of object lifetime.
>>
>> There’s not much semantically different between UNDEF and end of object but
>> not storage lifetime? At least for what middle-end optimizations do.
>>
>> EOL is used by stack slot sharing and that operates on the underlying storage,
>> not individual objects live in it.
>
> I thought EOL implies that ASan may poison underlying memory. In the respin
> of the Valgrind interop patch we instrument CLOBBER_UNDEF, but not CLOBBER_EOL.
EOL is like free (), while UNDEF is more
Like malloc ().
Richard
> Alexander
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-10 10:22 ` Richard Biener
2023-12-10 11:21 ` Alexander Monakov
@ 2023-12-10 18:34 ` Jason Merrill
2023-12-11 8:02 ` Richard Biener
1 sibling, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-10 18:34 UTC (permalink / raw)
To: Richard Biener; +Cc: Nathaniel Shead, gcc-patches
On 12/10/23 05:22, Richard Biener wrote:
>> Am 09.12.2023 um 21:13 schrieb Jason Merrill <jason@redhat.com>:
>>
>> On 11/2/23 21:18, Nathaniel Shead wrote:
>>> Bootstrapped and regtested on x86-64_pc_linux_gnu.
>>> I'm not entirely sure if the change I made to have destructors clobber with
>>> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to have
>>> broken by doing this and I wasn't able to find anything else that really
>>> depended on this distinction other than a warning pass. Otherwise I could
>>> experiment with a new clobber kind for destructor calls.
>>
>> It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is expiring at that point as well, which a (pseudo-)destructor does not imply; it's perfectly valid to destroy an object and then create another in the same storage.
>>
>> We probably do want another clobber kind for end of object lifetime. And/or one for beginning of object lifetime.
>
> There’s not much semantically different between UNDEF and end of object but not storage lifetime? At least for what middle-end optimizations do.
That's fine for the middle-end, but Nathaniel's patch wants to
distinguish between the clobbers at beginning and end of object lifetime
in order to diagnose stores to an out-of-lifetime object in constexpr
evaluation.
One option might be to remove the clobber at the beginning of the
constructor; are there any useful optimizations enabled by that, or is
it just pedantically breaking people's code?
> EOL is used by stack slot sharing and that operates on the underlying storage, not individual objects live in it.
I wonder about changing the name to EOS (end of storage [duration]) to
avoid similar confusion with object lifetime?
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-10 18:34 ` Jason Merrill
@ 2023-12-11 8:02 ` Richard Biener
2023-12-11 19:12 ` Jason Merrill
0 siblings, 1 reply; 29+ messages in thread
From: Richard Biener @ 2023-12-11 8:02 UTC (permalink / raw)
To: Jason Merrill; +Cc: Nathaniel Shead, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]
On Sun, 10 Dec 2023, Jason Merrill wrote:
> On 12/10/23 05:22, Richard Biener wrote:
> >> Am 09.12.2023 um 21:13 schrieb Jason Merrill <jason@redhat.com>:
> >>
> >> On 11/2/23 21:18, Nathaniel Shead wrote:
> >>> Bootstrapped and regtested on x86-64_pc_linux_gnu.
> >>> I'm not entirely sure if the change I made to have destructors clobber
> >>> with
> >>> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to
> >>> have
> >>> broken by doing this and I wasn't able to find anything else that really
> >>> depended on this distinction other than a warning pass. Otherwise I could
> >>> experiment with a new clobber kind for destructor calls.
> >>
> >> It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is
> >> expiring at that point as well, which a (pseudo-)destructor does not imply;
> >> it's perfectly valid to destroy an object and then create another in the
> >> same storage.
> >>
> >> We probably do want another clobber kind for end of object lifetime. And/or
> >> one for beginning of object lifetime.
> >
> > There?s not much semantically different between UNDEF and end of object but
> > not storage lifetime? At least for what middle-end optimizations do.
>
> That's fine for the middle-end, but Nathaniel's patch wants to distinguish
> between the clobbers at beginning and end of object lifetime in order to
> diagnose stores to an out-of-lifetime object in constexpr evaluation.
Ah, I see. I did want to add CLOBBER_SOL (start-of-life) when working
on PR90348, but I always fail to finish working on that stack-slot sharing
issue. But it would be for the storage life, not object life, also
added by gimplification.
> One option might be to remove the clobber at the beginning of the constructor;
> are there any useful optimizations enabled by that, or is it just pedantically
> breaking people's code?
It's allowing DSE to the object that was live before the new one. Not
all objects require explicit destruction (which would get you a clobber)
before storage can be re-used.
> > EOL is used by stack slot sharing and that operates on the underlying
> > storage, not individual objects live in it.
>
> I wonder about changing the name to EOS (end of storage [duration]) to avoid
> similar confusion with object lifetime?
EOS{L,D}? But sure, better names (and documentation) are appreciated.
Richard.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 8:02 ` Richard Biener
@ 2023-12-11 19:12 ` Jason Merrill
2023-12-11 19:17 ` Richard Biener
0 siblings, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-11 19:12 UTC (permalink / raw)
To: Richard Biener; +Cc: Nathaniel Shead, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]
On 12/11/23 03:02, Richard Biener wrote:
> On Sun, 10 Dec 2023, Jason Merrill wrote:
>
>> On 12/10/23 05:22, Richard Biener wrote:
>>>> Am 09.12.2023 um 21:13 schrieb Jason Merrill <jason@redhat.com>:
>>>>
>>>> On 11/2/23 21:18, Nathaniel Shead wrote:
>>>>> Bootstrapped and regtested on x86-64_pc_linux_gnu.
>>>>> I'm not entirely sure if the change I made to have destructors clobber
>>>>> with
>>>>> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to
>>>>> have
>>>>> broken by doing this and I wasn't able to find anything else that really
>>>>> depended on this distinction other than a warning pass. Otherwise I could
>>>>> experiment with a new clobber kind for destructor calls.
>>>>
>>>> It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is
>>>> expiring at that point as well, which a (pseudo-)destructor does not imply;
>>>> it's perfectly valid to destroy an object and then create another in the
>>>> same storage.
>>>>
>>>> We probably do want another clobber kind for end of object lifetime. And/or
>>>> one for beginning of object lifetime.
>>>
>>> There?s not much semantically different between UNDEF and end of object but
>>> not storage lifetime? At least for what middle-end optimizations do.
>>
>> That's fine for the middle-end, but Nathaniel's patch wants to distinguish
>> between the clobbers at beginning and end of object lifetime in order to
>> diagnose stores to an out-of-lifetime object in constexpr evaluation.
>
> Ah, I see. I did want to add CLOBBER_SOL (start-of-life) when working
> on PR90348, but I always fail to finish working on that stack-slot sharing
> issue. But it would be for the storage life, not object life, also
> added by gimplification.
>
>> One option might be to remove the clobber at the beginning of the constructor;
>> are there any useful optimizations enabled by that, or is it just pedantically
>> breaking people's code?
>
> It's allowing DSE to the object that was live before the new one. Not
> all objects require explicit destruction (which would get you a clobber)
> before storage can be re-used.
>
>>> EOL is used by stack slot sharing and that operates on the underlying
>>> storage, not individual objects live in it.
>>
>> I wonder about changing the name to EOS (end of storage [duration]) to avoid
>> similar confusion with object lifetime?
>
> EOS{L,D}? But sure, better names (and documentation) are appreciated.
Maybe something like this? Or shall we write out the names like
CLOBBER_OBJECT_START, CLOBBER_STORAGE_END, etc?
[-- Attachment #2: 0001-tree-add-to-clobber_kind.patch --]
[-- Type: text/x-patch, Size: 8258 bytes --]
From 14f71a9479bd0cf4249c8c9e917a9caf3eac8c82 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Mon, 11 Dec 2023 11:35:31 -0500
Subject: [PATCH] tree: add to clobber_kind
To: gcc-patches@gcc.gnu.org
In discussion of PR71093 it came up that more clobber_kind options would be
useful within the C++ front-end.
gcc/ChangeLog:
* tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
CLOBBER_EOSD. Add CLOBBER_BOSD, CLOBBER_BOBL, CLOBBER_EOBL.
* gimple-lower-bitint.cc
* gimple-ssa-warn-access.cc
* gimplify.cc
* tree-inline.cc
* tree-pretty-print.cc
* tree-ssa-ccp.cc: Adjust for rename.
---
gcc/tree-core.h | 13 ++++++++++---
gcc/gimple-lower-bitint.cc | 8 ++++----
gcc/gimple-ssa-warn-access.cc | 2 +-
gcc/gimplify.cc | 8 ++++----
gcc/tree-inline.cc | 4 ++--
gcc/tree-pretty-print.cc | 2 +-
gcc/tree-ssa-ccp.cc | 2 +-
7 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 04c04cf2f37..bdf14605c91 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -986,12 +986,19 @@ enum annot_expr_kind {
annot_expr_kind_last
};
-/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. */
+/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. Other than _UNDEF, these are
+ in roughly sequential order. */
enum clobber_kind {
/* Unspecified, this clobber acts as a store of an undefined value. */
CLOBBER_UNDEF,
- /* This clobber ends the lifetime of the storage. */
- CLOBBER_EOL,
+ /* Beginning of storage duration, e.g. malloc. */
+ CLOBBER_BOSD,
+ /* Beginning of object lifetime, e.g. C++ constructor. */
+ CLOBBER_BOBL,
+ /* End of object lifetime, e.g. C++ destructor. */
+ CLOBBER_EOBL,
+ /* End of storage duration, e.g. free. */
+ CLOBBER_EOSD,
CLOBBER_LAST
};
diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc
index c55c32fb40d..00c3a5b20a8 100644
--- a/gcc/gimple-lower-bitint.cc
+++ b/gcc/gimple-lower-bitint.cc
@@ -806,7 +806,7 @@ bitint_large_huge::handle_operand (tree op, tree idx)
&& m_after_stmt
&& bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
{
- tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOSD);
g = gimple_build_assign (m_vars[p], clobber);
gimple_stmt_iterator gsi = gsi_for_stmt (m_after_stmt);
gsi_insert_after (&gsi, g, GSI_SAME_STMT);
@@ -2063,7 +2063,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
tree ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (atype, CLOBBER_EOL);
+ tree clobber = build_clobber (atype, CLOBBER_EOSD);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -2100,7 +2100,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (m_limb_type, CLOBBER_EOL);
+ tree clobber = build_clobber (m_limb_type, CLOBBER_EOSD);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -3707,7 +3707,7 @@ bitint_large_huge::finish_arith_overflow (tree var, tree obj, tree type,
}
if (var)
{
- tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOSD);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 1646bd1be14..e71e37214c5 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4364,7 +4364,7 @@ void
pass_waccess::check_stmt (gimple *stmt)
{
if (m_check_dangling_p
- && gimple_clobber_p (stmt, CLOBBER_EOL))
+ && gimple_clobber_p (stmt, CLOBBER_EOSD))
{
/* Ignore clobber statements in blocks with exceptional edges. */
basic_block bb = gimple_bb (stmt);
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 342e43a7f25..f7a2a4d472c 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1518,7 +1518,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
tmp = build_call_expr_loc (EXPR_LOCATION (*e), tmp, 2, v,
build_zero_cst (ptr_type_node));
tsi_link_after (&e, tmp, TSI_SAME_STMT);
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOSD);
tmp = fold_build2_loc (loc, MODIFY_EXPR, TREE_TYPE (v), v,
fold_convert (TREE_TYPE (v), tmp));
++e;
@@ -1651,7 +1651,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
build_zero_cst (ptr_type_node));
gimplify_and_add (tmp, &cleanup);
gimple *clobber_stmt;
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOSD);
clobber_stmt = gimple_build_assign (v, tmp);
gimple_set_location (clobber_stmt, end_locus);
gimplify_seq_add_stmt (&cleanup, clobber_stmt);
@@ -1665,7 +1665,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
&& !is_gimple_reg (t)
&& flag_stack_reuse != SR_NONE)
{
- tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOSD);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (t, clobber);
gimple_set_location (clobber_stmt, end_locus);
@@ -7417,7 +7417,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
{
if (flag_stack_reuse == SR_ALL)
{
- tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOSD);
clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index a4fc839a22d..e59342b1458 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -5136,7 +5136,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (*varp)
&& !(id->debug_map && id->debug_map->get (p)))
{
- tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOSD);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (*varp, clobber);
gimple_set_location (clobber_stmt, gimple_location (stmt));
@@ -5208,7 +5208,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (id->retvar)
&& !stmt_ends_bb_p (stmt))
{
- tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOSD);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (id->retvar, clobber);
gimple_set_location (clobber_stmt, gimple_location (old_stmt));
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 0dabb6d1580..f05ef7a44a3 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2624,7 +2624,7 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
if (TREE_CLOBBER_P (node))
{
pp_string (pp, "CLOBBER");
- if (CLOBBER_KIND (node) == CLOBBER_EOL)
+ if (CLOBBER_KIND (node) == CLOBBER_EOSD)
pp_string (pp, "(eol)");
}
else if (TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index ddcbaaaa417..245abaaf1cf 100644
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -2525,7 +2525,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree var,
FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
{
- clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOSD);
clobber_stmt = gimple_build_assign (var, clobber);
i = gsi_for_stmt (stmt);
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 19:12 ` Jason Merrill
@ 2023-12-11 19:17 ` Richard Biener
2023-12-11 19:21 ` Marek Polacek
0 siblings, 1 reply; 29+ messages in thread
From: Richard Biener @ 2023-12-11 19:17 UTC (permalink / raw)
To: Jason Merrill; +Cc: Nathaniel Shead, gcc-patches
> Am 11.12.2023 um 20:12 schrieb Jason Merrill <jason@redhat.com>:
>
> On 12/11/23 03:02, Richard Biener wrote:
>>> On Sun, 10 Dec 2023, Jason Merrill wrote:
>>> On 12/10/23 05:22, Richard Biener wrote:
>>>>> Am 09.12.2023 um 21:13 schrieb Jason Merrill <jason@redhat.com>:
>>>>>
>>>>> On 11/2/23 21:18, Nathaniel Shead wrote:
>>>>>> Bootstrapped and regtested on x86-64_pc_linux_gnu.
>>>>>> I'm not entirely sure if the change I made to have destructors clobber
>>>>>> with
>>>>>> CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to
>>>>>> have
>>>>>> broken by doing this and I wasn't able to find anything else that really
>>>>>> depended on this distinction other than a warning pass. Otherwise I could
>>>>>> experiment with a new clobber kind for destructor calls.
>>>>>
>>>>> It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is
>>>>> expiring at that point as well, which a (pseudo-)destructor does not imply;
>>>>> it's perfectly valid to destroy an object and then create another in the
>>>>> same storage.
>>>>>
>>>>> We probably do want another clobber kind for end of object lifetime. And/or
>>>>> one for beginning of object lifetime.
>>>>
>>>> There?s not much semantically different between UNDEF and end of object but
>>>> not storage lifetime? At least for what middle-end optimizations do.
>>>
>>> That's fine for the middle-end, but Nathaniel's patch wants to distinguish
>>> between the clobbers at beginning and end of object lifetime in order to
>>> diagnose stores to an out-of-lifetime object in constexpr evaluation.
>> Ah, I see. I did want to add CLOBBER_SOL (start-of-life) when working
>> on PR90348, but I always fail to finish working on that stack-slot sharing
>> issue. But it would be for the storage life, not object life, also
>> added by gimplification.
>>> One option might be to remove the clobber at the beginning of the constructor;
>>> are there any useful optimizations enabled by that, or is it just pedantically
>>> breaking people's code?
>> It's allowing DSE to the object that was live before the new one. Not
>> all objects require explicit destruction (which would get you a clobber)
>> before storage can be re-used.
>>>> EOL is used by stack slot sharing and that operates on the underlying
>>>> storage, not individual objects live in it.
>>>
>>> I wonder about changing the name to EOS (end of storage [duration]) to avoid
>>> similar confusion with object lifetime?
>> EOS{L,D}? But sure, better names (and documentation) are appreciated.
>
> Maybe something like this? Or shall we write out the names like CLOBBER_OBJECT_START, CLOBBER_STORAGE_END, etc?
Yeah, the abbreviations look a bit confusing so spelling it out would be better
Richard
>
> <0001-tree-add-to-clobber_kind.patch>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 19:17 ` Richard Biener
@ 2023-12-11 19:21 ` Marek Polacek
2023-12-11 22:00 ` Jason Merrill
0 siblings, 1 reply; 29+ messages in thread
From: Marek Polacek @ 2023-12-11 19:21 UTC (permalink / raw)
To: Richard Biener; +Cc: Jason Merrill, Nathaniel Shead, gcc-patches
On Mon, Dec 11, 2023 at 08:17:22PM +0100, Richard Biener wrote:
>
>
> > Am 11.12.2023 um 20:12 schrieb Jason Merrill <jason@redhat.com>:
> > Maybe something like this? Or shall we write out the names like CLOBBER_OBJECT_START, CLOBBER_STORAGE_END, etc?
>
> Yeah, the abbreviations look a bit confusing so spelling it out would be better
What about pretty-print, should we keep
pp_string (pp, "(eol)");
or use the new, more specific description?
Marek
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 19:21 ` Marek Polacek
@ 2023-12-11 22:00 ` Jason Merrill
2023-12-11 22:22 ` Marek Polacek
2023-12-11 23:03 ` Jakub Jelinek
0 siblings, 2 replies; 29+ messages in thread
From: Jason Merrill @ 2023-12-11 22:00 UTC (permalink / raw)
To: Marek Polacek, Richard Biener; +Cc: Nathaniel Shead, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 672 bytes --]
On 12/11/23 14:21, Marek Polacek wrote:
> On Mon, Dec 11, 2023 at 08:17:22PM +0100, Richard Biener wrote:
>>
>>
>>> Am 11.12.2023 um 20:12 schrieb Jason Merrill <jason@redhat.com>:
>>> Maybe something like this? Or shall we write out the names like CLOBBER_OBJECT_START, CLOBBER_STORAGE_END, etc?
>>
>> Yeah, the abbreviations look a bit confusing so spelling it out would be better
>
> What about pretty-print, should we keep
>
> pp_string (pp, "(eol)");
>
> or use the new, more specific description?
I think we tend toward terseness in pretty-print, though I don't feel
strongly about it at all.
But we should handle the other cases as well.
So, how about:
[-- Attachment #2: 0001-tree-add-to-clobber_kind.patch --]
[-- Type: text/x-patch, Size: 8821 bytes --]
From e2e535a440a5447b45769e6630cca21d274108f1 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Mon, 11 Dec 2023 11:35:31 -0500
Subject: [PATCH] tree: add to clobber_kind
To: gcc-patches@gcc.gnu.org
In discussion of PR71093 it came up that more clobber_kind options would be
useful within the C++ front-end.
gcc/ChangeLog:
* tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
* gimple-lower-bitint.cc
* gimple-ssa-warn-access.cc
* gimplify.cc
* tree-inline.cc
* tree-ssa-ccp.cc: Adjust for rename.
* tree-pretty-print.cc: And handle new values.
---
gcc/tree-core.h | 13 ++++++++++---
gcc/gimple-lower-bitint.cc | 8 ++++----
gcc/gimple-ssa-warn-access.cc | 2 +-
gcc/gimplify.cc | 8 ++++----
gcc/tree-inline.cc | 4 ++--
gcc/tree-pretty-print.cc | 19 +++++++++++++++++--
gcc/tree-ssa-ccp.cc | 2 +-
7 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 04c04cf2f37..58aa598f3bb 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -986,12 +986,19 @@ enum annot_expr_kind {
annot_expr_kind_last
};
-/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. */
+/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. Other than _UNDEF, these are
+ in roughly sequential order. */
enum clobber_kind {
/* Unspecified, this clobber acts as a store of an undefined value. */
CLOBBER_UNDEF,
- /* This clobber ends the lifetime of the storage. */
- CLOBBER_EOL,
+ /* Beginning of storage duration, e.g. malloc. */
+ CLOBBER_STORAGE_BEGIN,
+ /* Beginning of object lifetime, e.g. C++ constructor. */
+ CLOBBER_OBJECT_BEGIN,
+ /* End of object lifetime, e.g. C++ destructor. */
+ CLOBBER_OBJECT_END,
+ /* End of storage duration, e.g. free. */
+ CLOBBER_STORAGE_END,
CLOBBER_LAST
};
diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc
index c55c32fb40d..84f92b6e654 100644
--- a/gcc/gimple-lower-bitint.cc
+++ b/gcc/gimple-lower-bitint.cc
@@ -806,7 +806,7 @@ bitint_large_huge::handle_operand (tree op, tree idx)
&& m_after_stmt
&& bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
{
- tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_STORAGE_END);
g = gimple_build_assign (m_vars[p], clobber);
gimple_stmt_iterator gsi = gsi_for_stmt (m_after_stmt);
gsi_insert_after (&gsi, g, GSI_SAME_STMT);
@@ -2063,7 +2063,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
tree ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (atype, CLOBBER_EOL);
+ tree clobber = build_clobber (atype, CLOBBER_STORAGE_END);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -2100,7 +2100,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (m_limb_type, CLOBBER_EOL);
+ tree clobber = build_clobber (m_limb_type, CLOBBER_STORAGE_END);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -3707,7 +3707,7 @@ bitint_large_huge::finish_arith_overflow (tree var, tree obj, tree type,
}
if (var)
{
- tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 1646bd1be14..f04c2530869 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4364,7 +4364,7 @@ void
pass_waccess::check_stmt (gimple *stmt)
{
if (m_check_dangling_p
- && gimple_clobber_p (stmt, CLOBBER_EOL))
+ && gimple_clobber_p (stmt, CLOBBER_STORAGE_END))
{
/* Ignore clobber statements in blocks with exceptional edges. */
basic_block bb = gimple_bb (stmt);
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 342e43a7f25..ffc1882d22a 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1518,7 +1518,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
tmp = build_call_expr_loc (EXPR_LOCATION (*e), tmp, 2, v,
build_zero_cst (ptr_type_node));
tsi_link_after (&e, tmp, TSI_SAME_STMT);
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_STORAGE_END);
tmp = fold_build2_loc (loc, MODIFY_EXPR, TREE_TYPE (v), v,
fold_convert (TREE_TYPE (v), tmp));
++e;
@@ -1651,7 +1651,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
build_zero_cst (ptr_type_node));
gimplify_and_add (tmp, &cleanup);
gimple *clobber_stmt;
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_STORAGE_END);
clobber_stmt = gimple_build_assign (v, tmp);
gimple_set_location (clobber_stmt, end_locus);
gimplify_seq_add_stmt (&cleanup, clobber_stmt);
@@ -1665,7 +1665,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
&& !is_gimple_reg (t)
&& flag_stack_reuse != SR_NONE)
{
- tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_STORAGE_END);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (t, clobber);
gimple_set_location (clobber_stmt, end_locus);
@@ -7417,7 +7417,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
{
if (flag_stack_reuse == SR_ALL)
{
- tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_STORAGE_END);
clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index a4fc839a22d..cca3227fa89 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -5136,7 +5136,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (*varp)
&& !(id->debug_map && id->debug_map->get (p)))
{
- tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_STORAGE_END);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (*varp, clobber);
gimple_set_location (clobber_stmt, gimple_location (stmt));
@@ -5208,7 +5208,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (id->retvar)
&& !stmt_ends_bb_p (stmt))
{
- tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_STORAGE_END);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (id->retvar, clobber);
gimple_set_location (clobber_stmt, gimple_location (old_stmt));
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 0dabb6d1580..cab99f9dfb6 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2624,8 +2624,23 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
if (TREE_CLOBBER_P (node))
{
pp_string (pp, "CLOBBER");
- if (CLOBBER_KIND (node) == CLOBBER_EOL)
- pp_string (pp, "(eol)");
+ switch (CLOBBER_KIND (node))
+ {
+ case CLOBBER_STORAGE_BEGIN:
+ pp_string (pp, "(bos)");
+ break;
+ case CLOBBER_STORAGE_END:
+ pp_string (pp, "(eos)");
+ break;
+ case CLOBBER_OBJECT_BEGIN:
+ pp_string (pp, "(bob)");
+ break;
+ case CLOBBER_OBJECT_END:
+ pp_string (pp, "(eob)");
+ break;
+ default:
+ break;
+ }
}
else if (TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
|| TREE_CODE (TREE_TYPE (node)) == UNION_TYPE)
diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index ddcbaaaa417..fab2a9b248a 100644
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -2525,7 +2525,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree var,
FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
{
- clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
clobber_stmt = gimple_build_assign (var, clobber);
i = gsi_for_stmt (stmt);
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 22:00 ` Jason Merrill
@ 2023-12-11 22:22 ` Marek Polacek
2023-12-11 23:03 ` Jakub Jelinek
1 sibling, 0 replies; 29+ messages in thread
From: Marek Polacek @ 2023-12-11 22:22 UTC (permalink / raw)
To: Jason Merrill; +Cc: Richard Biener, Nathaniel Shead, gcc-patches
On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
> On 12/11/23 14:21, Marek Polacek wrote:
> > On Mon, Dec 11, 2023 at 08:17:22PM +0100, Richard Biener wrote:
> > >
> > >
> > > > Am 11.12.2023 um 20:12 schrieb Jason Merrill <jason@redhat.com>:
> > > > Maybe something like this? Or shall we write out the names like CLOBBER_OBJECT_START, CLOBBER_STORAGE_END, etc?
> > >
> > > Yeah, the abbreviations look a bit confusing so spelling it out would be better
> >
> > What about pretty-print, should we keep
> >
> > pp_string (pp, "(eol)");
> >
> > or use the new, more specific description?
>
> I think we tend toward terseness in pretty-print, though I don't feel
> strongly about it at all.
>
> But we should handle the other cases as well.
Nice. I think you're going to have to adjust gcc.dg/pr87052.c because
that checks for "(eol)".
> So, how about:
Obviously I can't approve but, FWIW, it looks good. I don't see anything
in doc/ that needs updating. Thanks.
> From e2e535a440a5447b45769e6630cca21d274108f1 Mon Sep 17 00:00:00 2001
> From: Jason Merrill <jason@redhat.com>
> Date: Mon, 11 Dec 2023 11:35:31 -0500
> Subject: [PATCH] tree: add to clobber_kind
> To: gcc-patches@gcc.gnu.org
>
> In discussion of PR71093 it came up that more clobber_kind options would be
> useful within the C++ front-end.
>
> gcc/ChangeLog:
>
> * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
> CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
> CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
> * gimple-lower-bitint.cc
> * gimple-ssa-warn-access.cc
> * gimplify.cc
> * tree-inline.cc
> * tree-ssa-ccp.cc: Adjust for rename.
> * tree-pretty-print.cc: And handle new values.
> ---
> gcc/tree-core.h | 13 ++++++++++---
> gcc/gimple-lower-bitint.cc | 8 ++++----
> gcc/gimple-ssa-warn-access.cc | 2 +-
> gcc/gimplify.cc | 8 ++++----
> gcc/tree-inline.cc | 4 ++--
> gcc/tree-pretty-print.cc | 19 +++++++++++++++++--
> gcc/tree-ssa-ccp.cc | 2 +-
> 7 files changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index 04c04cf2f37..58aa598f3bb 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -986,12 +986,19 @@ enum annot_expr_kind {
> annot_expr_kind_last
> };
>
> -/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. */
> +/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. Other than _UNDEF, these are
> + in roughly sequential order. */
> enum clobber_kind {
> /* Unspecified, this clobber acts as a store of an undefined value. */
> CLOBBER_UNDEF,
> - /* This clobber ends the lifetime of the storage. */
> - CLOBBER_EOL,
> + /* Beginning of storage duration, e.g. malloc. */
> + CLOBBER_STORAGE_BEGIN,
> + /* Beginning of object lifetime, e.g. C++ constructor. */
> + CLOBBER_OBJECT_BEGIN,
> + /* End of object lifetime, e.g. C++ destructor. */
> + CLOBBER_OBJECT_END,
> + /* End of storage duration, e.g. free. */
> + CLOBBER_STORAGE_END,
> CLOBBER_LAST
> };
>
> diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc
> index c55c32fb40d..84f92b6e654 100644
> --- a/gcc/gimple-lower-bitint.cc
> +++ b/gcc/gimple-lower-bitint.cc
> @@ -806,7 +806,7 @@ bitint_large_huge::handle_operand (tree op, tree idx)
> && m_after_stmt
> && bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
> {
> - tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_STORAGE_END);
> g = gimple_build_assign (m_vars[p], clobber);
> gimple_stmt_iterator gsi = gsi_for_stmt (m_after_stmt);
> gsi_insert_after (&gsi, g, GSI_SAME_STMT);
> @@ -2063,7 +2063,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
> tree ret = build_fold_addr_expr (var);
> if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
> {
> - tree clobber = build_clobber (atype, CLOBBER_EOL);
> + tree clobber = build_clobber (atype, CLOBBER_STORAGE_END);
> g = gimple_build_assign (var, clobber);
> gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
> }
> @@ -2100,7 +2100,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
> ret = build_fold_addr_expr (var);
> if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
> {
> - tree clobber = build_clobber (m_limb_type, CLOBBER_EOL);
> + tree clobber = build_clobber (m_limb_type, CLOBBER_STORAGE_END);
> g = gimple_build_assign (var, clobber);
> gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
> }
> @@ -3707,7 +3707,7 @@ bitint_large_huge::finish_arith_overflow (tree var, tree obj, tree type,
> }
> if (var)
> {
> - tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
> g = gimple_build_assign (var, clobber);
> gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
> }
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index 1646bd1be14..f04c2530869 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -4364,7 +4364,7 @@ void
> pass_waccess::check_stmt (gimple *stmt)
> {
> if (m_check_dangling_p
> - && gimple_clobber_p (stmt, CLOBBER_EOL))
> + && gimple_clobber_p (stmt, CLOBBER_STORAGE_END))
> {
> /* Ignore clobber statements in blocks with exceptional edges. */
> basic_block bb = gimple_bb (stmt);
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index 342e43a7f25..ffc1882d22a 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -1518,7 +1518,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
> tmp = build_call_expr_loc (EXPR_LOCATION (*e), tmp, 2, v,
> build_zero_cst (ptr_type_node));
> tsi_link_after (&e, tmp, TSI_SAME_STMT);
> - tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
> + tmp = build_clobber (TREE_TYPE (v), CLOBBER_STORAGE_END);
> tmp = fold_build2_loc (loc, MODIFY_EXPR, TREE_TYPE (v), v,
> fold_convert (TREE_TYPE (v), tmp));
> ++e;
> @@ -1651,7 +1651,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
> build_zero_cst (ptr_type_node));
> gimplify_and_add (tmp, &cleanup);
> gimple *clobber_stmt;
> - tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
> + tmp = build_clobber (TREE_TYPE (v), CLOBBER_STORAGE_END);
> clobber_stmt = gimple_build_assign (v, tmp);
> gimple_set_location (clobber_stmt, end_locus);
> gimplify_seq_add_stmt (&cleanup, clobber_stmt);
> @@ -1665,7 +1665,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
> && !is_gimple_reg (t)
> && flag_stack_reuse != SR_NONE)
> {
> - tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_STORAGE_END);
> gimple *clobber_stmt;
> clobber_stmt = gimple_build_assign (t, clobber);
> gimple_set_location (clobber_stmt, end_locus);
> @@ -7417,7 +7417,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> {
> if (flag_stack_reuse == SR_ALL)
> {
> - tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_STORAGE_END);
> clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
> gimple_push_cleanup (temp, clobber, false, pre_p, true);
> }
> diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> index a4fc839a22d..cca3227fa89 100644
> --- a/gcc/tree-inline.cc
> +++ b/gcc/tree-inline.cc
> @@ -5136,7 +5136,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
> && !is_gimple_reg (*varp)
> && !(id->debug_map && id->debug_map->get (p)))
> {
> - tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_STORAGE_END);
> gimple *clobber_stmt;
> clobber_stmt = gimple_build_assign (*varp, clobber);
> gimple_set_location (clobber_stmt, gimple_location (stmt));
> @@ -5208,7 +5208,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
> && !is_gimple_reg (id->retvar)
> && !stmt_ends_bb_p (stmt))
> {
> - tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_STORAGE_END);
> gimple *clobber_stmt;
> clobber_stmt = gimple_build_assign (id->retvar, clobber);
> gimple_set_location (clobber_stmt, gimple_location (old_stmt));
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index 0dabb6d1580..cab99f9dfb6 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -2624,8 +2624,23 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
> if (TREE_CLOBBER_P (node))
> {
> pp_string (pp, "CLOBBER");
> - if (CLOBBER_KIND (node) == CLOBBER_EOL)
> - pp_string (pp, "(eol)");
> + switch (CLOBBER_KIND (node))
> + {
> + case CLOBBER_STORAGE_BEGIN:
> + pp_string (pp, "(bos)");
> + break;
> + case CLOBBER_STORAGE_END:
> + pp_string (pp, "(eos)");
> + break;
> + case CLOBBER_OBJECT_BEGIN:
> + pp_string (pp, "(bob)");
> + break;
> + case CLOBBER_OBJECT_END:
> + pp_string (pp, "(eob)");
> + break;
> + default:
> + break;
> + }
> }
> else if (TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
> || TREE_CODE (TREE_TYPE (node)) == UNION_TYPE)
> diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
> index ddcbaaaa417..fab2a9b248a 100644
> --- a/gcc/tree-ssa-ccp.cc
> +++ b/gcc/tree-ssa-ccp.cc
> @@ -2525,7 +2525,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree var,
> FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
> if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
> {
> - clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
> + clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
> clobber_stmt = gimple_build_assign (var, clobber);
>
> i = gsi_for_stmt (stmt);
> --
> 2.39.3
>
Marek
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 22:00 ` Jason Merrill
2023-12-11 22:22 ` Marek Polacek
@ 2023-12-11 23:03 ` Jakub Jelinek
2023-12-12 11:13 ` Alexander Monakov
1 sibling, 1 reply; 29+ messages in thread
From: Jakub Jelinek @ 2023-12-11 23:03 UTC (permalink / raw)
To: Jason Merrill; +Cc: Marek Polacek, Richard Biener, Nathaniel Shead, gcc-patches
On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
> In discussion of PR71093 it came up that more clobber_kind options would be
> useful within the C++ front-end.
>
> gcc/ChangeLog:
>
> * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
> CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
> CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
> * gimple-lower-bitint.cc
> * gimple-ssa-warn-access.cc
> * gimplify.cc
> * tree-inline.cc
> * tree-ssa-ccp.cc: Adjust for rename.
I doubt the above style will make it through the pre-commit hook, but I
might be wrong.
> --- a/gcc/gimple-lower-bitint.cc
> +++ b/gcc/gimple-lower-bitint.cc
> @@ -806,7 +806,7 @@ bitint_large_huge::handle_operand (tree op, tree idx)
> && m_after_stmt
> && bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
> {
> - tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_STORAGE_END);
This needs line wrapping I think.
> @@ -2100,7 +2100,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
> ret = build_fold_addr_expr (var);
> if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
> {
> - tree clobber = build_clobber (m_limb_type, CLOBBER_EOL);
> + tree clobber = build_clobber (m_limb_type, CLOBBER_STORAGE_END);
This too.
> && flag_stack_reuse != SR_NONE)
> {
> - tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_STORAGE_END);
> gimple *clobber_stmt;
> clobber_stmt = gimple_build_assign (t, clobber);
> gimple_set_location (clobber_stmt, end_locus);
> @@ -7417,7 +7417,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> {
> if (flag_stack_reuse == SR_ALL)
> {
> - tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_STORAGE_END);
> clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
> gimple_push_cleanup (temp, clobber, false, pre_p, true);
> }
These too.
> diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> index a4fc839a22d..cca3227fa89 100644
> --- a/gcc/tree-inline.cc
> +++ b/gcc/tree-inline.cc
> @@ -5136,7 +5136,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
> && !is_gimple_reg (*varp)
> && !(id->debug_map && id->debug_map->get (p)))
> {
> - tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_STORAGE_END);
> gimple *clobber_stmt;
> clobber_stmt = gimple_build_assign (*varp, clobber);
> gimple_set_location (clobber_stmt, gimple_location (stmt));
> @@ -5208,7 +5208,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
> && !is_gimple_reg (id->retvar)
> && !stmt_ends_bb_p (stmt))
> {
> - tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
> + tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_STORAGE_END);
> gimple *clobber_stmt;
> clobber_stmt = gimple_build_assign (id->retvar, clobber);
> gimple_set_location (clobber_stmt, gimple_location (old_stmt));
And these.
Otherwise LGTM.
Jakub
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-11 23:03 ` Jakub Jelinek
@ 2023-12-12 11:13 ` Alexander Monakov
2023-12-12 11:15 ` Jakub Jelinek
0 siblings, 1 reply; 29+ messages in thread
From: Alexander Monakov @ 2023-12-12 11:13 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Jason Merrill, Marek Polacek, Richard Biener, Nathaniel Shead,
gcc-patches
On Tue, 12 Dec 2023, Jakub Jelinek wrote:
> On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
> > In discussion of PR71093 it came up that more clobber_kind options would be
> > useful within the C++ front-end.
> >
> > gcc/ChangeLog:
> >
> > * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
> > CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
> > CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
> > * gimple-lower-bitint.cc
> > * gimple-ssa-warn-access.cc
> > * gimplify.cc
> > * tree-inline.cc
> > * tree-ssa-ccp.cc: Adjust for rename.
Doesn't build_clobber_this in the C++ front-end need to be adjusted too?
I think it is used to place clobbers at start of the ctor (should be
CLOBBER_OBJECT_BEGIN in the new nomenclature) and end of the dtor (i.e.
CLOBBER_OBJECT_END).
Alexander
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-12 11:13 ` Alexander Monakov
@ 2023-12-12 11:15 ` Jakub Jelinek
2023-12-12 15:24 ` Jason Merrill
0 siblings, 1 reply; 29+ messages in thread
From: Jakub Jelinek @ 2023-12-12 11:15 UTC (permalink / raw)
To: Alexander Monakov
Cc: Jason Merrill, Marek Polacek, Richard Biener, Nathaniel Shead,
gcc-patches
On Tue, Dec 12, 2023 at 02:13:43PM +0300, Alexander Monakov wrote:
>
>
> On Tue, 12 Dec 2023, Jakub Jelinek wrote:
>
> > On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
> > > In discussion of PR71093 it came up that more clobber_kind options would be
> > > useful within the C++ front-end.
> > >
> > > gcc/ChangeLog:
> > >
> > > * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
> > > CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
> > > CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
> > > * gimple-lower-bitint.cc
> > > * gimple-ssa-warn-access.cc
> > > * gimplify.cc
> > > * tree-inline.cc
> > > * tree-ssa-ccp.cc: Adjust for rename.
>
> Doesn't build_clobber_this in the C++ front-end need to be adjusted too?
> I think it is used to place clobbers at start of the ctor (should be
> CLOBBER_OBJECT_BEGIN in the new nomenclature) and end of the dtor (i.e.
> CLOBBER_OBJECT_END).
You're right.
Jakub
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-12 11:15 ` Jakub Jelinek
@ 2023-12-12 15:24 ` Jason Merrill
2023-12-12 17:50 ` Jason Merrill
0 siblings, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-12 15:24 UTC (permalink / raw)
To: Jakub Jelinek, Alexander Monakov
Cc: Marek Polacek, Richard Biener, Nathaniel Shead, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]
On 12/12/23 06:15, Jakub Jelinek wrote:
> On Tue, Dec 12, 2023 at 02:13:43PM +0300, Alexander Monakov wrote:
>>
>>
>> On Tue, 12 Dec 2023, Jakub Jelinek wrote:
>>
>>> On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
>>>> In discussion of PR71093 it came up that more clobber_kind options would be
>>>> useful within the C++ front-end.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
>>>> CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
>>>> CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
>>>> * gimple-lower-bitint.cc
>>>> * gimple-ssa-warn-access.cc
>>>> * gimplify.cc
>>>> * tree-inline.cc
>>>> * tree-ssa-ccp.cc: Adjust for rename.
>>
>> Doesn't build_clobber_this in the C++ front-end need to be adjusted too?
>> I think it is used to place clobbers at start of the ctor (should be
>> CLOBBER_OBJECT_BEGIN in the new nomenclature) and end of the dtor (i.e.
>> CLOBBER_OBJECT_END).
>
> You're right.
I had been thinking to leave that to Nathaniel's patch, but sure, I'll
hoist those bits out:
[-- Attachment #2: 0001-tree-add-to-clobber_kind.patch --]
[-- Type: text/x-patch, Size: 12198 bytes --]
From 29b54f1e2a832f74bdbdba738991d3330b0b4577 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Mon, 11 Dec 2023 11:35:31 -0500
Subject: [PATCH] tree: add to clobber_kind
To: gcc-patches@gcc.gnu.org
In discussion of PR71093 it came up that more clobber_kind options would be
useful within the C++ front-end.
gcc/ChangeLog:
* tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
* gimple-lower-bitint.cc
* gimple-ssa-warn-access.cc
* gimplify.cc
* tree-inline.cc
* tree-ssa-ccp.cc: Adjust for rename.
* tree-pretty-print.cc: And handle new values.
gcc/cp/ChangeLog:
* call.cc (build_trivial_dtor_call): Use CLOBBER_OBJECT_END.
* decl.cc (build_clobber_this): Take clobber_kind argument.
(start_preparsed_function): Pass CLOBBER_OBJECT_BEGIN.
(begin_destructor_body): Pass CLOBBER_OBJECT_END.
gcc/testsuite/ChangeLog:
* gcc.dg/pr87052.c: Adjust expected CLOBBER output.
Co-authored-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/tree-core.h | 13 ++++++++++---
gcc/cp/call.cc | 2 +-
gcc/cp/decl.cc | 9 +++++----
gcc/gimple-lower-bitint.cc | 10 ++++++----
gcc/gimple-ssa-warn-access.cc | 2 +-
gcc/gimplify.cc | 9 +++++----
gcc/testsuite/gcc.dg/pr87052.c | 4 ++--
gcc/tree-inline.cc | 6 ++++--
gcc/tree-pretty-print.cc | 19 +++++++++++++++++--
gcc/tree-ssa-ccp.cc | 2 +-
10 files changed, 52 insertions(+), 24 deletions(-)
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 04c04cf2f37..58aa598f3bb 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -986,12 +986,19 @@ enum annot_expr_kind {
annot_expr_kind_last
};
-/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. */
+/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. Other than _UNDEF, these are
+ in roughly sequential order. */
enum clobber_kind {
/* Unspecified, this clobber acts as a store of an undefined value. */
CLOBBER_UNDEF,
- /* This clobber ends the lifetime of the storage. */
- CLOBBER_EOL,
+ /* Beginning of storage duration, e.g. malloc. */
+ CLOBBER_STORAGE_BEGIN,
+ /* Beginning of object lifetime, e.g. C++ constructor. */
+ CLOBBER_OBJECT_BEGIN,
+ /* End of object lifetime, e.g. C++ destructor. */
+ CLOBBER_OBJECT_END,
+ /* End of storage duration, e.g. free. */
+ CLOBBER_STORAGE_END,
CLOBBER_LAST
};
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 4f0abf8e93f..aaee34f35b0 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9716,7 +9716,7 @@ build_trivial_dtor_call (tree instance, bool no_ptr_deref)
}
/* A trivial destructor should still clobber the object. */
- tree clobber = build_clobber (TREE_TYPE (instance));
+ tree clobber = build_clobber (TREE_TYPE (instance), CLOBBER_OBJECT_END);
return build2 (MODIFY_EXPR, void_type_node,
instance, clobber);
}
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index b1ada1d5215..4d17ead123a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17401,7 +17401,7 @@ implicit_default_ctor_p (tree fn)
storage is dead when we enter the constructor or leave the destructor. */
static tree
-build_clobber_this ()
+build_clobber_this (clobber_kind kind)
{
/* Clobbering an empty base is pointless, and harmful if its one byte
TYPE_SIZE overlays real data. */
@@ -17417,7 +17417,7 @@ build_clobber_this ()
if (!vbases)
ctype = CLASSTYPE_AS_BASE (ctype);
- tree clobber = build_clobber (ctype);
+ tree clobber = build_clobber (ctype, kind);
tree thisref = current_class_ref;
if (ctype != current_class_type)
@@ -17836,7 +17836,7 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
because part of the initialization might happen before we enter the
constructor, via AGGR_INIT_ZERO_FIRST (c++/68006). */
&& !implicit_default_ctor_p (decl1))
- finish_expr_stmt (build_clobber_this ());
+ finish_expr_stmt (build_clobber_this (CLOBBER_OBJECT_BEGIN));
if (!processing_template_decl
&& DECL_CONSTRUCTOR_P (decl1)
@@ -18074,7 +18074,8 @@ begin_destructor_body (void)
finish_decl_cleanup (NULL_TREE, stmt);
}
else
- finish_decl_cleanup (NULL_TREE, build_clobber_this ());
+ finish_decl_cleanup (NULL_TREE,
+ build_clobber_this (CLOBBER_OBJECT_END));
}
/* And insert cleanups for our bases and members so that they
diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc
index c55c32fb40d..65a7bbe3fa9 100644
--- a/gcc/gimple-lower-bitint.cc
+++ b/gcc/gimple-lower-bitint.cc
@@ -806,7 +806,8 @@ bitint_large_huge::handle_operand (tree op, tree idx)
&& m_after_stmt
&& bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
{
- tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (m_vars[p]),
+ CLOBBER_STORAGE_END);
g = gimple_build_assign (m_vars[p], clobber);
gimple_stmt_iterator gsi = gsi_for_stmt (m_after_stmt);
gsi_insert_after (&gsi, g, GSI_SAME_STMT);
@@ -2063,7 +2064,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
tree ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (atype, CLOBBER_EOL);
+ tree clobber = build_clobber (atype, CLOBBER_STORAGE_END);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -2100,7 +2101,8 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (m_limb_type, CLOBBER_EOL);
+ tree clobber = build_clobber (m_limb_type,
+ CLOBBER_STORAGE_END);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -3707,7 +3709,7 @@ bitint_large_huge::finish_arith_overflow (tree var, tree obj, tree type,
}
if (var)
{
- tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 1646bd1be14..f04c2530869 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4364,7 +4364,7 @@ void
pass_waccess::check_stmt (gimple *stmt)
{
if (m_check_dangling_p
- && gimple_clobber_p (stmt, CLOBBER_EOL))
+ && gimple_clobber_p (stmt, CLOBBER_STORAGE_END))
{
/* Ignore clobber statements in blocks with exceptional edges. */
basic_block bb = gimple_bb (stmt);
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 342e43a7f25..afeaea873c0 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1518,7 +1518,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
tmp = build_call_expr_loc (EXPR_LOCATION (*e), tmp, 2, v,
build_zero_cst (ptr_type_node));
tsi_link_after (&e, tmp, TSI_SAME_STMT);
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_STORAGE_END);
tmp = fold_build2_loc (loc, MODIFY_EXPR, TREE_TYPE (v), v,
fold_convert (TREE_TYPE (v), tmp));
++e;
@@ -1651,7 +1651,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
build_zero_cst (ptr_type_node));
gimplify_and_add (tmp, &cleanup);
gimple *clobber_stmt;
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_STORAGE_END);
clobber_stmt = gimple_build_assign (v, tmp);
gimple_set_location (clobber_stmt, end_locus);
gimplify_seq_add_stmt (&cleanup, clobber_stmt);
@@ -1665,7 +1665,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
&& !is_gimple_reg (t)
&& flag_stack_reuse != SR_NONE)
{
- tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_STORAGE_END);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (t, clobber);
gimple_set_location (clobber_stmt, end_locus);
@@ -7417,7 +7417,8 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
{
if (flag_stack_reuse == SR_ALL)
{
- tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (temp),
+ CLOBBER_STORAGE_END);
clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
diff --git a/gcc/testsuite/gcc.dg/pr87052.c b/gcc/testsuite/gcc.dg/pr87052.c
index 796fe6440c1..90f3c3b14a8 100644
--- a/gcc/testsuite/gcc.dg/pr87052.c
+++ b/gcc/testsuite/gcc.dg/pr87052.c
@@ -36,6 +36,6 @@ void test (void)
{ dg-final { scan-tree-dump-times "b = \"a\\\\x00bc\";" 1 "gimple" } }
{ dg-final { scan-tree-dump-times "c = \"\";" 1 "gimple" } }
{ dg-final { scan-tree-dump-times "d = " 1 "gimple" } }
- { dg-final { scan-tree-dump-times "d = {CLOBBER\\(eol\\)}" 1 "gimple" } }
+ { dg-final { scan-tree-dump-times "d = {CLOBBER\\(eos\\)}" 1 "gimple" } }
{ dg-final { scan-tree-dump-times "e = " 1 "gimple" } }
- { dg-final { scan-tree-dump-times "e = {CLOBBER\\(eol\\)}" 1 "gimple" } } */
+ { dg-final { scan-tree-dump-times "e = {CLOBBER\\(eos\\)}" 1 "gimple" } } */
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index a4fc839a22d..137b83b7c83 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -5136,7 +5136,8 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (*varp)
&& !(id->debug_map && id->debug_map->get (p)))
{
- tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (*varp),
+ CLOBBER_STORAGE_END);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (*varp, clobber);
gimple_set_location (clobber_stmt, gimple_location (stmt));
@@ -5208,7 +5209,8 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (id->retvar)
&& !stmt_ends_bb_p (stmt))
{
- tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (id->retvar),
+ CLOBBER_STORAGE_END);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (id->retvar, clobber);
gimple_set_location (clobber_stmt, gimple_location (old_stmt));
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 0dabb6d1580..cab99f9dfb6 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2624,8 +2624,23 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
if (TREE_CLOBBER_P (node))
{
pp_string (pp, "CLOBBER");
- if (CLOBBER_KIND (node) == CLOBBER_EOL)
- pp_string (pp, "(eol)");
+ switch (CLOBBER_KIND (node))
+ {
+ case CLOBBER_STORAGE_BEGIN:
+ pp_string (pp, "(bos)");
+ break;
+ case CLOBBER_STORAGE_END:
+ pp_string (pp, "(eos)");
+ break;
+ case CLOBBER_OBJECT_BEGIN:
+ pp_string (pp, "(bob)");
+ break;
+ case CLOBBER_OBJECT_END:
+ pp_string (pp, "(eob)");
+ break;
+ default:
+ break;
+ }
}
else if (TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
|| TREE_CODE (TREE_TYPE (node)) == UNION_TYPE)
diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index ddcbaaaa417..fab2a9b248a 100644
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -2525,7 +2525,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree var,
FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
{
- clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
clobber_stmt = gimple_build_assign (var, clobber);
i = gsi_for_stmt (stmt);
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-12 15:24 ` Jason Merrill
@ 2023-12-12 17:50 ` Jason Merrill
2023-12-13 4:40 ` Jason Merrill
0 siblings, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-12 17:50 UTC (permalink / raw)
To: Nathaniel Shead
Cc: Marek Polacek, Richard Biener, gcc-patches, Alexander Monakov,
Jakub Jelinek
On 12/12/23 10:24, Jason Merrill wrote:
> On 12/12/23 06:15, Jakub Jelinek wrote:
>> On Tue, Dec 12, 2023 at 02:13:43PM +0300, Alexander Monakov wrote:
>>>
>>>
>>> On Tue, 12 Dec 2023, Jakub Jelinek wrote:
>>>
>>>> On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
>>>>> In discussion of PR71093 it came up that more clobber_kind options
>>>>> would be
>>>>> useful within the C++ front-end.
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
>>>>> CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
>>>>> CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
>>>>> * gimple-lower-bitint.cc
>>>>> * gimple-ssa-warn-access.cc
>>>>> * gimplify.cc
>>>>> * tree-inline.cc
>>>>> * tree-ssa-ccp.cc: Adjust for rename.
>>>
>>> Doesn't build_clobber_this in the C++ front-end need to be adjusted too?
>>> I think it is used to place clobbers at start of the ctor (should be
>>> CLOBBER_OBJECT_BEGIN in the new nomenclature) and end of the dtor (i.e.
>>> CLOBBER_OBJECT_END).
>>
>> You're right.
>
> I had been thinking to leave that to Nathaniel's patch, but sure, I'll
> hoist those bits out:
I've now pushed this version of the patch; Nathaniel, do you want to
rebase on it?
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-12 17:50 ` Jason Merrill
@ 2023-12-13 4:40 ` Jason Merrill
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
0 siblings, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-13 4:40 UTC (permalink / raw)
To: Nathaniel Shead
Cc: Marek Polacek, Richard Biener, gcc-patches, Alexander Monakov,
Jakub Jelinek
On 12/12/23 12:50, Jason Merrill wrote:
> On 12/12/23 10:24, Jason Merrill wrote:
>> On 12/12/23 06:15, Jakub Jelinek wrote:
>>> On Tue, Dec 12, 2023 at 02:13:43PM +0300, Alexander Monakov wrote:
>>>>
>>>>
>>>> On Tue, 12 Dec 2023, Jakub Jelinek wrote:
>>>>
>>>>> On Mon, Dec 11, 2023 at 05:00:50PM -0500, Jason Merrill wrote:
>>>>>> In discussion of PR71093 it came up that more clobber_kind options
>>>>>> would be
>>>>>> useful within the C++ front-end.
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> * tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
>>>>>> CLOBBER_STORAGE_END. Add CLOBBER_STORAGE_BEGIN,
>>>>>> CLOBBER_OBJECT_BEGIN, CLOBBER_OBJECT_END.
>>>>>> * gimple-lower-bitint.cc
>>>>>> * gimple-ssa-warn-access.cc
>>>>>> * gimplify.cc
>>>>>> * tree-inline.cc
>>>>>> * tree-ssa-ccp.cc: Adjust for rename.
>>>>
>>>> Doesn't build_clobber_this in the C++ front-end need to be adjusted
>>>> too?
>>>> I think it is used to place clobbers at start of the ctor (should be
>>>> CLOBBER_OBJECT_BEGIN in the new nomenclature) and end of the dtor (i.e.
>>>> CLOBBER_OBJECT_END).
>>>
>>> You're right.
>>
>> I had been thinking to leave that to Nathaniel's patch, but sure, I'll
>> hoist those bits out:
>
> I've now pushed this version of the patch; Nathaniel, do you want to
> rebase on it?
Actually, I'll take care of that.
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* [pushed 1/4] c++: copy location to AGGR_INIT_EXPR
2023-12-13 4:40 ` Jason Merrill
@ 2023-12-13 16:47 ` Jason Merrill
2023-12-13 16:47 ` [pushed 2/4] c++: constant direct-initialization [PR108243] Jason Merrill
` (4 more replies)
0 siblings, 5 replies; 29+ messages in thread
From: Jason Merrill @ 2023-12-13 16:47 UTC (permalink / raw)
To: gcc-patches
Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
When building an AGGR_INIT_EXPR from a CALL_EXPR, we shouldn't lose location
information.
gcc/cp/ChangeLog:
* tree.cc (build_aggr_init_expr): Copy EXPR_LOCATION.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1y/constexpr-nsdmi7b.C: Adjust line.
* g++.dg/template/copy1.C: Likewise.
---
gcc/cp/tree.cc | 1 +
gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C | 4 ++--
gcc/testsuite/g++.dg/template/copy1.C | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index da4d5c51f07..c4e41fd7b5c 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -689,6 +689,7 @@ build_aggr_init_expr (tree type, tree init)
CALL_EXPR_OPERATOR_SYNTAX (rval) = CALL_EXPR_OPERATOR_SYNTAX (init);
CALL_EXPR_ORDERED_ARGS (rval) = CALL_EXPR_ORDERED_ARGS (init);
CALL_EXPR_REVERSE_ARGS (rval) = CALL_EXPR_REVERSE_ARGS (init);
+ SET_EXPR_LOCATION (rval, EXPR_LOCATION (init));
}
else
rval = init;
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
index a410e482664..586ee54124c 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
@@ -20,8 +20,8 @@ bar()
{
A a = foo();
a.p->n = 5;
- return a;
-} // { dg-error "non-.constexpr." "" { target c++20_down } }
+ return a; // { dg-error "non-.constexpr." "" { target c++20_down } }
+}
constexpr int
baz()
diff --git a/gcc/testsuite/g++.dg/template/copy1.C b/gcc/testsuite/g++.dg/template/copy1.C
index eacd9e2c025..7e0a3805a77 100644
--- a/gcc/testsuite/g++.dg/template/copy1.C
+++ b/gcc/testsuite/g++.dg/template/copy1.C
@@ -6,10 +6,10 @@
struct A
{
- // { dg-error "reference" "" { target c++14_down } .+1 }
A(A&); // { dg-message "A::A" "" { target c++14_down } }
template <class T> A(T); // { dg-message "A::A" "" { target c++14_down } }
};
+// { dg-error "reference" "" { target c++14_down } .+1 }
A a = 0; // { dg-error "no match" "" { target c++14_down } }
base-commit: d2b269ce30d77dbfc6c28c75887c330d4698b132
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [pushed 2/4] c++: constant direct-initialization [PR108243]
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
@ 2023-12-13 16:47 ` Jason Merrill
2023-12-13 16:47 ` [pushed 3/4] c++: fix in-charge parm in constexpr Jason Merrill
` (3 subsequent siblings)
4 siblings, 0 replies; 29+ messages in thread
From: Jason Merrill @ 2023-12-13 16:47 UTC (permalink / raw)
To: gcc-patches
Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
When testing the proposed patch for PR71093 I noticed that it changed the
diagnostic for consteval-prop6.C. I then noticed that the diagnostic wasn't
very helpful either way; it was complaining about modification of the 'x'
variable, but it's not a problem to initialize a local variable with a
consteval constructor as long as the value is actually constant, we want to
know why the value isn't constant. And then it turned out that this also
fixed a missed-optimization bug in the testsuite.
PR c++/108243
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_outermost_constant_expr): Turn
a constructor CALL_EXPR into a TARGET_EXPR.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/consteval-prop6.C: Adjust diagnostic.
* g++.dg/opt/is_constant_evaluated3.C: Remove xfails.
---
gcc/cp/constexpr.cc | 16 +++++++++++++++-
gcc/testsuite/g++.dg/cpp2a/consteval-prop6.C | 2 +-
.../g++.dg/opt/is_constant_evaluated3.C | 8 ++++----
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 58187a4fd12..4cf9dd71b05 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8651,7 +8651,21 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant,
}
if (!object)
{
- if (TREE_CODE (t) == TARGET_EXPR)
+ if (TREE_CODE (t) == CALL_EXPR)
+ {
+ /* If T is calling a constructor to initialize an object, reframe
+ it as an AGGR_INIT_EXPR to avoid trying to modify an object
+ from outside the constant evaluation, which will fail even if
+ the value is actually constant (is_constant_evaluated3.C). */
+ tree fn = cp_get_callee_fndecl_nofold (t);
+ if (fn && DECL_CONSTRUCTOR_P (fn))
+ {
+ object = CALL_EXPR_ARG (t, 0);
+ object = build_fold_indirect_ref (object);
+ r = build_aggr_init_expr (type, r);
+ }
+ }
+ else if (TREE_CODE (t) == TARGET_EXPR)
object = TARGET_EXPR_SLOT (t);
else if (TREE_CODE (t) == AGGR_INIT_EXPR)
object = AGGR_INIT_EXPR_SLOT (t);
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval-prop6.C b/gcc/testsuite/g++.dg/cpp2a/consteval-prop6.C
index 93ed398d9bf..ca7db7c63d3 100644
--- a/gcc/testsuite/g++.dg/cpp2a/consteval-prop6.C
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval-prop6.C
@@ -48,7 +48,7 @@ struct X {
int a = sizeof(undef(0));
int x = undef(0);
- X() = default; // { dg-error "modification of .x. is not a constant expression" }
+ X() = default; // { dg-error {'consteval int undef\(int\)' used before its definition} }
};
void
diff --git a/gcc/testsuite/g++.dg/opt/is_constant_evaluated3.C b/gcc/testsuite/g++.dg/opt/is_constant_evaluated3.C
index 0a1e46e5638..783127cf909 100644
--- a/gcc/testsuite/g++.dg/opt/is_constant_evaluated3.C
+++ b/gcc/testsuite/g++.dg/opt/is_constant_evaluated3.C
@@ -17,7 +17,7 @@ int main() {
}
// { dg-final { scan-tree-dump "a1 = {\\.n=42, \\.m=0}" "original" } }
-// { dg-final { scan-tree-dump "a2 = {\\.n=42, \\.m=0}" "original" { xfail *-*-* } } }
-// { dg-final { scan-tree-dump "a3 = {\\.n=42, \\.m=0}" "original" { xfail *-*-* } } }
-// { dg-final { scan-tree-dump "a4 = {\\.n=42, \\.m=0}" "original" { xfail *-*-* } } }
-// { dg-final { scan-tree-dump "a5 = {\\.n=42, \\.m=0}" "original" { xfail *-*-* } } }
+// { dg-final { scan-tree-dump "a2 = {\\.n=42, \\.m=0}" "original" } }
+// { dg-final { scan-tree-dump "a3 = {\\.n=42, \\.m=0}" "original" } }
+// { dg-final { scan-tree-dump "a4 = {\\.n=42, \\.m=0}" "original" } }
+// { dg-final { scan-tree-dump "a5 = {\\.n=42, \\.m=0}" "original" } }
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [pushed 3/4] c++: fix in-charge parm in constexpr
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
2023-12-13 16:47 ` [pushed 2/4] c++: constant direct-initialization [PR108243] Jason Merrill
@ 2023-12-13 16:47 ` Jason Merrill
2023-12-13 16:47 ` [pushed 4/4] c++: End lifetime of objects in constexpr after destructor call [PR71093] Jason Merrill
` (2 subsequent siblings)
4 siblings, 0 replies; 29+ messages in thread
From: Jason Merrill @ 2023-12-13 16:47 UTC (permalink / raw)
To: gcc-patches
Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
I was puzzled by the proposed patch for PR71093 specifically ignoring the
in-charge parameter; the problem turned out to be that when
cxx_eval_call_expression jumps from the clone to the cloned function, it
assumes that the latter has the same parameters, and so the in-charge parm
doesn't get an argument. Since a class with vbases can't have constexpr
'tors there isn't actually a need for an in-charge parameter in a
destructor, but we used to use it for deleting destructors and never removed
it. I have a patch to do that for GCC 15, but for now let's work around it.
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_call_expression): Handle missing in-charge
argument.
---
gcc/cp/constexpr.cc | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 4cf9dd71b05..9d9e96c2afd 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3169,6 +3169,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
ctx->global->put_value (remapped, arg);
remapped = DECL_CHAIN (remapped);
}
+ for (; remapped; remapped = TREE_CHAIN (remapped))
+ if (DECL_NAME (remapped) == in_charge_identifier)
+ {
+ /* FIXME destructors unnecessarily have in-charge parameters
+ even in classes without vbases, map it to 0 for now. */
+ gcc_assert (!CLASSTYPE_VBASECLASSES (DECL_CONTEXT (fun)));
+ ctx->global->put_value (remapped, integer_zero_node);
+ }
+ else
+ {
+ gcc_assert (seen_error ());
+ *non_constant_p = true;
+ }
/* Add the RESULT_DECL to the values map, too. */
gcc_assert (!DECL_BY_REFERENCE (res));
ctx->global->put_value (res, NULL_TREE);
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [pushed 4/4] c++: End lifetime of objects in constexpr after destructor call [PR71093]
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
2023-12-13 16:47 ` [pushed 2/4] c++: constant direct-initialization [PR108243] Jason Merrill
2023-12-13 16:47 ` [pushed 3/4] c++: fix in-charge parm in constexpr Jason Merrill
@ 2023-12-13 16:47 ` Jason Merrill
2023-12-13 18:05 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Patrick Palka
2023-12-14 0:00 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Marek Polacek
4 siblings, 0 replies; 29+ messages in thread
From: Jason Merrill @ 2023-12-13 16:47 UTC (permalink / raw)
To: gcc-patches; +Cc: Nathaniel Shead
Tested x86_64-pc-linux-gnu, applying to trunk.
This is modified from Nathaniel's last version by adjusting for my recent
CLOBBER changes and removing the special handling of __in_chrg which is no
longer needed since my previous commit.
-- 8< --
This patch adds checks for using objects after they've been manually
destroyed via explicit destructor call. Currently this is only
implemented for 'top-level' objects; FIELD_DECLs and individual elements
of arrays will need a lot more work to track correctly and are left for
a future patch.
The other limitation is that destruction of parameter objects is checked
too 'early', happening at the end of the function call rather than the
end of the owning full-expression as they should be for consistency;
see cpp2a/constexpr-lifetime2.C. This is because I wasn't able to find a
good way to link the constructed parameter declarations with the
variable declarations that are actually destroyed later on to propagate
their lifetime status, so I'm leaving this for a later patch.
PR c++/71093
gcc/cp/ChangeLog:
* constexpr.cc (constexpr_global_ctx::get_value_ptr): Don't
return NULL_TREE for objects we're initializing.
(constexpr_global_ctx::destroy_value): Rename from remove_value.
Only mark real variables as outside lifetime.
(constexpr_global_ctx::clear_value): New function.
(destroy_value_checked): New function.
(cxx_eval_call_expression): Defer complaining about non-constant
arg0 for operator delete. Use remove_value_safe.
(cxx_fold_indirect_ref_1): Handle conversion to 'as base' type.
(outside_lifetime_error): Include name of object we're
accessing.
(cxx_eval_store_expression): Handle clobbers. Improve error
messages.
(cxx_eval_constant_expression): Use remove_value_safe. Clear
bind variables before entering body.
gcc/testsuite/ChangeLog:
* g++.dg/cpp1y/constexpr-lifetime1.C: Improve error message.
* g++.dg/cpp1y/constexpr-lifetime2.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime3.C: Likewise.
* g++.dg/cpp1y/constexpr-lifetime4.C: Likewise.
* g++.dg/cpp2a/bitfield2.C: Likewise.
* g++.dg/cpp2a/constexpr-new3.C: Likewise. New check.
* g++.dg/cpp1y/constexpr-lifetime7.C: New test.
* g++.dg/cpp2a/constexpr-lifetime1.C: New test.
* g++.dg/cpp2a/constexpr-lifetime2.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/constexpr.cc | 148 +++++++++++++++---
.../g++.dg/cpp1y/constexpr-lifetime1.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime2.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime3.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime4.C | 2 +-
.../g++.dg/cpp1y/constexpr-lifetime7.C | 93 +++++++++++
gcc/testsuite/g++.dg/cpp2a/bitfield2.C | 2 +-
.../g++.dg/cpp2a/constexpr-lifetime1.C | 21 +++
.../g++.dg/cpp2a/constexpr-lifetime2.C | 23 +++
gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C | 17 +-
10 files changed, 284 insertions(+), 28 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9d9e96c2afd..e1b2d27fc36 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1193,13 +1193,20 @@ public:
return *p;
return NULL_TREE;
}
- tree *get_value_ptr (tree t)
+ tree *get_value_ptr (tree t, bool initializing)
{
if (modifiable && !modifiable->contains (t))
return nullptr;
if (tree *p = values.get (t))
- if (*p != void_node)
- return p;
+ {
+ if (*p != void_node)
+ return p;
+ else if (initializing)
+ {
+ *p = NULL_TREE;
+ return p;
+ }
+ }
return nullptr;
}
void put_value (tree t, tree v)
@@ -1208,13 +1215,19 @@ public:
if (!already_in_map && modifiable)
modifiable->add (t);
}
- void remove_value (tree t)
+ void destroy_value (tree t)
{
- if (DECL_P (t))
+ if (TREE_CODE (t) == VAR_DECL
+ || TREE_CODE (t) == PARM_DECL
+ || TREE_CODE (t) == RESULT_DECL)
values.put (t, void_node);
else
values.remove (t);
}
+ void clear_value (tree t)
+ {
+ values.remove (t);
+ }
};
/* Helper class for constexpr_global_ctx. In some cases we want to avoid
@@ -1238,7 +1251,7 @@ public:
~modifiable_tracker ()
{
for (tree t: set)
- global->remove_value (t);
+ global->clear_value (t);
global->modifiable = nullptr;
}
};
@@ -1278,6 +1291,40 @@ struct constexpr_ctx {
mce_value manifestly_const_eval;
};
+/* Remove T from the global values map, checking for attempts to destroy
+ a value that has already finished its lifetime. */
+
+static void
+destroy_value_checked (const constexpr_ctx* ctx, tree t, bool *non_constant_p)
+{
+ if (t == error_mark_node || TREE_TYPE (t) == error_mark_node)
+ return;
+
+ /* Don't error again here if we've already reported a problem. */
+ if (!*non_constant_p
+ && DECL_P (t)
+ /* Non-trivial destructors have their lifetimes ended explicitly
+ with a clobber, so don't worry about it here. */
+ && (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (t))
+ /* ...except parameters are remapped in cxx_eval_call_expression,
+ and the destructor call during cleanup won't be able to tell that
+ this value has already been destroyed, so complain now. This is
+ not quite unobservable, but is extremely unlikely to crop up in
+ practice; see g++.dg/cpp2a/constexpr-lifetime2.C. */
+ || TREE_CODE (t) == PARM_DECL)
+ && ctx->global->is_outside_lifetime (t))
+ {
+ if (!ctx->quiet)
+ {
+ auto_diagnostic_group d;
+ error ("destroying %qE outside its lifetime", t);
+ inform (DECL_SOURCE_LOCATION (t), "declared here");
+ }
+ *non_constant_p = true;
+ }
+ ctx->global->destroy_value (t);
+}
+
/* This internal flag controls whether we should avoid doing anything during
constexpr evaluation that would cause extra DECL_UID generation, such as
template instantiation and function body copying. */
@@ -2806,6 +2853,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
&& (CALL_FROM_NEW_OR_DELETE_P (t)
|| is_std_allocator_allocate (ctx->call)))
{
+ const bool new_op_p = IDENTIFIER_NEW_OP_P (DECL_NAME (fun));
const int nargs = call_expr_nargs (t);
tree arg0 = NULL_TREE;
for (int i = 0; i < nargs; ++i)
@@ -2813,12 +2861,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
tree arg = CALL_EXPR_ARG (t, i);
arg = cxx_eval_constant_expression (ctx, arg, vc_prvalue,
non_constant_p, overflow_p);
- VERIFY_CONSTANT (arg);
+ /* Deleting a non-constant pointer has a better error message
+ below. */
+ if (new_op_p || i != 0)
+ VERIFY_CONSTANT (arg);
if (i == 0)
arg0 = arg;
}
gcc_assert (arg0);
- if (IDENTIFIER_NEW_OP_P (DECL_NAME (fun)))
+ if (new_op_p)
{
tree type = build_array_type_nelts (char_type_node,
tree_to_uhwi (arg0));
@@ -2867,7 +2918,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
return t;
}
DECL_NAME (var) = heap_deleted_identifier;
- ctx->global->remove_value (var);
+ ctx->global->destroy_value (var);
ctx->global->heap_dealloc_count++;
return void_node;
}
@@ -2890,7 +2941,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
return t;
}
DECL_NAME (var) = heap_deleted_identifier;
- ctx->global->remove_value (var);
+ ctx->global->destroy_value (var);
ctx->global->heap_dealloc_count++;
return void_node;
}
@@ -3255,9 +3306,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
non_constant_p, overflow_p);
/* Remove the parms/result from the values map. */
- ctx->global->remove_value (res);
+ destroy_value_checked (ctx, res, non_constant_p);
for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
- ctx->global->remove_value (parm);
+ destroy_value_checked (ctx, parm, non_constant_p);
/* Free any parameter CONSTRUCTORs we aren't returning directly. */
while (!ctors->is_empty ())
@@ -5657,6 +5708,10 @@ cxx_fold_indirect_ref_1 (const constexpr_ctx *ctx, location_t loc, tree type,
}
}
+ /* Handle conversion to "as base" type. */
+ if (CLASSTYPE_AS_BASE (optype) == type)
+ return op;
+
/* Handle conversion to an empty base class, which is represented with a
NOP_EXPR. Do this before spelunking into the non-empty subobjects,
which is likely to be a waste of time (109678). */
@@ -5908,7 +5963,7 @@ outside_lifetime_error (location_t loc, tree r)
}
else
{
- error_at (loc, "accessing object outside its lifetime");
+ error_at (loc, "accessing %qE outside its lifetime", r);
inform (DECL_SOURCE_LOCATION (r), "declared here");
}
}
@@ -6125,8 +6180,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
constexpr_ctx new_ctx = *ctx;
tree init = TREE_OPERAND (t, 1);
- if (TREE_CLOBBER_P (init))
- /* Just ignore clobbers. */
+
+ if (TREE_CLOBBER_P (init)
+ && CLOBBER_KIND (init) < CLOBBER_OBJECT_END)
+ /* Only handle clobbers ending the lifetime of objects. */
return void_node;
/* First we figure out where we're storing to. */
@@ -6136,7 +6193,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
tree type = TREE_TYPE (target);
bool preeval = SCALAR_TYPE_P (type) || TREE_CODE (t) == MODIFY_EXPR;
- if (preeval)
+ if (preeval && !TREE_CLOBBER_P (init))
{
/* Evaluate the value to be stored without knowing what object it will be
stored in, so that any side-effects happen first. */
@@ -6244,11 +6301,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
&& const_object_being_modified == NULL_TREE)
const_object_being_modified = object;
+ if (DECL_P (object)
+ && TREE_CLOBBER_P (init)
+ && DECL_NAME (object) == heap_deleted_identifier)
+ /* Ignore clobbers of deleted allocations for now; we'll get a better error
+ message later when operator delete is called. */
+ return void_node;
+
/* And then find/build up our initializer for the path to the subobject
we're initializing. */
tree *valp;
if (DECL_P (object))
- valp = ctx->global->get_value_ptr (object);
+ valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
else
valp = NULL;
if (!valp)
@@ -6256,10 +6320,45 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
/* A constant-expression cannot modify objects from outside the
constant-expression. */
if (!ctx->quiet)
- error ("modification of %qE is not a constant expression", object);
+ {
+ auto_diagnostic_group d;
+ if (DECL_P (object) && DECL_NAME (object) == heap_deleted_identifier)
+ {
+ error ("modification of allocated storage after deallocation "
+ "is not a constant expression");
+ inform (DECL_SOURCE_LOCATION (object), "allocated here");
+ }
+ else if (DECL_P (object) && ctx->global->is_outside_lifetime (object))
+ {
+ if (TREE_CLOBBER_P (init))
+ error ("destroying %qE outside its lifetime", object);
+ else
+ error ("modification of %qE outside its lifetime "
+ "is not a constant expression", object);
+ inform (DECL_SOURCE_LOCATION (object), "declared here");
+ }
+ else
+ {
+ if (TREE_CLOBBER_P (init))
+ error ("destroying %qE from outside current evaluation "
+ "is not a constant expression", object);
+ else
+ error ("modification of %qE from outside current evaluation "
+ "is not a constant expression", object);
+ }
+ }
*non_constant_p = true;
return t;
}
+
+ /* Handle explicit end-of-lifetime. */
+ if (TREE_CLOBBER_P (init))
+ {
+ if (refs->is_empty ())
+ ctx->global->destroy_value (object);
+ return void_node;
+ }
+
type = TREE_TYPE (object);
bool no_zero_init = true;
@@ -6533,7 +6632,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
/* The hash table might have moved since the get earlier, and the
initializer might have mutated the underlying CONSTRUCTORs, so we must
recompute VALP. */
- valp = ctx->global->get_value_ptr (object);
+ valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
for (unsigned i = 0; i < vec_safe_length (indexes); i++)
{
ctors[i] = valp;
@@ -7650,7 +7749,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
/* Forget SAVE_EXPRs and TARGET_EXPRs created by this
full-expression. */
for (tree save_expr : save_exprs)
- ctx->global->remove_value (save_expr);
+ destroy_value_checked (ctx, save_expr, non_constant_p);
}
break;
@@ -8203,13 +8302,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
non_constant_p, overflow_p, jump_target);
case BIND_EXPR:
+ /* Pre-emptively clear the vars declared by this BIND_EXPR from the value
+ map, so that when checking whether they're already destroyed later we
+ don't get confused by remnants of previous calls. */
+ for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
+ ctx->global->clear_value (decl);
r = cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
lval,
non_constant_p, overflow_p,
jump_target);
for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
- ctx->global->remove_value (decl);
- return r;
+ destroy_value_checked (ctx, decl, non_constant_p);
+ break;
case PREINCREMENT_EXPR:
case POSTINCREMENT_EXPR:
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
index 43aa7c974c1..3fda29e0cc2 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
@@ -10,4 +10,4 @@ constexpr const int& test() {
auto local = S{}; // { dg-message "note: declared here" }
return local.get();
}
-constexpr int x = test(); // { dg-error "accessing object outside its lifetime" }
+constexpr int x = test(); // { dg-error "accessing .local. outside its lifetime" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
index 2f5ae8db6d5..d82ba5c8b73 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
@@ -8,7 +8,7 @@ struct S {
constexpr int error() {
const auto& local = S{}.get(); // { dg-message "note: declared here" }
- return local; // { dg-error "accessing object outside its lifetime" }
+ return local; // { dg-error "accessing '\[^'\]+' outside its lifetime" }
}
constexpr int x = error(); // { dg-message "in .constexpr. expansion" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
index 53785521d05..67e9b91c723 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
@@ -7,7 +7,7 @@ constexpr int f(int i) {
int j = 123; // { dg-message "note: declared here" }
p = &j;
}
- return *p; // { dg-error "accessing object outside its lifetime" }
+ return *p; // { dg-error "accessing 'j' outside its lifetime" }
}
constexpr int i = f(0); // { dg-message "in .constexpr. expansion" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
index 181a1201663..6f0d749dcf2 100644
--- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
@@ -5,7 +5,7 @@ constexpr const double& test() {
return local;
}
-static_assert(test() == 3.0, ""); // { dg-error "constant|accessing object outside its lifetime" }
+static_assert(test() == 3.0, ""); // { dg-error "constant|accessing '\[^'\]+' outside its lifetime" }
// no deference, shouldn't error
static_assert((test(), true), "");
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
new file mode 100644
index 00000000000..4148f42f7be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C
@@ -0,0 +1,93 @@
+// PR c++/71093
+// { dg-do compile { target c++14 } }
+
+constexpr int f (const int *p)
+{
+ typedef int T;
+ p->~T (); // { dg-error "destroying" }
+ return *p;
+}
+
+constexpr int i = 0;
+constexpr int j = f (&i);
+
+
+template <typename T>
+constexpr bool test_access() {
+ T x {};
+ x.~T();
+ T y = x; // { dg-error "lifetime" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_modification() {
+ T x {};
+ x.~T();
+ x = T(); // { dg-error "lifetime" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_scope() {
+ {
+ T x {};
+ x.~T();
+ } // { dg-error "destroying" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_destroy_temp() {
+ T{}.~T(); // { dg-error "destroying" }
+ return true;
+}
+
+template <typename T>
+constexpr bool test_parameter(T t) {
+ // note: error message occurs at point of call
+ t.~T();
+ return true;
+}
+
+template <typename T>
+constexpr void test_bindings_impl(int n) {
+ if (n == 0) return;
+ T a {};
+ if (n == 1) return;
+ T b {};
+}
+
+template <typename T>
+constexpr bool test_bindings() {
+ test_bindings_impl<T>(1);
+ test_bindings_impl<T>(0);
+ test_bindings_impl<T>(2);
+ return true;
+}
+
+constexpr bool i1 = test_access<int>(); // { dg-message "in .constexpr." }
+constexpr bool i2 = test_modification<int>(); // { dg-message "in .constexpr." }
+constexpr bool i3 = test_scope<int>(); // { dg-message "in .constexpr." }
+constexpr bool i4 = test_destroy_temp<int>(); // { dg-message "in .constexpr." "" { xfail *-*-* } }
+constexpr bool i5 = test_parameter(int{}); // { dg-error "destroying" }
+constexpr bool i6 = test_bindings<int>();
+
+struct Trivial { int x; };
+constexpr bool t1 = test_access<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t2 = test_modification<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t3 = test_scope<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t4 = test_destroy_temp<Trivial>(); // { dg-message "in .constexpr." }
+constexpr bool t5 = test_parameter(Trivial{}); // { dg-error "destroying" }
+constexpr bool t6 = test_bindings<Trivial>();
+
+#if __cplusplus >= 202002L
+struct NonTrivial { int x; constexpr ~NonTrivial() {} }; // { dg-error "destroying" "" { target c++20 } }
+constexpr bool n1 = test_access<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n2 = test_modification<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n3 = test_scope<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n4 = test_destroy_temp<NonTrivial>(); // { dg-message "in .constexpr." "" { target c++20 } }
+constexpr bool n5 = test_parameter(NonTrivial{}); // { dg-error "destroying" "" { target c++20 } }
+constexpr bool n6 = test_bindings<NonTrivial>();
+#endif
+
diff --git a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
index dcb424fc8f6..885d4f0e26d 100644
--- a/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
+++ b/gcc/testsuite/g++.dg/cpp2a/bitfield2.C
@@ -13,7 +13,7 @@ template <bool V, int W>
struct U {
int j : W = 7; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
int k : W { 8 }; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
- int l : V ? 7 : a = 3; // { dg-error "modification of .a. is not a constant expression" }
+ int l : V ? 7 : a = 3; // { dg-error "modification of .a. from outside current evaluation is not a constant expression" }
// { dg-error "width not an integer constant" "" { target *-*-* } .-1 }
int m : (V ? W : b) = 9; // { dg-warning "default member initializers for bit-fields only available with" "" { target c++17_down } }
// { dg-error "zero width for bit-field" "" { target *-*-* } .-1 }
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
new file mode 100644
index 00000000000..36163844eca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime1.C
@@ -0,0 +1,21 @@
+// { dg-do compile { target c++20 } }
+
+#include "construct_at.h"
+
+struct S { int x; };
+constexpr int f() {
+ S s;
+ s.~S();
+ std::construct_at(&s, 5);
+ return s.x;
+}
+static_assert(f() == 5);
+
+struct T { int x; constexpr ~T() {} };
+constexpr int g() {
+ T t;
+ t.~T();
+ std::construct_at(&t, 12);
+ return t.x;
+}
+static_assert(g() == 12);
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
new file mode 100644
index 00000000000..56cc9e3c1c8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-lifetime2.C
@@ -0,0 +1,23 @@
+// { dg-do compile { target c++20 } }
+
+#include "construct_at.h"
+
+struct S { int x; };
+
+constexpr bool foo(S s, S*& p) {
+ p = &s;
+ s.~S();
+ return true;
+}
+
+constexpr bool bar() {
+ // This is, strictly speaking, implementation-defined behaviour;
+ // see [expr.call] p6. However, in all other cases we destroy
+ // at the end of the full-expression, so the below should be fixed.
+ S* p;
+ foo(S{}, p), std::construct_at(p); // { dg-bogus "destroying" "" { xfail *-*-* } }
+
+ return true;
+}
+
+constexpr bool x = bar();
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
index 3ba440fec53..5d9f192507b 100644
--- a/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new3.C
@@ -34,7 +34,7 @@ constexpr auto v3 = f3 (); // { dg-message "in 'constexpr' expansion of" }
constexpr bool
f4 (int *p)
{
- delete p; // { dg-error "deallocation of storage that was not previously allocated" }
+ delete p; // { dg-error "destroying 'q' from outside current evaluation" }
return false;
}
@@ -70,3 +70,18 @@ f7 ()
}
constexpr auto v7 = f7 ();
+
+constexpr bool
+f8_impl (int *p)
+{
+ delete p; // { dg-error "deallocation of storage that was not previously allocated" }
+ return false;
+}
+
+constexpr bool
+f8 ()
+{
+ int q = 0;
+ return f8_impl (&q);
+}
+constexpr auto v8 = f8 (); // { dg-message "in 'constexpr' expansion of" }
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [pushed 1/4] c++: copy location to AGGR_INIT_EXPR
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
` (2 preceding siblings ...)
2023-12-13 16:47 ` [pushed 4/4] c++: End lifetime of objects in constexpr after destructor call [PR71093] Jason Merrill
@ 2023-12-13 18:05 ` Patrick Palka
2023-12-13 20:06 ` [pushed] c++: TARGET_EXPR location in default arg [PR96997] Jason Merrill
2023-12-14 0:00 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Marek Polacek
4 siblings, 1 reply; 29+ messages in thread
From: Patrick Palka @ 2023-12-13 18:05 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, 13 Dec 2023, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk.
>
> -- 8< --
>
> When building an AGGR_INIT_EXPR from a CALL_EXPR, we shouldn't lose location
> information.
>
> gcc/cp/ChangeLog:
>
> * tree.cc (build_aggr_init_expr): Copy EXPR_LOCATION.
I made a similar change in the past which caused the debug regression
PR96997 which I fixed by reverting the change in r11-7263-g78a6d0e30d7950
(didn't do much deeper analysis than that). Unfortunately it seems this
regression is back now.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp1y/constexpr-nsdmi7b.C: Adjust line.
> * g++.dg/template/copy1.C: Likewise.
> ---
> gcc/cp/tree.cc | 1 +
> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C | 4 ++--
> gcc/testsuite/g++.dg/template/copy1.C | 2 +-
> 3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index da4d5c51f07..c4e41fd7b5c 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -689,6 +689,7 @@ build_aggr_init_expr (tree type, tree init)
> CALL_EXPR_OPERATOR_SYNTAX (rval) = CALL_EXPR_OPERATOR_SYNTAX (init);
> CALL_EXPR_ORDERED_ARGS (rval) = CALL_EXPR_ORDERED_ARGS (init);
> CALL_EXPR_REVERSE_ARGS (rval) = CALL_EXPR_REVERSE_ARGS (init);
> + SET_EXPR_LOCATION (rval, EXPR_LOCATION (init));
> }
> else
> rval = init;
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
> index a410e482664..586ee54124c 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi7b.C
> @@ -20,8 +20,8 @@ bar()
> {
> A a = foo();
> a.p->n = 5;
> - return a;
> -} // { dg-error "non-.constexpr." "" { target c++20_down } }
> + return a; // { dg-error "non-.constexpr." "" { target c++20_down } }
> +}
>
> constexpr int
> baz()
> diff --git a/gcc/testsuite/g++.dg/template/copy1.C b/gcc/testsuite/g++.dg/template/copy1.C
> index eacd9e2c025..7e0a3805a77 100644
> --- a/gcc/testsuite/g++.dg/template/copy1.C
> +++ b/gcc/testsuite/g++.dg/template/copy1.C
> @@ -6,10 +6,10 @@
>
> struct A
> {
> - // { dg-error "reference" "" { target c++14_down } .+1 }
> A(A&); // { dg-message "A::A" "" { target c++14_down } }
> template <class T> A(T); // { dg-message "A::A" "" { target c++14_down } }
> };
>
> +// { dg-error "reference" "" { target c++14_down } .+1 }
> A a = 0; // { dg-error "no match" "" { target c++14_down } }
>
>
> base-commit: d2b269ce30d77dbfc6c28c75887c330d4698b132
> --
> 2.39.3
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [pushed] c++: TARGET_EXPR location in default arg [PR96997]
2023-12-13 18:05 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Patrick Palka
@ 2023-12-13 20:06 ` Jason Merrill
0 siblings, 0 replies; 29+ messages in thread
From: Jason Merrill @ 2023-12-13 20:06 UTC (permalink / raw)
To: gcc-patches; +Cc: Patrick Palka
Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
My r14-6505-g52b4b7d7f5c7c0 change to copy the location in
build_aggr_init_expr reopened PR96997; let's fix it properly this time, by
clearing the location like we do for other trees.
PR c++/96997
gcc/cp/ChangeLog:
* tree.cc (bot_manip): Check data.clear_location for TARGET_EXPR.
gcc/testsuite/ChangeLog:
* g++.dg/debug/cleanup2.C: New test.
---
gcc/cp/tree.cc | 3 +++
gcc/testsuite/g++.dg/debug/cleanup2.C | 10 ++++++++++
2 files changed, 13 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/debug/cleanup2.C
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index c4e41fd7b5c..d26e73aaf95 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -3170,6 +3170,9 @@ bot_manip (tree* tp, int* walk_subtrees, void* data_)
if (TREE_OPERAND (u, 1) == error_mark_node)
return error_mark_node;
+ if (data.clear_location)
+ SET_EXPR_LOCATION (u, input_location);
+
/* Replace the old expression with the new version. */
*tp = u;
/* We don't have to go below this point; the recursive call to
diff --git a/gcc/testsuite/g++.dg/debug/cleanup2.C b/gcc/testsuite/g++.dg/debug/cleanup2.C
new file mode 100644
index 00000000000..03bf92c8424
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/cleanup2.C
@@ -0,0 +1,10 @@
+// PR c++/96997
+// { dg-additional-options "-g -fdump-tree-gimple-lineno" }
+
+struct A { A(); ~A(); };
+void f(const A& = A());
+int main() { f(); }
+
+// The destructor call for the A temporary should not have the location of the
+// f declaration.
+// { dg-final { scan-tree-dump-not ".C:5" "gimple" } }
base-commit: da730b29f10fb48d5ed812535768c69ff7d74248
--
2.39.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [pushed 1/4] c++: copy location to AGGR_INIT_EXPR
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
` (3 preceding siblings ...)
2023-12-13 18:05 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Patrick Palka
@ 2023-12-14 0:00 ` Marek Polacek
2023-12-14 1:38 ` Jason Merrill
4 siblings, 1 reply; 29+ messages in thread
From: Marek Polacek @ 2023-12-14 0:00 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, Dec 13, 2023 at 11:47:37AM -0500, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk.
>
> -- 8< --
>
> When building an AGGR_INIT_EXPR from a CALL_EXPR, we shouldn't lose location
> information.
I think the following should be an obvious fix, so I'll check it in.
-- >8 --
Since r14-6505 I see:
FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++23 at line 91 (test for errors, line 89)
FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++23 (test for excess errors)
FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++26 at line 91 (test for errors, line 89)
FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++26 (test for excess errors)
and it wasn't fixed by r14-6511. So I'm fixing it with the below.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-ex1.C: Adjust expected diagnostic line.
---
gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
index 383d38a42d4..b26eb5d0c90 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
@@ -88,7 +88,7 @@ struct resource {
};
constexpr resource f(resource d)
{ return d; } // { dg-error "non-.constexpr." "" { target { { { ! implicit_constexpr } && c++20_down } || c++11_only } } }
-// { dg-error "non-.constexpr." "" { target { c++23 && { ! implicit_constexpr } } } .-2 }
+// { dg-error "non-.constexpr." "" { target { c++23 && { ! implicit_constexpr } } } .-1 }
constexpr resource d = f(9); // { dg-message ".constexpr." "" { target { { ! implicit_constexpr } || c++11_only } } }
// 4.4 floating-point constant expressions
base-commit: c535360788e142a92e1d8b1db25bf4452e26f5fb
--
2.43.0
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [pushed 1/4] c++: copy location to AGGR_INIT_EXPR
2023-12-14 0:00 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Marek Polacek
@ 2023-12-14 1:38 ` Jason Merrill
2023-12-14 14:25 ` Marek Polacek
0 siblings, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2023-12-14 1:38 UTC (permalink / raw)
To: Marek Polacek; +Cc: gcc-patches
On 12/13/23 19:00, Marek Polacek wrote:
> On Wed, Dec 13, 2023 at 11:47:37AM -0500, Jason Merrill wrote:
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>>
>> -- 8< --
>>
>> When building an AGGR_INIT_EXPR from a CALL_EXPR, we shouldn't lose location
>> information.
>
> I think the following should be an obvious fix, so I'll check it in.
Thanks, I wonder why I wasn't seeing that?
> -- >8 --
> Since r14-6505 I see:
>
> FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++23 at line 91 (test for errors, line 89)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++23 (test for excess errors)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++26 at line 91 (test for errors, line 89)
> FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++26 (test for excess errors)
>
> and it wasn't fixed by r14-6511. So I'm fixing it with the below.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-ex1.C: Adjust expected diagnostic line.
> ---
> gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
> index 383d38a42d4..b26eb5d0c90 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
> @@ -88,7 +88,7 @@ struct resource {
> };
> constexpr resource f(resource d)
> { return d; } // { dg-error "non-.constexpr." "" { target { { { ! implicit_constexpr } && c++20_down } || c++11_only } } }
> -// { dg-error "non-.constexpr." "" { target { c++23 && { ! implicit_constexpr } } } .-2 }
> +// { dg-error "non-.constexpr." "" { target { c++23 && { ! implicit_constexpr } } } .-1 }
> constexpr resource d = f(9); // { dg-message ".constexpr." "" { target { { ! implicit_constexpr } || c++11_only } } }
>
> // 4.4 floating-point constant expressions
>
> base-commit: c535360788e142a92e1d8b1db25bf4452e26f5fb
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [pushed 1/4] c++: copy location to AGGR_INIT_EXPR
2023-12-14 1:38 ` Jason Merrill
@ 2023-12-14 14:25 ` Marek Polacek
0 siblings, 0 replies; 29+ messages in thread
From: Marek Polacek @ 2023-12-14 14:25 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, Dec 13, 2023 at 08:38:12PM -0500, Jason Merrill wrote:
> On 12/13/23 19:00, Marek Polacek wrote:
> > On Wed, Dec 13, 2023 at 11:47:37AM -0500, Jason Merrill wrote:
> > > Tested x86_64-pc-linux-gnu, applying to trunk.
> > >
> > > -- 8< --
> > >
> > > When building an AGGR_INIT_EXPR from a CALL_EXPR, we shouldn't lose location
> > > information.
> >
> > I think the following should be an obvious fix, so I'll check it in.
>
> Thanks, I wonder why I wasn't seeing that?
It must be due to -fimplicit-constexpr. So if you have
GXX_TESTSUITE_STDS=98,11,14,17,20,impcx
then I think the FAIL won't show up.
> > -- >8 --
> > Since r14-6505 I see:
> >
> > FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++23 at line 91 (test for errors, line 89)
> > FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++23 (test for excess errors)
> > FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++26 at line 91 (test for errors, line 89)
> > FAIL: g++.dg/cpp0x/constexpr-ex1.C -std=c++26 (test for excess errors)
> >
> > and it wasn't fixed by r14-6511. So I'm fixing it with the below.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/constexpr-ex1.C: Adjust expected diagnostic line.
> > ---
> > gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
> > index 383d38a42d4..b26eb5d0c90 100644
> > --- a/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ex1.C
> > @@ -88,7 +88,7 @@ struct resource {
> > };
> > constexpr resource f(resource d)
> > { return d; } // { dg-error "non-.constexpr." "" { target { { { ! implicit_constexpr } && c++20_down } || c++11_only } } }
> > -// { dg-error "non-.constexpr." "" { target { c++23 && { ! implicit_constexpr } } } .-2 }
> > +// { dg-error "non-.constexpr." "" { target { c++23 && { ! implicit_constexpr } } } .-1 }
> > constexpr resource d = f(9); // { dg-message ".constexpr." "" { target { { ! implicit_constexpr } || c++11_only } } }
> > // 4.4 floating-point constant expressions
> >
> > base-commit: c535360788e142a92e1d8b1db25bf4452e26f5fb
>
Marek
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2023-12-14 14:25 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-03 1:18 [PATCH] c++: End lifetime of objects in constexpr after destructor call [PR71093] Nathaniel Shead
2023-11-03 1:34 ` Nathaniel Shead
2023-11-27 11:08 ` Nathaniel Shead
2023-12-09 20:12 ` Jason Merrill
2023-12-10 10:22 ` Richard Biener
2023-12-10 11:21 ` Alexander Monakov
2023-12-10 15:58 ` Richard Biener
2023-12-10 18:34 ` Jason Merrill
2023-12-11 8:02 ` Richard Biener
2023-12-11 19:12 ` Jason Merrill
2023-12-11 19:17 ` Richard Biener
2023-12-11 19:21 ` Marek Polacek
2023-12-11 22:00 ` Jason Merrill
2023-12-11 22:22 ` Marek Polacek
2023-12-11 23:03 ` Jakub Jelinek
2023-12-12 11:13 ` Alexander Monakov
2023-12-12 11:15 ` Jakub Jelinek
2023-12-12 15:24 ` Jason Merrill
2023-12-12 17:50 ` Jason Merrill
2023-12-13 4:40 ` Jason Merrill
2023-12-13 16:47 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Jason Merrill
2023-12-13 16:47 ` [pushed 2/4] c++: constant direct-initialization [PR108243] Jason Merrill
2023-12-13 16:47 ` [pushed 3/4] c++: fix in-charge parm in constexpr Jason Merrill
2023-12-13 16:47 ` [pushed 4/4] c++: End lifetime of objects in constexpr after destructor call [PR71093] Jason Merrill
2023-12-13 18:05 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Patrick Palka
2023-12-13 20:06 ` [pushed] c++: TARGET_EXPR location in default arg [PR96997] Jason Merrill
2023-12-14 0:00 ` [pushed 1/4] c++: copy location to AGGR_INIT_EXPR Marek Polacek
2023-12-14 1:38 ` Jason Merrill
2023-12-14 14:25 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).