Re: [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675]
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
> ---
> 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
Re: [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675]
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
> > ---
> > 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 ca
Re: [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675]
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
> > > ---
> > > 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
Re: [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675]
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
> > > > ---
> > > > 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 assert
