Invalid mangled names emitted for local types declared in Apple Block expressions used in non-static data member initializers

Clang appears to be generating ill-formed mangled names for local types
declared in Apple Block expressions used as non-static data member
initializers:

$ cat t.cpp
template<typename T> void tf(T) {}
struct S {
   void(^bp)() = ^{
     enum E {} e;
     tf(e);
   };
};
auto x = S{}.bp;

$ clang++ --version
clang version 4.0.0 (tags/RELEASE_400/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
...

$ clang++ -c -std=c++11 -fblocks t.cpp
$ nm t.o
...
0000000000000000 W _Z2tfIZ1S2bpMUb0_E1EEvT_
...

The mangled name generated for the function template specialization
instantiated for the local enum type 'E' uses a <pointer-to-member-type>
production [1] (as signified by 'M'), but only one of the two required
dependent productions is present (<class type> via 'Ub0_', but not
<member type>).

If someone can confirm that this is not intentional (it could be since
Apple Blocks aren't governed by the Itanium ABI), I'll file a new BZ for
this.

Additionally, for unnamed local types, discriminators are not added to
disambiguate such types. Bug 34511 [2] has been filed for that issue.

Tom.

[1]:
http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.pointer-to-member-type
[2]: https://bugs.llvm.org/show_bug.cgi?id=34511

Clang appears to be generating ill-formed mangled names for local types
declared in Apple Block expressions used as non-static data member
initializers:

$ cat t.cpp
template<typename T> void tf(T) {}
struct S {
   void(^bp)() = ^{
     enum E {} e;
     tf(e);
   };
};
auto x = S{}.bp;

$ clang++ --version
clang version 4.0.0 (tags/RELEASE_400/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
...

$ clang++ -c -std=c++11 -fblocks t.cpp
$ nm t.o
...
0000000000000000 W _Z2tfIZ1S2bpMUb0_E1EEvT_
...

The mangled name generated for the function template specialization
instantiated for the local enum type 'E' uses a <pointer-to-member-type>
production [1] (as signified by 'M'),

I believe you're mistaken. This is not a context in which a <type>
production can appear. That's a <data-member-prefix>:

http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.data-member-prefix

... which specifies a context of the default member initializer of S::bp.

The following construct (the Ub0_) is a vendor extension for naming a block
closure.

but only one of the two required

dependent productions is present (<class type> via 'Ub0_', but not
<member type>).

If someone can confirm that this is not intentional (it could be since
Apple Blocks aren't governed by the Itanium ABI), I'll file a new BZ for
this.

This behavior looks correct to me, except that I'm surprised that this
block is numbered as Ub0_ instead of Ub_. Looks like we have an off-by-one
error here, introduced in r214699 while fixing a different off-by-one error.

John: should we restore the pre-r214699 ABI, or would you prefer that we
just accept the new mangling as our ABI now?

Nothing about blocks ever has identity across translation units, so there’s no harm in fixing the bug and restoring the original ABI to start counting at b_.

John.

On Sep 6, 2017, at 5:50 PM, Richard Smith <richard@metafoo.co.uk
    The mangled name generated for the function template specialization
    instantiated for the local enum type 'E' uses a
    <pointer-to-member-type>
    production [1] (as signified by 'M'),

I believe you're mistaken. This is not a context in which a <type>
production can appear. That's a <data-member-prefix>:

http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.data-member-prefix

Ah, so it is. Thanks for the correction.

This behavior looks correct to me, except that I'm surprised that this
block is numbered as Ub0_ instead of Ub_. Looks like we have an
off-by-one error here, introduced in r214699 while fixing a different
off-by-one error.

John: should we restore the pre-r214699 ABI, or would you prefer that
we just accept the new mangling as our ABI now?

Nothing about blocks ever has identity across translation units, so
there's no harm in fixing the bug and restoring the original ABI to
start counting at b_.

That doesn't seem right to me. For example, aren't static local
variables declared within block expressions in inline functions required
to match across TUs?

$ cat t.h
inline int f() {
   return ^{
     static int slv;
     return slv++;
   }();
}

$ cat t1.cpp
#include "t.h"
int f1() {
   return f();
}

$ cat t2.cpp
#include "t.h"
int f2() {
   return f();
}

$ clang++ -c -std=c++11 -fblocks -O3 t1.cpp t2.cpp

$ nm t1.o
0000000000000000 T _Z2f1v
0000000000000000 V _ZZZ1fvEUb0_E3slv

$ nm t2.o
0000000000000000 T _Z2f2v
0000000000000000 V _ZZZ1fvEUb0_E3slv

Compiling with -O3 results in f() being inlined in f1() and f2() such
that each TU resolves the reference to 'slv' without dispatch through f().

https://godbolt.org/g/w3rQMy

Granted, this is a fairly contrived example, so restoring the original
ABI doesn't strike me as very likely to cause problems.

Tom.

On Sep 6, 2017, at 5:50 PM, Richard Smith <richard@metafoo.co.uk
   The mangled name generated for the function template specialization
   instantiated for the local enum type 'E' uses a
   <pointer-to-member-type>
   production [1] (as signified by 'M'),

I believe you're mistaken. This is not a context in which a <type>
production can appear. That's a <data-member-prefix>:

http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.data-member-prefix

Ah, so it is. Thanks for the correction.

This behavior looks correct to me, except that I'm surprised that this
block is numbered as Ub0_ instead of Ub_. Looks like we have an
off-by-one error here, introduced in r214699 while fixing a different
off-by-one error.

John: should we restore the pre-r214699 ABI, or would you prefer that
we just accept the new mangling as our ABI now?

Nothing about blocks ever has identity across translation units, so
there's no harm in fixing the bug and restoring the original ABI to
start counting at b_.

That doesn't seem right to me. For example, aren't static local
variables declared within block expressions in inline functions required
to match across TUs?

Ah, yes, you're right; I was thinking of the data directly associated with the block
without considering its contents.

Granted, this is a fairly contrived example, so restoring the original
ABI doesn't strike me as very likely to cause problems.

Yeah, I think it's better to just fix the bug. I'm not too worried about this level of contrivance.

John.

>>
>>> On Sep 6, 2017, at 5:50 PM, Richard Smith <richard@metafoo.co.uk
>>> The mangled name generated for the function template specialization
>>> instantiated for the local enum type 'E' uses a
>>> <pointer-to-member-type>
>>> production [1] (as signified by 'M'),
>>>
>>>
>>> I believe you're mistaken. This is not a context in which a <type>
>>> production can appear. That's a <data-member-prefix>:
>>>
>>> http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.
data-member-prefix
>
> Ah, so it is. Thanks for the correction.
>
>>> This behavior looks correct to me, except that I'm surprised that this
>>> block is numbered as Ub0_ instead of Ub_. Looks like we have an
>>> off-by-one error here, introduced in r214699 while fixing a different
>>> off-by-one error.
>>>
>>> John: should we restore the pre-r214699 ABI, or would you prefer that
>>> we just accept the new mangling as our ABI now?
>>
>> Nothing about blocks ever has identity across translation units, so
>> there's no harm in fixing the bug and restoring the original ABI to
>> start counting at b_.
>
> That doesn't seem right to me. For example, aren't static local
> variables declared within block expressions in inline functions required
> to match across TUs?

Ah, yes, you're right; I was thinking of the data directly associated with
the block
without considering its contents.

> Granted, this is a fairly contrived example, so restoring the original
> ABI doesn't strike me as very likely to cause problems.

Yeah, I think it's better to just fix the bug. I'm not too worried about
this level of contrivance.

OK, done in r312700.