On Wed, Aug 21, 2024 at 02:08:16PM -0400, Jason Merrill wrote: > I was concerned about the use of a single boolean to guard the destruction > of multiple objects, suspecting that it would break in obscure EH cases. > When I finally managed to construct a testcase that breaks, I was surprised > to see that it broke before this patch as well. And then I realized that it > breaks even without structured bindings:
Ouch. > In any case, we aren't going to address that in this patch. > > > + if (processing_template_decl || error_operand_p (decl)) > > + cp_finish_decomp (decl, &decomp); > > + && (processing_template_decl || error_operand_p (decl))) > > cp_finish_decomp (decl, decomp); > > Rather than all the callers needing to check this, how about changing the > new test_p flag to be tristate so that the second call from cp_finish_decl > is also indicated, and let cp_finish_decomp itself decide whether to > actually do anything? Here are 2 versions of the patch. Included here is a version which uses RAII to call cp_finish_decomp from cp_finish_decl and drops all cp_finish_decomp calls in the callers from after the cp_finish_decl call (which I like better), and attached is one which introduces a tristate argument to cp_finish_decomp. So far I've bootstrapped/regtested on x86_64-linux and i686-linux the RAII one. 2024-08-30 Jakub Jelinek <ja...@redhat.com> PR c++/115769 * cp-tree.h: Partially implement CWG 2867 - Order of initialization for structured bindings. (cp_finish_decomp): Add TEST_P argument defaulted to false. * decl.cc (initialize_local_var): Add DECOMP argument, if true, don't build cleanup and temporarily override stmts_are_full_exprs_p to 0 rather than 1. Formatting fix. (cp_finish_decl): Invoke cp_finish_decomp fpr structured bindings here if !processing_template_decl, first with test_p. For automatic structured binding bases if the test cp_finish_decomp returned true wrap the initialization together with what non-test cp_finish_decomp emits with a CLEANUP_POINT_EXPR, and if there are any CLEANUP_STMTs needed, emit them around the whole CLEANUP_POINT_EXPR with guard variables for the cleanups. Call cp_finish_decomp using RAII if not called with decomp != NULL otherwise. (cp_finish_decomp): Add TEST_P argument, change return type from void to bool, if TEST_P is true, return true instead of emitting actual code for the tuple case, otherwise return false. * parser.cc (cp_convert_range_for): Don't call cp_finish_decomp after cp_finish_decl. (cp_parser_decomposition_declaration): Set DECL_DECOMP_BASE before cp_finish_decl call. Don't call cp_finish_decomp after cp_finish_decl. (cp_finish_omp_range_for): Don't call cp_finish_decomp after cp_finish_decl. * pt.cc (tsubst_stmt): Likewise. * g++.dg/DRs/dr2867-1.C: New test. * g++.dg/DRs/dr2867-2.C: New test. --- gcc/cp/cp-tree.h.jj 2024-08-30 09:09:45.466623869 +0200 +++ gcc/cp/cp-tree.h 2024-08-30 11:00:39.861747964 +0200 @@ -7024,7 +7024,7 @@ extern void omp_declare_variant_finalize struct cp_decomp { tree decl; unsigned int count; }; extern void cp_finish_decl (tree, tree, bool, tree, int, cp_decomp * = nullptr); extern tree lookup_decomp_type (tree); -extern void cp_finish_decomp (tree, cp_decomp *); +extern bool cp_finish_decomp (tree, cp_decomp *, bool = false); extern int cp_complete_array_type (tree *, tree, bool); extern int cp_complete_array_type_or_error (tree *, tree, bool, tsubst_flags_t); extern tree build_ptrmemfunc_type (tree); --- gcc/cp/decl.cc.jj 2024-08-30 09:09:45.495623494 +0200 +++ gcc/cp/decl.cc 2024-08-30 11:11:51.554212784 +0200 @@ -103,7 +103,7 @@ static tree check_special_function_retur static tree push_cp_library_fn (enum tree_code, tree, int); 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); +static void initialize_local_var (tree, tree, bool); static void expand_static_init (tree, tree); static location_t smallest_type_location (const cp_decl_specifier_seq*); static bool identify_goto (tree, location_t, const location_t *, @@ -8050,14 +8050,13 @@ wrap_temporary_cleanups (tree init, tree /* Generate code to initialize DECL (a local variable). */ static void -initialize_local_var (tree decl, tree init) +initialize_local_var (tree decl, tree init, bool decomp) { tree type = TREE_TYPE (decl); tree cleanup; int already_used; - gcc_assert (VAR_P (decl) - || TREE_CODE (decl) == RESULT_DECL); + gcc_assert (VAR_P (decl) || TREE_CODE (decl) == RESULT_DECL); gcc_assert (!TREE_STATIC (decl)); if (DECL_SIZE (decl) == NULL_TREE) @@ -8077,7 +8076,8 @@ initialize_local_var (tree decl, tree in DECL_READ_P (decl) = 1; /* Generate a cleanup, if necessary. */ - cleanup = cxx_maybe_build_cleanup (decl, tf_warning_or_error); + cleanup = (decomp ? NULL_TREE + : cxx_maybe_build_cleanup (decl, tf_warning_or_error)); /* Perform the initialization. */ if (init) @@ -8112,10 +8112,16 @@ initialize_local_var (tree decl, tree in gcc_assert (building_stmt_list_p ()); saved_stmts_are_full_exprs_p = stmts_are_full_exprs_p (); - current_stmt_tree ()->stmts_are_full_exprs_p = 1; + /* Avoid CLEANUP_POINT_EXPR for the structured binding + bases, those will have CLEANUP_POINT_EXPR at the end of + code emitted by cp_finish_decomp. */ + if (decomp) + current_stmt_tree ()->stmts_are_full_exprs_p = 0; + else + current_stmt_tree ()->stmts_are_full_exprs_p = 1; finish_expr_stmt (init); - current_stmt_tree ()->stmts_are_full_exprs_p = - saved_stmts_are_full_exprs_p; + current_stmt_tree ()->stmts_are_full_exprs_p + = saved_stmts_are_full_exprs_p; } } @@ -8438,6 +8444,16 @@ cp_finish_decl (tree decl, tree init, bo int was_readonly = 0; bool var_definition_p = false; tree auto_node; + auto_vec<tree> extra_cleanups; + struct decomp_cleanup { + tree decl; + cp_decomp *decomp; + ~decomp_cleanup () + { + if (decomp && DECL_DECOMPOSITION_P (decl)) + cp_finish_decomp (decl, decomp); + } + } decomp_cl = { decl, decomp }; if (decl == error_mark_node) return; @@ -8924,6 +8940,8 @@ cp_finish_decl (tree decl, tree init, bo add_decl_expr (decl); } + bool need_decomp_init = false; + tree decomp_init = NULL_TREE; /* Let the middle end know about variables and functions -- but not static data members in uninstantiated class templates. */ if (VAR_OR_FUNCTION_DECL_P (decl)) @@ -8985,6 +9003,13 @@ cp_finish_decl (tree decl, tree init, bo if (var_definition_p) abstract_virtuals_error (decl, type); + if (decomp && !processing_template_decl) + { + need_decomp_init = cp_finish_decomp (decl, decomp, true); + if (!need_decomp_init) + decomp_cl.decomp = NULL; + } + if (TREE_TYPE (decl) == error_mark_node) /* No initialization required. */ ; @@ -9017,8 +9042,90 @@ cp_finish_decl (tree decl, tree init, bo } /* A variable definition. */ else if (DECL_FUNCTION_SCOPE_P (decl) && !TREE_STATIC (decl)) - /* Initialize the local variable. */ - initialize_local_var (decl, init); + { + /* Initialize the local variable. */ + if (!need_decomp_init) + initialize_local_var (decl, init, false); + else + { + tree cleanup = NULL_TREE; + if (DECL_SIZE (decl)) + cleanup = cxx_maybe_build_cleanup (decl, tf_warning_or_error); + /* If cp_finish_decomp needs to emit any code, we need to emit that + code after code emitted by initialize_local_var in a single + CLEANUP_POINT_EXPR, so that temporaries are destructed only + after the cp_finish_decomp emitted code. + If there are any cleanups, either extend_ref_init_temps + created ones or e.g. array destruction, push those first + with the cleanups guarded on a bool temporary, initially + set to false and set to true after initialize_local_var + emitted code. */ + tree guard = NULL_TREE; + if (cleanups || cleanup) + { + guard = force_target_expr (boolean_type_node, + boolean_false_node, tf_none); + add_stmt (guard); + guard = TARGET_EXPR_SLOT (guard); + } + tree sl = push_stmt_list (); + initialize_local_var (decl, init, true); + if (guard) + { + add_stmt (build2 (MODIFY_EXPR, boolean_type_node, + guard, boolean_true_node)); + for (tree &t : *cleanups) + t = build3 (COND_EXPR, void_type_node, + guard, t, void_node); + if (cleanup) + cleanup = build3 (COND_EXPR, void_type_node, + guard, cleanup, void_node); + } + unsigned before = stmt_list_stack->length (); + cp_finish_decomp (decl, decomp); + decomp_cl.decomp = NULL; + unsigned n_extra_cleanups = stmt_list_stack->length () - before; + sl = pop_stmt_list (sl); + need_decomp_init = false; + if (n_extra_cleanups) + { + /* If cp_finish_decomp needs any cleanups, such as for + extend_ref_init_temps created vars, pop_stmt_list + popped that all, so push those extra cleanups around + the whole sequence with a guard variable. */ + gcc_assert (TREE_CODE (sl) == STATEMENT_LIST); + guard = force_target_expr (integer_type_node, + integer_zero_node, tf_none); + add_stmt (guard); + guard = TARGET_EXPR_SLOT (guard); + for (unsigned i = 0; i < n_extra_cleanups; ++i) + { + tree_stmt_iterator tsi = tsi_last (sl); + gcc_assert (!tsi_end_p (tsi)); + tree last = tsi_stmt (tsi); + gcc_assert (TREE_CODE (last) == CLEANUP_STMT + && !CLEANUP_EH_ONLY (last)); + tree cst = build_int_cst (integer_type_node, i + 1); + tree cl = build3 (COND_EXPR, void_type_node, + build2 (GE_EXPR, boolean_type_node, + guard, cst), + CLEANUP_EXPR (last), void_node); + extra_cleanups.safe_push (cl); + tsi_link_before (&tsi, build2 (MODIFY_EXPR, + integer_type_node, + guard, cst), + TSI_SAME_STMT); + tree sl2 = CLEANUP_BODY (last); + gcc_assert (TREE_CODE (sl2) == STATEMENT_LIST); + tsi_link_before (&tsi, sl2, TSI_SAME_STMT); + tsi_delink (&tsi); + } + } + decomp_init = maybe_cleanup_point_expr_void (sl); + if (cleanup) + finish_decl_cleanup (decl, cleanup); + } + } /* If a variable is defined, and then a subsequent definition with external linkage is encountered, we will @@ -9045,6 +9152,17 @@ cp_finish_decl (tree decl, tree init, bo release_tree_vector (cleanups); } + for (tree t : &extra_cleanups) + push_cleanup (NULL_TREE, t, false); + + if (decomp_init) + add_stmt (decomp_init); + else if (need_decomp_init) + { + cp_finish_decomp (decl, decomp); + decomp_cl.decomp = NULL; + } + if (was_readonly) TREE_READONLY (decl) = 1; @@ -9327,10 +9445,11 @@ cp_maybe_mangle_decomp (tree decl, cp_de /* Finish a decomposition declaration. DECL is the underlying declaration "e", FIRST is the head of a chain of decls for the individual identifiers chained through DECL_CHAIN in reverse order and COUNT is the number of - those decls. */ + those decls. If TEST_P is true, return true if any code would need to be + actually emitted but don't emit it. Return false otherwise. */ -void -cp_finish_decomp (tree decl, cp_decomp *decomp) +bool +cp_finish_decomp (tree decl, cp_decomp *decomp, bool test_p) { tree first = decomp->decl; unsigned count = decomp->count; @@ -9349,7 +9468,7 @@ cp_finish_decomp (tree decl, cp_decomp * } if (DECL_P (decl) && DECL_NAMESPACE_SCOPE_P (decl)) SET_DECL_ASSEMBLER_NAME (decl, get_identifier ("<decomp>")); - return; + return false; } location_t loc = DECL_SOURCE_LOCATION (decl); @@ -9373,7 +9492,7 @@ cp_finish_decomp (tree decl, cp_decomp * fit_decomposition_lang_decl (first, decl); first = DECL_CHAIN (first); } - return; + return false; } auto_vec<tree, 16> v; @@ -9515,6 +9634,8 @@ cp_finish_decomp (tree decl, cp_decomp * eltscnt = tree_to_uhwi (tsize); if (count != eltscnt) goto cnt_mismatch; + if (test_p) + return true; if (!processing_template_decl && DECL_DECOMP_BASE (decl)) { /* For structured bindings used in conditions we need to evaluate @@ -9666,6 +9787,7 @@ cp_finish_decomp (tree decl, cp_decomp * DECL_HAS_VALUE_EXPR_P (v[i]) = 1; } } + return false; } /* Returns a declaration for a VAR_DECL as if: --- gcc/cp/parser.cc.jj 2024-08-30 09:40:26.583785645 +0200 +++ gcc/cp/parser.cc 2024-08-30 11:13:18.612089567 +0200 @@ -14513,8 +14513,6 @@ cp_convert_range_for (tree statement, tr cp_finish_decl (range_decl, deref_begin, /*is_constant_init*/false, NULL_TREE, LOOKUP_ONLYCONVERTING, decomp); - if (DECL_DECOMPOSITION_P (range_decl)) - cp_finish_decomp (range_decl, decomp); warn_for_range_copy (range_decl, deref_begin); @@ -16429,13 +16427,12 @@ cp_parser_decomposition_declaration (cp_ if (decl != error_mark_node) { cp_decomp decomp = { prev, cnt }; - cp_finish_decl (decl, initializer, non_constant_p, NULL_TREE, - (is_direct_init ? LOOKUP_NORMAL : LOOKUP_IMPLICIT), - &decomp); if (keyword != RID_MAX) DECL_DECOMP_BASE (decl) = keyword == RID_SWITCH ? integer_one_node : integer_zero_node; - cp_finish_decomp (decl, &decomp); + cp_finish_decl (decl, initializer, non_constant_p, NULL_TREE, + (is_direct_init ? LOOKUP_NORMAL : LOOKUP_IMPLICIT), + &decomp); } } else if (decl != error_mark_node) @@ -44810,8 +44807,6 @@ cp_finish_omp_range_for (tree orig, tree NULL_TREE, tf_warning_or_error), /*is_constant_init*/false, NULL_TREE, LOOKUP_ONLYCONVERTING, decomp); - if (DECL_DECOMPOSITION_P (decl)) - cp_finish_decomp (decl, decomp); } /* Return true if next tokens contain a standard attribute that contains --- gcc/cp/pt.cc.jj 2024-08-30 09:09:45.608622030 +0200 +++ gcc/cp/pt.cc 2024-08-30 11:16:33.291577816 +0200 @@ -18633,7 +18633,6 @@ tsubst_stmt (tree t, tree args, tsubst_f { bool const_init = false; cp_decomp decomp_d, *decomp = NULL; - tree ndecl = error_mark_node; tree asmspec_tree = NULL_TREE; maybe_push_decl (decl); @@ -18646,8 +18645,10 @@ tsubst_stmt (tree t, tree args, tsubst_f && TREE_TYPE (pattern_decl) != error_mark_node) { decomp = &decomp_d; - ndecl = tsubst_decomp_names (decl, pattern_decl, args, - complain, in_decl, decomp); + if (tsubst_decomp_names (decl, pattern_decl, args, + complain, in_decl, decomp) + == error_mark_node) + decomp = NULL; } init = tsubst_init (init, decl, args, complain, in_decl); @@ -18674,9 +18675,6 @@ tsubst_stmt (tree t, tree args, tsubst_f cp_finish_decl (decl, init, const_init, asmspec_tree, 0, decomp); - - if (ndecl != error_mark_node) - cp_finish_decomp (ndecl, decomp); } } } --- gcc/testsuite/g++.dg/DRs/dr2867-1.C.jj 2024-08-30 10:02:41.838454529 +0200 +++ gcc/testsuite/g++.dg/DRs/dr2867-1.C 2024-08-30 10:02:41.838454529 +0200 @@ -0,0 +1,153 @@ +// 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 c, d, i; + +struct A { + A () { assert (c == 3); ++c; } + ~A () { assert (c == 12); ++c; } + 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); +} + +void +bar () +{ + c = 1; + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + const auto &[s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <int N> +void +baz () +{ + c = 1; + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + const auto &[s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <typename T, typename U> +void +qux () +{ + c = 1; + const auto &[x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + const auto &[s, t, u] = foo (U {}, U {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +void +corge () +{ + c = 1; + auto [x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + auto [s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <int N> +void +garply () +{ + c = 1; + auto [x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + auto [s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <typename T, typename U> +void +freddy () +{ + c = 1; + auto [x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + auto [s, t, u] = foo (U {}, U {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +int +main () +{ + bar (); + assert (c == 13); + baz <0> (); + assert (c == 13); + qux <B, C> (); + assert (c == 13); + corge (); + assert (c == 13); + garply <42> (); + assert (c == 13); + freddy <B, C> (); + assert (c == 13); +} --- gcc/testsuite/g++.dg/DRs/dr2867-2.C.jj 2024-08-30 10:02:41.838454529 +0200 +++ gcc/testsuite/g++.dg/DRs/dr2867-2.C 2024-08-30 10:02:41.838454529 +0200 @@ -0,0 +1,101 @@ +// 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 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 (c >= 24 && c <= 27); ++c; } +}; + +struct A { + A () { assert (c == 3); ++c; } + ~A () { assert (c == 28); ++c; } + 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); +} + +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. + // Then D::~D () is invoked 4 times, then A::~A (). + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 23); + ++c; +} + +template <int N> +void +baz () +{ + c = 1; + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 23); + ++c; +} + +template <typename T> +void +qux () +{ + c = 1; + const auto &[x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 23); + ++c; +} + +int +main () +{ + bar (); + assert (c == 29); + baz <42> (); + assert (c == 29); + qux <B> (); + assert (c == 29); +} Jakub
2024-08-30 Jakub Jelinek <ja...@redhat.com> PR c++/115769 * cp-tree.h: Partially implement CWG 2867 - Order of initialization for structured bindings. (enum cfd_mode): New type. (cp_finish_decomp): Add MODE argument defaulted to cfd_normal. * decl.cc (initialize_local_var): Add DECOMP argument, if true, don't build cleanup and temporarily override stmts_are_full_exprs_p to 0 rather than 1. Formatting fix. (cp_finish_decl): Invoke cp_finish_decomp fpr structured bindings here if !processing_template_decl, first with cfd_test. For automatic structured binding bases if the test cp_finish_decomp returned true wrap the initialization together with what non-test cp_finish_decomp emits with a CLEANUP_POINT_EXPR, and if there are any CLEANUP_STMTs needed, emit them around the whole CLEANUP_POINT_EXPR with guard variables for the cleanups. (cp_finish_decomp): Add MODE argument, change return type from void to bool, if cfd_test, return true instead of emitting actual code for the tuple case, otherwise return false. If cfd_post_finish_decl, return early unless error_operand_p or processing_template_decl. * parser.cc (cp_convert_range_for): Pass cfd_post_finish_decl to cp_finish_decomp. (cp_parser_decomposition_declaration): Set DECL_DECOMP_BASE before cp_finish_decl call, pass cfd_post_finish_decl to cp_finish_decomp. (cp_finish_omp_range_for): Pass cfd_post_finish_decl to cp_finish_decomp. * pt.cc (tsubst_stmt): Likewise. * g++.dg/DRs/dr2867-1.C: New test. * g++.dg/DRs/dr2867-2.C: New test. --- gcc/cp/cp-tree.h.jj 2024-08-30 09:09:45.466623869 +0200 +++ gcc/cp/cp-tree.h 2024-08-30 10:14:04.530573733 +0200 @@ -7024,7 +7024,17 @@ extern void omp_declare_variant_finalize struct cp_decomp { tree decl; unsigned int count; }; extern void cp_finish_decl (tree, tree, bool, tree, int, cp_decomp * = nullptr); extern tree lookup_decomp_type (tree); -extern void cp_finish_decomp (tree, cp_decomp *); +enum cfd_mode { + /* Perform all structured binding finalization. */ + cfd_normal, + /* Likewise, but if any code would need to be emitted for the tuple case, + return true and don't emit any code. */ + cfd_test, + /* For calls done after cp_finish_decomp just to handle cases + cp_finish_decomp possibly missed. */ + cfd_post_finish_decl +}; +extern bool cp_finish_decomp (tree, cp_decomp *, cfd_mode = cfd_normal); extern int cp_complete_array_type (tree *, tree, bool); extern int cp_complete_array_type_or_error (tree *, tree, bool, tsubst_flags_t); extern tree build_ptrmemfunc_type (tree); --- gcc/cp/decl.cc.jj 2024-08-30 09:09:45.495623494 +0200 +++ gcc/cp/decl.cc 2024-08-30 10:17:34.170860878 +0200 @@ -103,7 +103,7 @@ static tree check_special_function_retur static tree push_cp_library_fn (enum tree_code, tree, int); 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); +static void initialize_local_var (tree, tree, bool); static void expand_static_init (tree, tree); static location_t smallest_type_location (const cp_decl_specifier_seq*); static bool identify_goto (tree, location_t, const location_t *, @@ -8050,14 +8050,13 @@ wrap_temporary_cleanups (tree init, tree /* Generate code to initialize DECL (a local variable). */ static void -initialize_local_var (tree decl, tree init) +initialize_local_var (tree decl, tree init, bool decomp) { tree type = TREE_TYPE (decl); tree cleanup; int already_used; - gcc_assert (VAR_P (decl) - || TREE_CODE (decl) == RESULT_DECL); + gcc_assert (VAR_P (decl) || TREE_CODE (decl) == RESULT_DECL); gcc_assert (!TREE_STATIC (decl)); if (DECL_SIZE (decl) == NULL_TREE) @@ -8077,7 +8076,8 @@ initialize_local_var (tree decl, tree in DECL_READ_P (decl) = 1; /* Generate a cleanup, if necessary. */ - cleanup = cxx_maybe_build_cleanup (decl, tf_warning_or_error); + cleanup = (decomp ? NULL_TREE + : cxx_maybe_build_cleanup (decl, tf_warning_or_error)); /* Perform the initialization. */ if (init) @@ -8112,10 +8112,16 @@ initialize_local_var (tree decl, tree in gcc_assert (building_stmt_list_p ()); saved_stmts_are_full_exprs_p = stmts_are_full_exprs_p (); - current_stmt_tree ()->stmts_are_full_exprs_p = 1; + /* Avoid CLEANUP_POINT_EXPR for the structured binding + bases, those will have CLEANUP_POINT_EXPR at the end of + code emitted by cp_finish_decomp. */ + if (decomp) + current_stmt_tree ()->stmts_are_full_exprs_p = 0; + else + current_stmt_tree ()->stmts_are_full_exprs_p = 1; finish_expr_stmt (init); - current_stmt_tree ()->stmts_are_full_exprs_p = - saved_stmts_are_full_exprs_p; + current_stmt_tree ()->stmts_are_full_exprs_p + = saved_stmts_are_full_exprs_p; } } @@ -8438,6 +8444,7 @@ cp_finish_decl (tree decl, tree init, bo int was_readonly = 0; bool var_definition_p = false; tree auto_node; + auto_vec<tree> extra_cleanups; if (decl == error_mark_node) return; @@ -8924,6 +8931,8 @@ cp_finish_decl (tree decl, tree init, bo add_decl_expr (decl); } + bool need_decomp_init = false; + tree decomp_init = NULL_TREE; /* Let the middle end know about variables and functions -- but not static data members in uninstantiated class templates. */ if (VAR_OR_FUNCTION_DECL_P (decl)) @@ -8985,6 +8994,9 @@ cp_finish_decl (tree decl, tree init, bo if (var_definition_p) abstract_virtuals_error (decl, type); + if (decomp && !processing_template_decl) + need_decomp_init = cp_finish_decomp (decl, decomp, cfd_test); + if (TREE_TYPE (decl) == error_mark_node) /* No initialization required. */ ; @@ -9017,8 +9029,89 @@ cp_finish_decl (tree decl, tree init, bo } /* A variable definition. */ else if (DECL_FUNCTION_SCOPE_P (decl) && !TREE_STATIC (decl)) - /* Initialize the local variable. */ - initialize_local_var (decl, init); + { + /* Initialize the local variable. */ + if (!need_decomp_init) + initialize_local_var (decl, init, false); + else + { + tree cleanup = NULL_TREE; + if (DECL_SIZE (decl)) + cleanup = cxx_maybe_build_cleanup (decl, tf_warning_or_error); + /* If cp_finish_decomp needs to emit any code, we need to emit that + code after code emitted by initialize_local_var in a single + CLEANUP_POINT_EXPR, so that temporaries are destructed only + after the cp_finish_decomp emitted code. + If there are any cleanups, either extend_ref_init_temps + created ones or e.g. array destruction, push those first + with the cleanups guarded on a bool temporary, initially + set to false and set to true after initialize_local_var + emitted code. */ + tree guard = NULL_TREE; + if (cleanups || cleanup) + { + guard = force_target_expr (boolean_type_node, + boolean_false_node, tf_none); + add_stmt (guard); + guard = TARGET_EXPR_SLOT (guard); + } + tree sl = push_stmt_list (); + initialize_local_var (decl, init, true); + if (guard) + { + add_stmt (build2 (MODIFY_EXPR, boolean_type_node, + guard, boolean_true_node)); + for (tree &t : *cleanups) + t = build3 (COND_EXPR, void_type_node, + guard, t, void_node); + if (cleanup) + cleanup = build3 (COND_EXPR, void_type_node, + guard, cleanup, void_node); + } + unsigned before = stmt_list_stack->length (); + cp_finish_decomp (decl, decomp); + unsigned n_extra_cleanups = stmt_list_stack->length () - before; + sl = pop_stmt_list (sl); + need_decomp_init = false; + if (n_extra_cleanups) + { + /* If cp_finish_decomp needs any cleanups, such as for + extend_ref_init_temps created vars, pop_stmt_list + popped that all, so push those extra cleanups around + the whole sequence with a guard variable. */ + gcc_assert (TREE_CODE (sl) == STATEMENT_LIST); + guard = force_target_expr (integer_type_node, + integer_zero_node, tf_none); + add_stmt (guard); + guard = TARGET_EXPR_SLOT (guard); + for (unsigned i = 0; i < n_extra_cleanups; ++i) + { + tree_stmt_iterator tsi = tsi_last (sl); + gcc_assert (!tsi_end_p (tsi)); + tree last = tsi_stmt (tsi); + gcc_assert (TREE_CODE (last) == CLEANUP_STMT + && !CLEANUP_EH_ONLY (last)); + tree cst = build_int_cst (integer_type_node, i + 1); + tree cl = build3 (COND_EXPR, void_type_node, + build2 (GE_EXPR, boolean_type_node, + guard, cst), + CLEANUP_EXPR (last), void_node); + extra_cleanups.safe_push (cl); + tsi_link_before (&tsi, build2 (MODIFY_EXPR, + integer_type_node, + guard, cst), + TSI_SAME_STMT); + tree sl2 = CLEANUP_BODY (last); + gcc_assert (TREE_CODE (sl2) == STATEMENT_LIST); + tsi_link_before (&tsi, sl2, TSI_SAME_STMT); + tsi_delink (&tsi); + } + } + decomp_init = maybe_cleanup_point_expr_void (sl); + if (cleanup) + finish_decl_cleanup (decl, cleanup); + } + } /* If a variable is defined, and then a subsequent definition with external linkage is encountered, we will @@ -9045,6 +9138,14 @@ cp_finish_decl (tree decl, tree init, bo release_tree_vector (cleanups); } + for (tree t : &extra_cleanups) + push_cleanup (NULL_TREE, t, false); + + if (decomp_init) + add_stmt (decomp_init); + else if (need_decomp_init) + cp_finish_decomp (decl, decomp); + if (was_readonly) TREE_READONLY (decl) = 1; @@ -9327,10 +9428,13 @@ cp_maybe_mangle_decomp (tree decl, cp_de /* Finish a decomposition declaration. DECL is the underlying declaration "e", FIRST is the head of a chain of decls for the individual identifiers chained through DECL_CHAIN in reverse order and COUNT is the number of - those decls. */ + those decls. If MODE is cfd_test, return true if any code would need to be + actually emitted but don't emit it. Return false otherwise. + If MODE is cfd_post_finish_decl, only handle the cases cp_finish_decl + possibly didn't. */ -void -cp_finish_decomp (tree decl, cp_decomp *decomp) +bool +cp_finish_decomp (tree decl, cp_decomp *decomp, cfd_mode mode) { tree first = decomp->decl; unsigned count = decomp->count; @@ -9349,9 +9453,15 @@ cp_finish_decomp (tree decl, cp_decomp * } if (DECL_P (decl) && DECL_NAMESPACE_SCOPE_P (decl)) SET_DECL_ASSEMBLER_NAME (decl, get_identifier ("<decomp>")); - return; + return false; } + /* cp_finish_decl should have called cp_finish_decomp with cfd_normal + mode already for all !processing_template_decl cases, except when + decl is erroneous. */ + if (mode == cfd_post_finish_decl && !processing_template_decl) + return false; + location_t loc = DECL_SOURCE_LOCATION (decl); if (type_dependent_expression_p (decl) /* This happens for range for when not in templates. @@ -9373,7 +9483,7 @@ cp_finish_decomp (tree decl, cp_decomp * fit_decomposition_lang_decl (first, decl); first = DECL_CHAIN (first); } - return; + return false; } auto_vec<tree, 16> v; @@ -9515,6 +9625,8 @@ cp_finish_decomp (tree decl, cp_decomp * eltscnt = tree_to_uhwi (tsize); if (count != eltscnt) goto cnt_mismatch; + if (mode == cfd_test) + return true; if (!processing_template_decl && DECL_DECOMP_BASE (decl)) { /* For structured bindings used in conditions we need to evaluate @@ -9666,6 +9778,7 @@ cp_finish_decomp (tree decl, cp_decomp * DECL_HAS_VALUE_EXPR_P (v[i]) = 1; } } + return false; } /* Returns a declaration for a VAR_DECL as if: --- gcc/cp/parser.cc.jj 2024-08-30 09:40:26.583785645 +0200 +++ gcc/cp/parser.cc 2024-08-30 10:20:00.348983715 +0200 @@ -14514,7 +14514,7 @@ cp_convert_range_for (tree statement, tr /*is_constant_init*/false, NULL_TREE, LOOKUP_ONLYCONVERTING, decomp); if (DECL_DECOMPOSITION_P (range_decl)) - cp_finish_decomp (range_decl, decomp); + cp_finish_decomp (range_decl, decomp, cfd_post_finish_decl); warn_for_range_copy (range_decl, deref_begin); @@ -16429,13 +16429,13 @@ cp_parser_decomposition_declaration (cp_ if (decl != error_mark_node) { cp_decomp decomp = { prev, cnt }; - cp_finish_decl (decl, initializer, non_constant_p, NULL_TREE, - (is_direct_init ? LOOKUP_NORMAL : LOOKUP_IMPLICIT), - &decomp); if (keyword != RID_MAX) DECL_DECOMP_BASE (decl) = keyword == RID_SWITCH ? integer_one_node : integer_zero_node; - cp_finish_decomp (decl, &decomp); + cp_finish_decl (decl, initializer, non_constant_p, NULL_TREE, + (is_direct_init ? LOOKUP_NORMAL : LOOKUP_IMPLICIT), + &decomp); + cp_finish_decomp (decl, &decomp, cfd_post_finish_decl); } } else if (decl != error_mark_node) @@ -44811,7 +44811,7 @@ cp_finish_omp_range_for (tree orig, tree /*is_constant_init*/false, NULL_TREE, LOOKUP_ONLYCONVERTING, decomp); if (DECL_DECOMPOSITION_P (decl)) - cp_finish_decomp (decl, decomp); + cp_finish_decomp (decl, decomp, cfd_post_finish_decl); } /* Return true if next tokens contain a standard attribute that contains --- gcc/cp/pt.cc.jj 2024-08-30 09:09:45.608622030 +0200 +++ gcc/cp/pt.cc 2024-08-30 10:20:46.841386680 +0200 @@ -18676,7 +18676,7 @@ tsubst_stmt (tree t, tree args, tsubst_f decomp); if (ndecl != error_mark_node) - cp_finish_decomp (ndecl, decomp); + cp_finish_decomp (ndecl, decomp, cfd_post_finish_decl); } } } --- gcc/testsuite/g++.dg/DRs/dr2867-1.C.jj 2024-08-30 10:02:41.838454529 +0200 +++ gcc/testsuite/g++.dg/DRs/dr2867-1.C 2024-08-30 10:02:41.838454529 +0200 @@ -0,0 +1,153 @@ +// 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 c, d, i; + +struct A { + A () { assert (c == 3); ++c; } + ~A () { assert (c == 12); ++c; } + 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); +} + +void +bar () +{ + c = 1; + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + const auto &[s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <int N> +void +baz () +{ + c = 1; + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + const auto &[s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <typename T, typename U> +void +qux () +{ + c = 1; + const auto &[x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + const auto &[s, t, u] = foo (U {}, U {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +void +corge () +{ + c = 1; + auto [x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + auto [s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <int N> +void +garply () +{ + c = 1; + auto [x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + auto [s, t, u] = foo (C {}, C {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +template <typename T, typename U> +void +freddy () +{ + c = 1; + auto [x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 11); + ++c; + d = 1; + auto [s, t, u] = foo (U {}, U {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (d == 4); +} + +int +main () +{ + bar (); + assert (c == 13); + baz <0> (); + assert (c == 13); + qux <B, C> (); + assert (c == 13); + corge (); + assert (c == 13); + garply <42> (); + assert (c == 13); + freddy <B, C> (); + assert (c == 13); +} --- gcc/testsuite/g++.dg/DRs/dr2867-2.C.jj 2024-08-30 10:02:41.838454529 +0200 +++ gcc/testsuite/g++.dg/DRs/dr2867-2.C 2024-08-30 10:02:41.838454529 +0200 @@ -0,0 +1,101 @@ +// 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 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 (c >= 24 && c <= 27); ++c; } +}; + +struct A { + A () { assert (c == 3); ++c; } + ~A () { assert (c == 28); ++c; } + 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); +} + +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. + // Then D::~D () is invoked 4 times, then A::~A (). + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 23); + ++c; +} + +template <int N> +void +baz () +{ + c = 1; + const auto &[x, y, z, w] = foo (B {}, B {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 23); + ++c; +} + +template <typename T> +void +qux () +{ + c = 1; + const auto &[x, y, z, w] = foo (T {}, T {}); // { dg-warning "structured bindings only available with" "" { target c++14_down } } + assert (c == 23); + ++c; +} + +int +main () +{ + bar (); + assert (c == 29); + baz <42> (); + assert (c == 29); + qux <B> (); + assert (c == 29); +}