[cfe-dev] Mangling & linkage of unnamed but instantiated nested classes

[David Blaikie]

On Wed, Oct 31, 2012 at 2:19 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Wed, Oct 31, 2012 at 12:37 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> Hi John, (& cfe-dev)
>>
>> I mentioned this in person last week & wanted to provide you with some
>> more details & ask for your opinion.
>>
>> Backstory:
>>
>> I originally came across this while trying to use
>> -Wunused-member-function & found it was flagging code like this:
>>
>> struct foo {
>>   struct {
>>     void func() { ... } // warning that this is 'unused'
>>   } x;
>> };
>>
>> This surprised me, so I looked around & found that this function has
>> "no linkage". This seemed strange (because I would expect to be able
>> to call the function from multiple TUs that included the header
>> defining 'foo', compare its address for equality, & such things) & it
>> looks like the Right Thing is for func() to have the same linkage as,
>> say, an inline member function of 'foo' would have.
>>
>> The standard (3.5[basic.link]) seems to "miss" this case depending on
>> how you read it:
>>
>> 1) arguably p2, which says that "A name is said to have linkage when
>> it might denote the same ... function ... as a name introduced by a
>> declaration in another scope: - When a name has external linkage, the
>> entity it denotes can be referred to by names from scopes of other
>> translation units or from other scopes of the same translation unit"
>>
>> 2) p5: "... a member function ... has external linkage if the name of
>> the class has external linkage" (with an exception only for unnamed
>> classes (& enumerations) defined in class-scope typedef declarations
>> such that the class or enumeration has the dypedef name for linkage
>> purposes (7.1.3)) & there are no rules that seem to govern the linkage
>> of this unnamed class.
>>     p8 "Names not covered by these rules have no linkage"
>
> [basic.link]p8: "A type is said to have linkage if and only if [...]
> it is an unnamed class or enumeration member of a class with linkage".
>
> I'm going to assume it's an oversight in the standard that the members
> of such an unnamed class don't have linkage.
>
>> The next step was also to look at the mangling compared to GCC. After
>> modifying the linkage of functions like this (with this change):
>>
>>
>> --- lib/AST/Decl.cpp
>> +++ lib/AST/Decl.cpp
>> @@ -496,8 +496,7 @@ static LinkageInfo getLVForClassMember(const
>> NamedDecl *D, bool OnlyTemplate) {
>>    if (!(isa<CXXMethodDecl>(D) ||
>>          isa<VarDecl>(D) ||
>>          isa<FieldDecl>(D) ||
>> -        (isa<TagDecl>(D) &&
>> -         (D->getDeclName() || cast<TagDecl>(D)->getTypedefNameForAnonDecl()))))
>> +        isa<TagDecl>(D)))
>>      return LinkageInfo::none();
>>
>>    LinkageInfo LV;
>>
>> (& Richard reckons we might be able to simplify that check - just to
>> eliminate some template cases that still seem to get filtered out by
>> it)
>
> This looks like a step in the right direction.
>
>> I caused one test to fail:
>> test/CodeGenCXX/template-anonymous-types.cpp. A modified version (to
>> better investigate GCC 4.7's mangling) looks like this:
>>
>> struct S {
>>   enum { FOO = 42 };
>>   enum { BAR = 42 };
>> };
>>
>> struct T {
>>   enum { FOO = 42 };
>>   enum { BAR = 42 };
>> };
>>
>> template <typename T> struct X {
>>   T value;
>>   X(T t) : value(t) {}
>>   int f() { return value; }
>> };
>>
>> template <typename T> int f(T t) {
>>   X<T> x(t);
>>   return x.f();
>> }
>>
>> void test() {
>>   (void)f(S::FOO);
>>   (void)f(S::BAR);
>>   (void)f(T::FOO);
>>   (void)f(T::BAR);
>> }
>>
>> & with my change we get the right linkage for the instantiations of
>> 'f' (linkonce_odr instead of internal) but the mangling is still
>> inconsistent with GCC at least. Clang mangles these 4 'f's as:
>>
>> _Z1fIN1S3$_0EEiT_
>> _Z1fIN1S3$_1EEiT_
>> _Z1fIN1T3$_2EEiT_
>> _Z1fIN1T3$_3EEiT_
>>
>> GCC 4.7 mangles them as:
>>
>> _Z1fIN1SUt_EEiT_
>> _Z1fIN1SUt0_EEiT_
>> _Z1fIN1TUt_EEiT_
>> _Z1fIN1TUt0_EEiT_
>>
>> I don't think we have any class-specific unnamed nested type counter
>> that would implement that Ut_, Ut0_, ... mangling scheme, though I can
>> imagine where one might be added (I'm not very familiar with IRGen
>> though, so I'll certainly be happy to have any pointers about how that
>> could/should be implemented).
>
> We have a similar scheme already implemented for lambdas; look at
> getLambdaManglingNumber() etc.

Thanks for the pointer. Following the lines there I've had a first
blush at this (see attached).

I've not addressed some of the issues Richard brought up but I'd be
happy to take the time to generalize this to handle.

- David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mangle.diff
Type: application/octet-stream
Size: 7857 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121104/d6971b14/attachment.obj>