diff mbox series

[v2,1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675]

Message ID 20230329023258.13487-2-nathanieloshead@gmail.com
State New
Headers show
Series c++: Track lifetimes in constant evaluation [PR70331, ...] | expand

Commit Message

Nathaniel Shead March 29, 2023, 2:32 a.m. UTC 
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).

	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 <nathanieloshead@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

Comments

Patrick Palka June 23, 2023, 4:43 p.m. UTC | #1 
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 <nathanieloshead@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
> 
>
Nathaniel Shead June 24, 2023, 2:02 p.m. UTC | #2 
On Fri, Jun 23, 2023 at 12:43:21PM -0400, Patrick Palka wrote:
> 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 <nathanieloshead@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.

I've tried this but I haven't been able to get it to work well. The main
issue I'm running into is the caching of function calls in constant
evaluation. For example, consider the following:

    constexpr const double& test() {
      const double& local = 3.0;
      return local;
    }

    constexpr int foo(const double&) { return 5; }

    constexpr int a = foo(test());
    static_assert(test() == 3.0);

When constant-evaluating 'a', we evaluate 'test()'. It returns a value
that ends its lifetime immediately, so we mark this in 'ctx->global' as
expired. However, 'foo()' never actually evaluates this expired value,
so the initialisation of 'a' succeeds.

However, then when the static assertion attempts to constant evaluate a
second time, the result of 'test' has already been cached, and we just
get directly handed a value. This is a new constant evaluation, so
'ctx->global' has been reset, and because we just got the result of the
cached function we don't actually know whether this is expired or not
anymore, and so this compiles without any error in case it was valid.

I haven't yet been able to come up with a good way of avoiding this
issue without complicating the caching of call expressions overly much.
I suppose I could add an extra field to 'constexpr_call' to track if the
value has already been expired (which would solve this particular case),
but I'm worried that I'll overlook other cases that will sidestep this.

Do you have any other thoughts on the best approach here? Thanks.

> >      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
> > 
> > 
>
Patrick Palka June 26, 2023, 7:37 p.m. UTC | #3 
On Sun, 25 Jun 2023, Nathaniel Shead wrote:

> On Fri, Jun 23, 2023 at 12:43:21PM -0400, Patrick Palka wrote:
> > 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 <nathanieloshead@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.
> 
> I've tried this but I haven't been able to get it to work well. The main
> issue I'm running into is the caching of function calls in constant
> evaluation. For example, consider the following:
> 
>     constexpr const double& test() {
>       const double& local = 3.0;
>       return local;
>     }
> 
>     constexpr int foo(const double&) { return 5; }
> 
>     constexpr int a = foo(test());
>     static_assert(test() == 3.0);
> 
> When constant-evaluating 'a', we evaluate 'test()'. It returns a value
> that ends its lifetime immediately, so we mark this in 'ctx->global' as
> expired. However, 'foo()' never actually evaluates this expired value,
> so the initialisation of 'a' succeeds.
> 
> However, then when the static assertion attempts to constant evaluate a
> second time, the result of 'test' has already been cached, and we just
> get directly handed a value. This is a new constant evaluation, so
> 'ctx->global' has been reset, and because we just got the result of the
> cached function we don't actually know whether this is expired or not
> anymore, and so this compiles without any error in case it was valid.

Ouch, good catch..

> 
> I haven't yet been able to come up with a good way of avoiding this
> issue without complicating the caching of call expressions overly much.
> I suppose I could add an extra field to 'constexpr_call' to track if the
> value has already been expired (which would solve this particular case),
> but I'm worried that I'll overlook other cases that will sidestep this.
> 
> Do you have any other thoughts on the best approach here? Thanks.

This situation seems similar to that of a constexpr call returning a delete'd
pointer, which we handle by preventing caching of the call:

From constexpr.cc (cxx_eval_call_expression):
3207         /* Also don't cache a call that returns a deallocated pointer.  */
3208         if (cacheable && (cp_walk_tree_without_duplicates
3209                           (&result, find_heap_var_refs, NULL)))
3210           cacheable = false;

Maybe we could also disable caching in this situation as well, i.e. whenever a
constexpr call returns a reference to an expired variable?

> 
> > >      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
> > > 
> > > 
> > 
> 
>
Nathaniel Shead June 30, 2023, 3:45 p.m. UTC | #4 
On Mon, Jun 26, 2023 at 03:37:32PM -0400, Patrick Palka wrote:
> On Sun, 25 Jun 2023, Nathaniel Shead wrote:
> 
> > On Fri, Jun 23, 2023 at 12:43:21PM -0400, Patrick Palka wrote:
> > > 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 <nathanieloshead@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.
> > 
> > I've tried this but I haven't been able to get it to work well. The main
> > issue I'm running into is the caching of function calls in constant
> > evaluation. For example, consider the following:
> > 
> >     constexpr const double& test() {
> >       const double& local = 3.0;
> >       return local;
> >     }
> > 
> >     constexpr int foo(const double&) { return 5; }
> > 
> >     constexpr int a = foo(test());
> >     static_assert(test() == 3.0);
> > 
> > When constant-evaluating 'a', we evaluate 'test()'. It returns a value
> > that ends its lifetime immediately, so we mark this in 'ctx->global' as
> > expired. However, 'foo()' never actually evaluates this expired value,
> > so the initialisation of 'a' succeeds.
> > 
> > However, then when the static assertion attempts to constant evaluate a
> > second time, the result of 'test' has already been cached, and we just
> > get directly handed a value. This is a new constant evaluation, so
> > 'ctx->global' has been reset, and because we just got the result of the
> > cached function we don't actually know whether this is expired or not
> > anymore, and so this compiles without any error in case it was valid.
> 
> Ouch, good catch..
> 
> > 
> > I haven't yet been able to come up with a good way of avoiding this
> > issue without complicating the caching of call expressions overly much.
> > I suppose I could add an extra field to 'constexpr_call' to track if the
> > value has already been expired (which would solve this particular case),
> > but I'm worried that I'll overlook other cases that will sidestep this.
> > 
> > Do you have any other thoughts on the best approach here? Thanks.
> 
> This situation seems similar to that of a constexpr call returning a delete'd
> pointer, which we handle by preventing caching of the call:
> 
> From constexpr.cc (cxx_eval_call_expression):
> 3207         /* Also don't cache a call that returns a deallocated pointer.  */
> 3208         if (cacheable && (cp_walk_tree_without_duplicates
> 3209                           (&result, find_heap_var_refs, NULL)))
> 3210           cacheable = false;
> 
> Maybe we could also disable caching in this situation as well, i.e. whenever a
> constexpr call returns a reference to an expired variable?

Thanks for the pointer, I tried this and it works well. I was initially
a little worried about something similar happening with parameters but
it looks like that's already handled, since non-constant arguments also
already disable caching.

I'm still bootstrapping/regtesting but I'll send out a new version of
this patch series tomorrow when it's done.

> > 
> > > >      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
> > > > 
> > > > 
> > > 
> > 
> > 
>
diff mbox series

Patch

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);
+      }
     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 "" }