Re: [PATCH] c++: Fix up mangling of function/block scope static structured bindings [PR111069]

[Jason Merrill via Gcc-patches]

On 8/22/23 04:12, Jakub Jelinek wrote:
As can be seen on the testcase, we weren't correctly mangling
static/thread_local structured bindings (C++20 feature) at function/block
scope.  The following patch fixes that by using what write_local_name
does for those cases (note, structured binding mandling doesn't use the
standard path because it needs to pass a list of all the identifiers in
the structured binding to the mangling).  In addition to that it fixes
mangling of various helpers which use write_guarded_name (_ZGV*, _ZTH*,
_ZTW*) and kills find_decomp_unqualified_name which for the local names
would be too hard to implement and uses write_guarded_name for structured
binding related _ZGR* names as well.
All the mangled names on the testcase match now clang++ and my expectations.
Because the old mangled names were plain wrong (they mangled the same as
structured binding at global scope and resulted in assembly errors if there
was more than one static structured binding with the same identifiers in
the same (or another) function, I think we don't need to play with another
mangling ABI level which turns on/off the old broken way, unsure whether
we should backport the patch to 13 branch though.

Probably not.

BTW, I think we should also handle ABI tags in mangle_decomp which we
currently don't do, but guess that should be subject to another mangling ABI
version.

I'd be surprised if this would affect any real code, but I suppose so. 
In any case I'd like to fix this at the same time as the local statics, 
to avoid changing their mangled name twice.

@@ -9049,6 +9050,25 @@ cp_maybe_mangle_decomp (tree decl, tree
        tree d = first;
        for (unsigned int i = 0; i < count; i++, d = DECL_CHAIN (d))
        v[count - i - 1] = d;
+      if (DECL_FUNCTION_SCOPE_P (decl))
+       {
+         size_t sz = 3;
+         for (unsigned int i = 0; i < count; ++i)
+           sz += IDENTIFIER_LENGTH (DECL_NAME (v[i])) + 1;
+         char *name = XALLOCAVEC (char, sz);
+         name[0] = 'D';
+         name[1] = 'C';
+         char *p = name + 2;
+         for (unsigned int i = 0; i < count; ++i)
+           {
+             size_t len = IDENTIFIER_LENGTH (DECL_NAME (v[i]));
+             *p++ = ' ';
+             memcpy (p, IDENTIFIER_POINTER (DECL_NAME (v[i])), len);
+             p += len;
+           }
+         *p = '\0';
+         determine_local_discriminator (decl, get_identifier (name));
+       }

Maybe do this in mangle_decomp, based on the actual mangling in process 
instead of this pseudo-mangling?

@@ -4564,6 +4519,13 @@ write_guarded_var_name (const tree varia
      /* The name of a guard variable for a reference temporary should refer
         to the reference, not the temporary.  */
      write_string (IDENTIFIER_POINTER (DECL_NAME (variable)) + 4);
+  else if (DECL_DECOMPOSITION_P (variable)
+          && DECL_NAME (variable) == NULL_TREE
+          && startswith (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (variable)),
+                         "_Z"))

Maybe add a startswith overload that takes an identifier?

@@ -4630,7 +4592,10 @@ mangle_ref_init_variable (const tree var
    start_mangling (variable);
    write_string ("_ZGR");
    check_abi_tags (variable);
-  write_name (variable, /*ignore_local_scope=*/0);
+  if (DECL_DECOMPOSITION_P (variable))
+    write_guarded_var_name (variable);
+  else
+    write_name (variable, /*ignore_local_scope=*/0);

Why not use write_guarded_name unconditionally?

Jason