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

[Eli Friedman]

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.

-Eli