On Thu, 11 May 2023, Patrick Palka wrote:
> r13-2701-g7107ea6fb933f1 made us correctly accept 'mutable' member
> accesses during constexpr evaluation of objects constructed during that
> evaluation, while continuing to reject such accesses for constexpr
> objects constructed outside of that evaluation, by considering the
> CONSTRUCTOR_MUTABLE_POISON flag during cxx_eval_component_reference.
>
> However, this flag is set only for the outermost CONSTRUCTOR of a
> constexpr variable initializer, so if we're accessing a 'mutable'
> subobject within a nested CONSTRUCTOR, the flag won't be set and
> we'll incorrectly accept the access. This can lead to us rejecting
> valid code, as in the first testcase, or even wrong code due to
d'oh, this should say "this can lead to us accepting invalid code"
> speculative constexpr evaluation as in the second and third testcase.
>
> This patch fixes this by setting CONSTRUCTOR_MUTABLE_POISON recursively
> rather than only on the outermost CONSTRUCTOR.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/13?
>
> PR c++/109745
>
> gcc/cp/ChangeLog:
>
> * typeck2.cc (poison_mutable_constructors): Define.
> (store_init_value): Use it instead of setting
> CONSTRUCTOR_MUTABLE_POISON directly.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-mutable4.C: New test.
> * g++.dg/cpp0x/constexpr-mutable5.C: New test.
> * g++.dg/cpp1y/constexpr-mutable2.C: New test.
> ---
> gcc/cp/typeck2.cc | 26 +++++++++++--
> .../g++.dg/cpp0x/constexpr-mutable4.C | 16 ++++++++
> .../g++.dg/cpp0x/constexpr-mutable5.C | 39 +++++++++++++++++++
> .../g++.dg/cpp1y/constexpr-mutable2.C | 20 ++++++++++
> 4 files changed, 97 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C
>
> diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> index f5cc7c8371c..8a187708482 100644
> --- a/gcc/cp/typeck2.cc
> +++ b/gcc/cp/typeck2.cc
> @@ -776,6 +776,27 @@ split_nonconstant_init (tree dest, tree init)
> return code;
> }
>
> +/* T is the initializer of a constexpr variable. Set
> CONSTRUCTOR_MUTABLE_POISON
> + for any CONSTRUCTOR within T that contains (directly or indirectly) a
> mutable
> + member, thereby poisoning it so it can't be copied to another a constexpr
> + variable, or read during constexpr evaluation. */
> +
> +static void
> +poison_mutable_constructors (tree t)
> +{
> + if (TREE_CODE (t) != CONSTRUCTOR)
> + return;
> +
> + if (cp_has_mutable_p (TREE_TYPE (t)))
> + {
> + CONSTRUCTOR_MUTABLE_POISON (t) = true;
> +
> + if (vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (t))
> + for (const constructor_elt &ce : *elts)
> + poison_mutable_constructors (ce.value);
> + }
> +}
> +
> /* Perform appropriate conversions on the initial value of a variable,
> store it in the declaration DECL,
> and print any error messages that are appropriate.
> @@ -886,10 +907,7 @@ store_init_value (tree decl, tree init, vec<tree,
> va_gc>** cleanups, int flags)
> else
> value = fold_non_dependent_init (value, tf_warning_or_error,
> /*manifestly_const_eval=*/true, decl);
> - if (TREE_CODE (value) == CONSTRUCTOR && cp_has_mutable_p (type))
> - /* Poison this CONSTRUCTOR so it can't be copied to another
> - constexpr variable. */
> - CONSTRUCTOR_MUTABLE_POISON (value) = true;
> + poison_mutable_constructors (value);
> const_init = (reduced_constant_expression_p (value)
> || error_operand_p (value));
> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = const_init;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C
> b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C
> new file mode 100644
> index 00000000000..01f32dea1bd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C
> @@ -0,0 +1,16 @@
> +// PR c++/109745
> +// { dg-do compile { target c++11 } }
> +
> +struct A { mutable int m = 0; };
> +
> +struct B { A a; };
> +
> +struct C { B b; };
> +
> +int main() {
> + constexpr B b;
> + constexpr int bam = b.a.m; // { dg-error "mutable" }
> +
> + constexpr C c;
> + constexpr int cbam = c.b.a.m; // { dg-error "mutable" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C
> b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C
> new file mode 100644
> index 00000000000..6a530e2abe6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C
> @@ -0,0 +1,39 @@
> +// PR c++/109745
> +// { dg-do run { target c++11 } }
> +// { dg-additional-options "-O" }
> +
> +struct A {
> + mutable int m = 0;
> + void f() const { ++m; };
> + constexpr int get_m() const { return m; }
> +};
> +
> +struct B { A a; };
> +
> +struct C { B b; };
> +
> +int main() {
> + constexpr A a;
> + a.m++;
> + if (a.get_m() != 1 || a.m != 1)
> + __builtin_abort();
> + a.m++;
> + if (a.get_m() != 2 || a.m != 2)
> + __builtin_abort();
> +
> + constexpr B b;
> + b.a.m++;
> + if (b.a.get_m() != 1 || b.a.m != 1)
> + __builtin_abort();
> + b.a.m++;
> + if (b.a.get_m() != 2 || b.a.m != 2)
> + __builtin_abort();
> +
> + constexpr C c;
> + c.b.a.m++;
> + if (c.b.a.get_m() != 1 || c.b.a.m != 1)
> + __builtin_abort();
> + c.b.a.m++;
> + if (c.b.a.get_m() != 2 || c.b.a.m != 2)
> + __builtin_abort();
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C
> b/gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C
> new file mode 100644
> index 00000000000..bb3429aea13
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C
> @@ -0,0 +1,20 @@
> +// PR c++/109745
> +// { dg-do run { target c++14 } }
> +// { dg-additional-options "-O" }
> +
> +template <typename T>
> +struct Foo { T val; };
> +
> +struct Bar {
> + constexpr Bar() = default;
> + constexpr Bar(Bar const& other) { other.val_ = 42; }
> + constexpr int val() const { return val_; }
> + mutable int val_{};
> +};
> +
> +int main() {
> + constexpr Foo<Bar> x{};
> + Foo<Bar> y{x};
> + if (x.val.val() != 42 || x.val.val_ != 42)
> + __builtin_abort();
> +}
> --
> 2.40.1.552.g91428f078b
>
>
