On 5/1/25 3:26 AM, Jakub Jelinek wrote:
Hi!

The following patch implements the C++26
P1061R10 - Structured Bindings can introduce a Pack
paper.
One thing unresolved in the patch is mangling, I've raised
https://github.com/itanium-cxx-abi/cxx-abi/issues/200
for that but no comments there yet.  One question is if it is ok
not to mention the fact that there is a structured binding pack in
the mangling of the structured bindings but more important is in case
of std::tuple* we might need to mangle individual structured binding
pack elements separately (each might need an exported name for the
var itself and perhaps its guard variable as well).  The patch just
uses the normal mangling for the whole structured bindings and emits
sorry if we need to mangle the structured binding pack elements.
The patch just marks the structured binding pack specially (considered
e.g. using some bit on it, but in the end I'm identifying it using
a made up type which causes DECL_PACK_P to be true; it is kind of
self-referential solution, because the type on the pack mentions the
DECL_DECOMPOSITION_P VAR_DECL on which the type is attached as its pack,
so it needs to be handled carefully during instantiation to avoid infinite
recursion, but it is the type that should be used if something else actually
needs to use the same type as the structured binding pack, e.g. a capture
proxy), and stores the pack elements when actually processed through
cp_finish_decomp with non-dependent initializer into a TREE_VEC used as
DECL_VALUE_EXPR of the pack; though because several spots use the
DECL_VALUE_EXPR and assume it is ARRAY_REF from which they can find out the
base variable and the index, it stores the base variable and index in the
first 2 TREE_VEC elts and has the structured binding elements only after
that.

This information about the DECL_VALUE_EXPR should be a comment somewhere, perhaps on the declaration of packv in cp_finish_decomp.

https://eel.is/c++draft/temp.dep.expr#3.6 says the packs are type dependent
regardless of whether the initializer of the structured binding is type
dependent or not, so I hope having a dependent type on the structured
binding VAR_DECL is ok.

That should be OK.

The paper also has an exception for sizeof... which is then not value
dependent when the structured bindings are initialized with non-dependent
initializer: https://eel.is/c++draft/temp.dep.constexpr#4
The patch special cases that in 3 spots (I've been wondering if e.g. during
parsing I couldn't just fold the sizeof... to the INTEGER_CST right away,
but guess I'd need to repeat that also during partial instantiation).

I imagine that could be factored into something that's called both during parsing and during instantiation, which is a common pattern?

And one thing still unresolved is debug info, I've just added DECL_IGNORED_P
on the structured binding pack VAR_DECL because there were ICEs with -g
for now, hope it can be fixed incrementally but am not sure what exactly
we should emit in the debug info for that.

The Clang mangling of the underlying variable seems fine, just mentioning the bound names; we can't get mangling collisions between pack and non-pack versions of the same name.

But It looks like they use .N discriminators for the individual elements, which is wrong because . is reserved for implementation details. But I'd think it should be fine to use [<discriminator>] instead.

Speaking of which, I see
DW_TAG_GNU_template_parameter_pack
DW_TAG_GNU_formal_parameter_pack
etc. DIEs emitted regardless of DWARF version, shouldn't we try to upstream
those into DWARF 6 or check what other compilers emit for the packs?

Done.

And bet we'd need DW_TAG_GNU_structured_binding_pack as well.

Hmm, probably.  I guess I should write that up as a follow-up...

@@ -21007,7 +21047,27 @@ tsubst_expr (tree t, tree args, tsubst_f
          ++c_inhibit_evaluation_warnings;
          /* We only want to compute the number of arguments.  */
          if (PACK_EXPANSION_P (op))
-           expanded = tsubst_pack_expansion (op, args, complain, in_decl);
+           {
+             expanded = NULL_TREE;
+             if (DECL_DECOMPOSITION_P (PACK_EXPANSION_PATTERN (op)))
+               {
+                 tree d = PACK_EXPANSION_PATTERN (op);
+                 if (DECL_HAS_VALUE_EXPR_P (d))
+                   {
+                     d = DECL_VALUE_EXPR (d);
+                     processing_template_decl_sentinel s;
+                     processing_template_decl = 1;
+                     if (TREE_CODE (d) == TREE_VEC
+                         && !type_dependent_expression_p (TREE_VEC_ELT (d, 0)))

You could use type_dependent_expression_p_push instead of messing with processing_template_decl here.

OK with this change and the comment on packv.

Jason

Reply via email to