Hi!
The following patch extends the CWG 2867 support to function scope
static structured bindings.
The reason why I'm sending this separately is that I'm afraid it is an
ABI change (admittedly for a C++20 P1091R3 feature, but we accept it with
warning already in C++11).
The current state in both GCC and clang is that e.g. on the dr2867-3.C
bar case, there are _ZGVZ3barvEDC1x1y1z1wE and _ZGVZ3barvE1{x,y,z,w}
guard variables, _ZGVZ3barvEDC1x1y1z1wE guards the initialization of
the base (e in the standard), temporaries in that initialization are
destroyed before the guard is released, followed by acquiring
_ZGVZ3barvE1x guard, doing get for x and perhaps destroying its temporaries,
releasing that guard, acquiring _ZGVZ3barvE1y guard, etc.
So, in a multi-threaded program one thread can initialize e, another thread
x, another thread y, another thread z and another thread w. The
initialization is still correctly sequenced, other threads wait on the
prior guard variables before they can acquire another one, but if the
e initialization creates any temporaries (other than lifetime extended
reference binding which promotes the vars also to static), I wonder if it
is ok to initialize x etc. in a different thread.
The following patch takes the easy path and just does the cp_finish_decomp
initialization inside of a CLEANUP_POINT_EXPR which initializes e,
so guarded by _ZGVZ3barvEDC1x1y1z1wE. The ABI change is that if some
other thread in different compiler compiled code initializes e and
releases _ZGVZ3barvEDC1x1y1z1wE but doesn't initialize x, y, z and w,
while the current thread would expect _ZGVZ3barvEDC1x1y1z1wE to also
initialize x, y, z and w, nothing will initialize it.
Though, now that I think about it again, perhaps what we could do instead
is just make sure the _ZGVZ3barvEDC1x1y1z1wE initialization doesn't have
a CLEANUP_POINT_EXPR in it and wrap both the _ZGVZ3barvEDC1x1y1z1wE
and cp_finish_decomp created stuff into a single CLEANUP_POINT_EXPR.
That way, perhaps _ZGVZ3barvEDC1x1y1z1wE could be initialized by one thread
and _ZGVZ3barvE1x by a different, but the temporaries from
_ZGVZ3barvEDC1x1y1z1wE
initialization would be only destructed after the _ZGVZ3barvE1w guard
was released by the thread which initialized _ZGVZ3barvEDC1x1y1z1wE.
Anyway, the following passed bootstrap/regtest on x86_64-linux and
i686-linux, but I'll try the above variant next.
2024-08-14 Jakub Jelinek <ja...@redhat.com>
PR c++/115769
* decl.cc: Partially implement CWG 2867 - Order of initialization
for structured bindings.
(expand_static_init): Add DECOMP argument. If true, call
cp_finish_decomp and if needed integrate it into the guarded
sequence wrapped with CLEANUP_POINT_EXPR.
(cp_finish_decl): Adjust expand_static_init caller, for
tuple cases of structured bindings at function scope pass
non-NULL decomp and ensure cp_finish_decomp isn't called later.
* g++.dg/DRs/dr2867-3.C: New test.
* g++.dg/DRs/dr2867-4.C: New test.
--- gcc/cp/decl.cc.jj 2024-08-13 19:18:42.170052535 +0200
+++ gcc/cp/decl.cc 2024-08-13 21:05:42.875446138 +0200
@@ -103,7 +103,7 @@ static tree push_cp_library_fn (enum tre
static tree build_cp_library_fn (tree, enum tree_code, tree, int);
static void store_parm_decls (tree);
static void initialize_local_var (tree, tree, bool);
-static void expand_static_init (tree, tree);
+static void expand_static_init (tree, tree, cp_decomp *);
static location_t smallest_type_location (const cp_decl_specifier_seq*);
static bool identify_goto (tree, location_t, const location_t *,
diagnostic_t, bool);
@@ -9121,7 +9121,15 @@ cp_finish_decl (tree decl, tree init, bo
initializer. It is not legal to redeclare a static data
member, so this issue does not arise in that case. */
else if (var_definition_p && TREE_STATIC (decl))
- expand_static_init (decl, init);
+ {
+ if (need_decomp_init && init && DECL_FUNCTION_SCOPE_P (decl))
+ {
+ expand_static_init (decl, init, decomp);
+ need_decomp_init = false;
+ }
+ else
+ expand_static_init (decl, init, NULL);
+ }
}
/* If a CLEANUP_STMT was created to destroy a temporary bound to a
@@ -10183,7 +10191,7 @@ register_dtor_fn (tree decl)
and destruction of DECL. */
static void
-expand_static_init (tree decl, tree init)
+expand_static_init (tree decl, tree init, cp_decomp *decomp)
{
gcc_assert (VAR_P (decl));
gcc_assert (TREE_STATIC (decl));
@@ -10214,6 +10222,8 @@ expand_static_init (tree decl, tree init
"initialization and destruction");
informed = true;
}
+ if (decomp)
+ cp_finish_decomp (decl, decomp);
return;
}
@@ -10323,11 +10333,25 @@ expand_static_init (tree decl, tree init
variable. Do this before calling __cxa_guard_release. */
init = add_stmt_to_compound (init, register_dtor_fn (decl));
+ if (decomp)
+ {
+ tree sl = push_stmt_list ();
+ cp_finish_decomp (decl, decomp);
+ init = add_stmt_to_compound (init, pop_stmt_list (sl));
+ }
+
init = add_stmt_to_compound (init, build_call_n (release_fn, 1,
guard_addr));
}
else
{
+ if (decomp)
+ {
+ tree sl = push_stmt_list ();
+ cp_finish_decomp (decl, decomp);
+ init = add_stmt_to_compound (init, pop_stmt_list (sl));
+ }
+
init = add_stmt_to_compound (init, set_guard (guard));
/* Use atexit to register a function for destroying this static
--- gcc/testsuite/g++.dg/DRs/dr2867-3.C.jj 2024-08-13 21:05:42.876446125
+0200
+++ gcc/testsuite/g++.dg/DRs/dr2867-3.C 2024-08-13 21:05:42.876446125 +0200
@@ -0,0 +1,159 @@
+// CWG2867 - Order of initialization for structured bindings.
+// { dg-do run { target c++11 } }
+// { dg-options "" }
+
+#define assert(X) do { if (!(X)) __builtin_abort(); } while (0)
+
+namespace std {
+ template<typename T> struct tuple_size;
+ template<int, typename> struct tuple_element;
+}
+
+int a, c, d, i;
+
+struct A {
+ A () { assert (c == 3); ++c; }
+ ~A () { ++a; }
+ template <int I> int &get () const { assert (c == 5 + I); ++c; return i; }
+};
+
+template <> struct std::tuple_size <A> { static const int value = 4; };
+template <int I> struct std::tuple_element <I, A> { using type = int; };
+template <> struct std::tuple_size <const A> { static const int value = 4; };
+template <int I> struct std::tuple_element <I, const A> { using type = int; };
+
+struct B {
+ B () { assert (c >= 1 && c <= 2); ++c; }
+ ~B () { assert (c >= 9 && c <= 10); ++c; }
+};
+
+struct C {
+ constexpr C () {}
+ constexpr C (const C &) {}
+ template <int I> int &get () const { assert (d == 1 + I); ++d; return i; }
+};
+
+template <> struct std::tuple_size <C> { static const int value = 3; };
+template <int I> struct std::tuple_element <I, C> { using type = int; };
+template <> struct std::tuple_size <const C> { static const int value = 3; };
+template <int I> struct std::tuple_element <I, const C> { using type = int; };
+
+A
+foo (const B &, const B &)
+{
+ A a;
+ assert (c == 4);
+ ++c;
+ return a;
+}
+
+constexpr C
+foo (const C &, const C &)
+{
+ return C {};
+}
+
+int
+foo (const int &, const int &)
+{
+ assert (false);
+}
+
+inline void
+bar ()
+{
+ c = 1;
+ static const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (c == 11); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+ ++c;
+ d = 1;
+ static const auto &[s, t, u] = foo (C {}, C {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (d == 4); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+}
+
+template <int N>
+inline void
+baz ()
+{
+ c = 1;
+ static const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (c == 11); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+ ++c;
+ d = 1;
+ static const auto &[s, t, u] = foo (C {}, C {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (d == 4); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+}
+
+template <typename T, typename U>
+inline void
+qux ()
+{
+ c = 1;
+ static const auto &[x, y, z, w] = foo (T {}, T {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (c == 11); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+ ++c;
+ d = 1;
+ static const auto &[s, t, u] = foo (U {}, U {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (d == 4); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+}
+
+inline void
+corge ()
+{
+ c = 1;
+ static auto [x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured
bindings only available with" "" { target c++14_down } }
+ assert (c == 11); // { dg-warning "structured
binding declaration can be 'static' only in" "" { target c++17_down } .-1 }
+ ++c;
+ d = 1;
+ static auto [s, t, u] = foo (C {}, C {}); // { dg-warning "structured
bindings only available with" "" { target c++14_down } }
+ assert (d == 4); // { dg-warning "structured
binding declaration can be 'static' only in" "" { target c++17_down } .-1 }
+}
+
+template <int N>
+inline void
+garply ()
+{
+ c = 1;
+ static auto [x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured
bindings only available with" "" { target c++14_down } }
+ assert (c == 11); // { dg-warning "structured
binding declaration can be 'static' only in" "" { target c++17_down } .-1 }
+ ++c;
+ d = 1;
+ static auto [s, t, u] = foo (C {}, C {}); // { dg-warning "structured
bindings only available with" "" { target c++14_down } }
+ assert (d == 4); // { dg-warning "structured
binding declaration can be 'static' only in" "" { target c++17_down } .-1 }
+}
+
+template <typename T, typename U>
+inline void
+freddy ()
+{
+ c = 1;
+ static auto [x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured
bindings only available with" "" { target c++14_down } }
+ assert (c == 11); // { dg-warning "structured
binding declaration can be 'static' only in" "" { target c++17_down } .-1 }
+ ++c;
+ d = 1;
+ static auto [s, t, u] = foo (U {}, U {}); // { dg-warning "structured
bindings only available with" "" { target c++14_down } }
+ assert (d == 4); // { dg-warning "structured
binding declaration can be 'static' only in" "" { target c++17_down } .-1 }
+}
+
+struct E {
+ ~E () { assert (a == 6); }
+};
+
+int
+main ()
+{
+ static E e;
+ bar ();
+ assert (c == 12);
+ baz <0> ();
+ assert (c == 12);
+ qux <B, C> ();
+ assert (c == 12);
+ corge ();
+ assert (c == 12);
+ garply <42> ();
+ assert (c == 12);
+ freddy <B, C> ();
+ assert (c == 12);
+ assert (a == 0);
+}
--- gcc/testsuite/g++.dg/DRs/dr2867-4.C.jj 2024-08-13 21:05:42.876446125
+0200
+++ gcc/testsuite/g++.dg/DRs/dr2867-4.C 2024-08-13 21:05:42.876446125 +0200
@@ -0,0 +1,108 @@
+// CWG2867 - Order of initialization for structured bindings.
+// { dg-do run { target c++11 } }
+// { dg-options "" }
+
+#define assert(X) do { if (!(X)) __builtin_abort(); } while (0)
+
+namespace std {
+ template<typename T> struct tuple_size;
+ template<int, typename> struct tuple_element;
+}
+
+int a, c;
+
+struct C {
+ C () { assert (c >= 5 && c <= 17 && (c - 5) % 4 == 0); ++c; }
+ ~C () { assert (c >= 8 && c <= 20 && c % 4 == 0); ++c; }
+};
+
+struct D {
+ D () { assert (c >= 7 && c <= 19 && (c - 7) % 4 == 0); ++c; }
+ ~D () { assert (a % 5 != 4); ++a; }
+};
+
+struct A {
+ A () { assert (c == 3); ++c; }
+ ~A () { assert (a % 5 == 4); ++a; }
+ template <int I> D get (const C & = C{}) const { assert (c == 6 + 4 * I);
++c; return D {}; }
+};
+
+template <> struct std::tuple_size <A> { static const int value = 4; };
+template <int I> struct std::tuple_element <I, A> { using type = D; };
+template <> struct std::tuple_size <const A> { static const int value = 4; };
+template <int I> struct std::tuple_element <I, const A> { using type = D; };
+
+struct B {
+ B () { assert (c >= 1 && c <= 2); ++c; }
+ ~B () { assert (c >= 21 && c <= 22); ++c; }
+};
+
+A
+foo (const B &, const B &)
+{
+ A a;
+ assert (c == 4);
+ ++c;
+ return a;
+}
+
+int
+foo (const int &, const int &)
+{
+ assert (false);
+}
+
+inline void
+bar ()
+{
+ c = 1;
+ // First B::B () is invoked twice, then foo called, which invokes A::A ().
+ // e is reference bound to the A::A () constructed temporary.
+ // Then 4 times (in increasing I):
+ // C::C () is invoked, get is called, D::D () is invoked, C::~C () is
+ // invoked.
+ // After that B::~B () is invoked twice, then the following 2 user
+ // statements.
+ // At exit time D::~D () is invoked 4 times, then A::~A (), repeated 3
+ // times.
+ static const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (c == 23); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+ ++c;
+}
+
+template <int N>
+inline void
+baz ()
+{
+ c = 1;
+ static const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (c == 23); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+ ++c;
+}
+
+template <typename T>
+inline void
+qux ()
+{
+ c = 1;
+ static const auto &[x, y, z, w] = foo (T {}, T {}); // { dg-warning
"structured bindings only available with" "" { target c++14_down } }
+ assert (c == 23); // { dg-warning
"structured binding declaration can be 'static' only in" "" { target c++17_down
} .-1 }
+ ++c;
+}
+
+struct E {
+ ~E () { assert (a == 15); }
+};
+
+int
+main ()
+{
+ static E e;
+ bar ();
+ assert (c == 24);
+ baz <42> ();
+ assert (c == 24);
+ qux <B> ();
+ assert (c == 24);
+ assert (a == 0);
+}
Jakub
