c++: End lifetime of objects in constexpr after destructor call [PR71093]
Checks
Commit Message
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
Comments
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
>
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
> >
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
> 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
>
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
> 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
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
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.
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?
> 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>
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
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:
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
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
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
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
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:
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
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
@@ -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);
}
@@ -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:
@@ -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
@@ -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" }
@@ -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" }
@@ -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" }
@@ -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), "");
new file mode 100644
@@ -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
+
@@ -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 }
new file mode 100644
@@ -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);
new file mode 100644
@@ -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();
@@ -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" }