On 8/14/24 3:41 AM, Jakub Jelinek wrote:
Hi!
The following patch partially implements CWG 2867
- Order of initialization for structured bindings.
The DR requires that initialization of e is sequenced before r_i and
that r_i initialization is sequenced before r_j for j > i, we already do it
that way, the former ordering is a necessity so that the get calls are
actually emitted on already initialized variable, the rest just because
we implemented it that way, by going through the structured binding
vars in ascending order and doing their initialization.
The hard part not implemented yet is the lifetime extension of the
temporaries from the e initialization to after the get calls (if any).
Unlike the range-for lifetime extension patch which I've posted recently
where IMO we can just ignore lifetime extension of reference bound
temporaries because all the temporaries are extended to the same spot,
here lifetime extension of reference bound temporaries should last until
the end of lifetime of e, while other temporaries only after all the get
calls.
The patch just attempts to deal with automatic structured bindings for now,
I'll post a patch for static locals incrementally and I don't have a patch
for namespace scope structured bindings yet, this patch should just keep
existing behavior for both static locals and namespace scope structured
bindings.
What GCC currently emits is a CLEANUP_POINT_EXPR around the e
initialization, followed optionally by nested CLEANUP_STMTs for cleanups
like the e dtor if any and dtors of lifetime extended temporaries from
reference binding; inside of the CLEANUP_STMT CLEANUP_BODY then the
initialization of the individual variables for the tuple case, again with
optional CLEANUP_STMT if e.g. lifetime extended temporaries from reference
binding are needed in those.
The following patch drops that first CLEANUP_POINT_EXPR and instead
wraps the whole sequence of the e initialization and the individual variable
initialization with get calls after it into a single CLEANUP_POINT_EXPR.
If there are any CLEANUP_STMTs needed, they are all emitted first, with
the CLEANUP_POINT_EXPR for e initialization and the individual variable
initialization inside of those, and a guard variable set after different
phases in those expressions guarding the corresponding cleanups, so that
they aren't invoked until the respective variables are constructed.
This is implemented by cp_finish_decl doing cp_finish_decomp on its own
when !processing_template_decl (otherwise we often don't cp_finish_decl
or process it at a different time from when we want to call
cp_finish_decomp) or unless the decl is erroneous (cp_finish_decl has
too many early returns for erroneous cases, and for those we can actually
call it even multiple times, for the non-erroneous cases
non-processing_template_decl cases we need to call it just once).
The two testcases try to construct various temporaries and variables and
verify the order in which the temporaries and variables are constructed and
destructed.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2024-08-14 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 bool 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 true. 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 TEST_P argument, change return type from
void to bool, if TEST_P, 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
unless range_decl is erroneous.
(cp_parser_decomposition_declaration): Set DECL_DECOMP_BASE
before cp_finish_decl call, call cp_finish_decomp after it only
if processing_template_decl or decl is erroneous.
(cp_finish_omp_range_for): Call cp_finish_decomp only if
processing_template_decl or decl is erroneous.
* pt.cc (tsubst_stmt): Likewise.
* g++.dg/DRs/dr2867-1.C: New test.
* g++.dg/DRs/dr2867-2.C: New test.
+ 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. */
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:
extern "C" void abort();
int c;
struct Px {
Px (int &) { if (c) throw 42; ++c; }
~Px () { --c; }
};
struct A {
Px &&i,&&j;
// Aggregate for lifetime extension
};
int main()
{
try {
int i,j;
A a {i,j};
} catch (...) { }
if (c) abort();
}
Here, the Px temporaries are reference-extended to the lifetime of A.
When the second one is constructed, it throws, but the first one is
never destroyed.
Seems like we need a whole different approach to ref-extended
temporaries. Perhaps by extending TARGET_EXPR to support extended
lifetime and delaying a lot of the reorganization to genericize or
gimplify time?
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?
Jason
