On Wed, 29 Mar 2023, Nathaniel Shead via Gcc-patches wrote: > This adds rudimentary lifetime tracking in C++ constexpr contexts, > allowing the compiler to report errors with using values after their > backing has gone out of scope. We don't yet handle other ways of ending > lifetimes (e.g. explicit destructor calls).
Awesome! > > PR c++/96630 > PR c++/98675 > PR c++/70331 > > gcc/cp/ChangeLog: > > * constexpr.cc (constexpr_global_ctx::put_value): Mark value as > in lifetime. > (constexpr_global_ctx::remove_value): Mark value as expired. > (cxx_eval_call_expression): Remove comment that is no longer > applicable. > (non_const_var_error): Add check for expired values. > (cxx_eval_constant_expression): Add checks for expired values. Forget > local variables at end of bind expressions. Forget temporaries at end > of cleanup points. > * cp-tree.h (struct lang_decl_base): New flag to track expired values > in constant evaluation. > (DECL_EXPIRED_P): Access the new flag. > (SET_DECL_EXPIRED_P): Modify the new flag. > * module.cc (trees_out::lang_decl_bools): Write out the new flag. > (trees_in::lang_decl_bools): Read in the new flag. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/constexpr-ice20.C: Update error raised by test. > * g++.dg/cpp1y/constexpr-lifetime1.C: New test. > * g++.dg/cpp1y/constexpr-lifetime2.C: New test. > * g++.dg/cpp1y/constexpr-lifetime3.C: New test. > * g++.dg/cpp1y/constexpr-lifetime4.C: New test. > * g++.dg/cpp1y/constexpr-lifetime5.C: New test. > > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com> > --- > gcc/cp/constexpr.cc | 69 +++++++++++++++---- > gcc/cp/cp-tree.h | 10 ++- > gcc/cp/module.cc | 2 + > gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C | 2 +- > .../g++.dg/cpp1y/constexpr-lifetime1.C | 13 ++++ > .../g++.dg/cpp1y/constexpr-lifetime2.C | 20 ++++++ > .../g++.dg/cpp1y/constexpr-lifetime3.C | 13 ++++ > .../g++.dg/cpp1y/constexpr-lifetime4.C | 11 +++ > .../g++.dg/cpp1y/constexpr-lifetime5.C | 11 +++ > 9 files changed, 137 insertions(+), 14 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 3de60cfd0f8..bdbc12144a7 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -1185,10 +1185,22 @@ public: > void put_value (tree t, tree v) > { > bool already_in_map = values.put (t, v); > + if (!already_in_map && DECL_P (t)) > + { > + if (!DECL_LANG_SPECIFIC (t)) > + retrofit_lang_decl (t); > + if (DECL_LANG_SPECIFIC (t)) > + SET_DECL_EXPIRED_P (t, false); > + } Since this new flag would only be used only during constexpr evaluation, could we instead use an on-the-side hash_set in constexpr_global_ctx for tracking expired-ness? That way we won't have to allocate a DECL_LANG_SPECIFIC structure for decls that lack it, and won't have to worry about the flag in other parts of the compiler. > if (!already_in_map && modifiable) > modifiable->add (t); > } > - void remove_value (tree t) { values.remove (t); } > + void remove_value (tree t) > + { > + if (DECL_P (t) && DECL_LANG_SPECIFIC (t)) > + SET_DECL_EXPIRED_P (t, true); > + values.remove (t); > + } > }; > > /* Helper class for constexpr_global_ctx. In some cases we want to avoid > @@ -3157,10 +3169,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, > tree t, > for (tree save_expr : save_exprs) > ctx->global->remove_value (save_expr); > > - /* Remove the parms/result from the values map. Is it worth > - bothering to do this when the map itself is only live for > - one constexpr evaluation? If so, maybe also clear out > - other vars from call, maybe in BIND_EXPR handling? */ > + /* Remove the parms/result from the values map. */ > ctx->global->remove_value (res); > for (tree parm = parms; parm; parm = TREE_CHAIN (parm)) > ctx->global->remove_value (parm); > @@ -5708,6 +5717,13 @@ non_const_var_error (location_t loc, tree r, bool > fundef_p) > inform (DECL_SOURCE_LOCATION (r), "allocated here"); > return; > } > + if (DECL_EXPIRED_P (r)) > + { > + if (constexpr_error (loc, fundef_p, "accessing object outside its " > + "lifetime")) > + inform (DECL_SOURCE_LOCATION (r), "declared here"); > + return; > + } > if (!constexpr_error (loc, fundef_p, "the value of %qD is not usable in " > "a constant expression", r)) > return; > @@ -7048,6 +7064,13 @@ cxx_eval_constant_expression (const constexpr_ctx > *ctx, tree t, > r = build_constructor (TREE_TYPE (t), NULL); > TREE_CONSTANT (r) = true; > } > + else if (DECL_EXPIRED_P (t)) > + { > + if (!ctx->quiet) > + non_const_var_error (loc, r, /*fundef_p*/false); > + *non_constant_p = true; > + break; > + } > else if (ctx->strict) > r = decl_really_constant_value (t, /*unshare_p=*/false); > else > @@ -7093,7 +7116,15 @@ cxx_eval_constant_expression (const constexpr_ctx > *ctx, tree t, > else > { > if (!ctx->quiet) > - error ("%qE is not a constant expression", t); > + { > + if (DECL_EXPIRED_P (r)) > + { > + error_at (loc, "accessing object outside its lifetime"); > + inform (DECL_SOURCE_LOCATION (r), "declared here"); > + } > + else > + error_at (loc, "%qE is not a constant expression", t); > + } > *non_constant_p = true; > } > break; > @@ -7315,17 +7346,28 @@ cxx_eval_constant_expression (const constexpr_ctx > *ctx, tree t, > auto_vec<tree, 2> cleanups; > vec<tree> *prev_cleanups = ctx->global->cleanups; > ctx->global->cleanups = &cleanups; > - r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), > + > + auto_vec<tree, 10> save_exprs; > + constexpr_ctx new_ctx = *ctx; > + new_ctx.save_exprs = &save_exprs; > + > + r = cxx_eval_constant_expression (&new_ctx, TREE_OPERAND (t, 0), > lval, > non_constant_p, overflow_p, > jump_target); > + > ctx->global->cleanups = prev_cleanups; > unsigned int i; > tree cleanup; > /* Evaluate the cleanups. */ > FOR_EACH_VEC_ELT_REVERSE (cleanups, i, cleanup) > - cxx_eval_constant_expression (ctx, cleanup, vc_discard, > + cxx_eval_constant_expression (&new_ctx, cleanup, vc_discard, > non_constant_p, overflow_p); > + > + /* Forget SAVE_EXPRs and TARGET_EXPRs created by this > + full-expression. */ > + for (tree save_expr : save_exprs) > + ctx->global->remove_value (save_expr); > } > break; > > @@ -7831,10 +7873,13 @@ cxx_eval_constant_expression (const constexpr_ctx > *ctx, tree t, > non_constant_p, overflow_p, jump_target); > > case BIND_EXPR: > - return cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t), > - lval, > - non_constant_p, overflow_p, > - jump_target); > + 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; > > case PREINCREMENT_EXPR: > case POSTINCREMENT_EXPR: > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index b74c18b03ad..3cc08da816f 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -2860,6 +2860,7 @@ struct GTY(()) lang_decl_base { > unsigned concept_p : 1; /* applies to vars and functions > */ > unsigned var_declared_inline_p : 1; /* var */ > unsigned dependent_init_p : 1; /* var */ > + unsigned expired_p : 1; /* var or parm */ > > /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > decls. */ > @@ -2871,7 +2872,7 @@ struct GTY(()) lang_decl_base { > /* VAR_DECL or FUNCTION_DECL has keyed decls. */ > unsigned module_keyed_decls_p : 1; > > - /* 12 spare bits. */ > + /* 11 spare bits. */ > }; > > /* True for DECL codes which have template info and access. */ > @@ -4366,6 +4367,13 @@ get_vec_init_expr (tree t) > #define SET_DECL_DEPENDENT_INIT_P(NODE, X) \ > (DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.dependent_init_p = (X)) > > +/* Nonzero if NODE is a VAR_DECL, PARM_DECL, or FIELD_DECL that is within > + its lifetime for constant evaluation purposes. */ > +#define DECL_EXPIRED_P(NODE) \ > + (DECL_LANG_SPECIFIC (NODE) && DECL_LANG_SPECIFIC (NODE)->u.base.expired_p) > +#define SET_DECL_EXPIRED_P(NODE, X) \ > + (DECL_LANG_SPECIFIC (NODE)->u.base.expired_p = (X)) > + > /* Nonzero if NODE is an artificial VAR_DECL for a C++17 structured binding > declaration or one of VAR_DECLs for the user identifiers in it. */ > #define DECL_DECOMPOSITION_P(NODE) \ > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index ac2fe66b080..7af43b5736d 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -5654,6 +5654,7 @@ trees_out::lang_decl_bools (tree t) > WB (lang->u.base.concept_p); > WB (lang->u.base.var_declared_inline_p); > WB (lang->u.base.dependent_init_p); > + WB (lang->u.base.expired_p); > /* When building a header unit, everthing is marked as purview, (so > we know which decls to write). But when we import them we do not > want to mark them as in module purview. */ > @@ -5728,6 +5729,7 @@ trees_in::lang_decl_bools (tree t) > RB (lang->u.base.concept_p); > RB (lang->u.base.var_declared_inline_p); > RB (lang->u.base.dependent_init_p); > + RB (lang->u.base.expired_p); > RB (lang->u.base.module_purview_p); > RB (lang->u.base.module_attach_p); > if (VAR_OR_FUNCTION_DECL_P (t)) > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C > b/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C > index e2d4853a284..ebaa95e5324 100644 > --- a/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C > @@ -4,4 +4,4 @@ > typedef bool (*Function)(int); > constexpr bool check(int x, Function p) { return p(x); } // { dg-message > "in .constexpr. expansion of" } > > -static_assert(check(2, check), ""); // { dg-error "conversion|constant|in > .constexpr. expansion of" } > +static_assert(check(2, check), ""); // { dg-error > "conversion|constant|lifetime|in .constexpr. expansion of" } > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C > new file mode 100644 > index 00000000000..43aa7c974c1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C > @@ -0,0 +1,13 @@ > +// PR c++/96630 > +// { dg-do compile { target c++14 } } > + > +struct S { > + int x = 0; > + constexpr const int& get() const { return x; } > +}; > + > +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" } > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C > new file mode 100644 > index 00000000000..22cd919fcda > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C > @@ -0,0 +1,20 @@ > +// PR c++/98675 > +// { dg-do compile { target c++14 } } > + > +struct S { > + int x = 0; > + constexpr const int& get() const { return x; } > +}; > + > +constexpr int error() { > + const auto& local = S{}.get(); // { dg-message "note: declared here" } > + return local; > +} > +constexpr int x = error(); // { dg-error "accessing object outside its > lifetime" } > + > +constexpr int ok() { > + // temporary should only be destroyed after end of full-expression > + auto local = S{}.get(); > + return local; > +} > +constexpr int y = ok(); > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C > new file mode 100644 > index 00000000000..6329f8cf6c6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C > @@ -0,0 +1,13 @@ > +// PR c++/70331 > +// { dg-do compile { target c++14 } } > + > +constexpr int f(int i) { > + int *p = &i; > + if (i == 0) { > + int j = 123; // { dg-message "note: declared here" } > + p = &j; > + } > + return *p; > +} > + > +constexpr int i = f(0); // { dg-error "accessing object outside its > lifetime" } > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C > new file mode 100644 > index 00000000000..181a1201663 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C > @@ -0,0 +1,11 @@ > +// { dg-do compile { target c++14 } } > + > +constexpr const double& test() { > + const double& local = 3.0; // { dg-message "note: declared here" } > + return local; > +} > + > +static_assert(test() == 3.0, ""); // { dg-error "constant|accessing object > outside its lifetime" } > + > +// no deference, shouldn't error > +static_assert((test(), true), ""); > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C > new file mode 100644 > index 00000000000..a4bc71d890a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C > @@ -0,0 +1,11 @@ > +// { dg-do compile { target c++14 } } > +// { dg-options "-Wno-return-local-addr" } > + > +constexpr const int& id(int x) { return x; } > + > +constexpr bool test() { > + const int& y = id(3); > + return y == 3; > +} > + > +constexpr bool x = test(); // { dg-error "" } > -- > 2.34.1 > >