[C++][Modules] Definition with same mangled name for template arguments of types defined in unnamed namespace in different files

[davidstone]

Given the following translation units:

export module a;

struct a_inner {
	~a_inner() {
	}
	void f(auto) {
	}
};

export template<typename T>
struct a {
	a() {
		struct local {};
		inner.f(local());
	}
private:
	a_inner inner;
};


namespace {

struct s {
};

} // namespace

void f() {
	a<s> x;
}

import a;

namespace {

struct s {
};

} // namespace

void g() {
	a<s> x;
}

clang fails with

/opt/compiler-explorer/clang-assertions-trunk/bin/clang++ --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot   -fcolor-diagnostics -fno-crash-diagnostics -std=c++20 -MD -MT CMakeFiles/foo.dir/b.cpp.o -MF CMakeFiles/foo.dir/b.cpp.o.d @CMakeFiles/foo.dir/b.cpp.o.modmap -o CMakeFiles/foo.dir/b.cpp.o -c /app/b.cpp
In file included from /app/b.cpp:1:
a.cpp:11:8: error: definition with same mangled name '_ZNW1a1aIN12_GLOBAL__N_11sEED2Ev' as another definition
   11 | struct a {
      |        ^
a.cpp:11:8: note: previous definition is here

See it live: https://godbolt.org/z/vsdznvsbP

[llvmbot]

@llvm/issue-subscribers-clang-modules

Author: David Stone (davidstone)

Given the following translation units:

export module a;

struct a_inner {
	~a_inner() {
	}
	void f(auto) {
	}
};

export template<typename T>
struct a {
	a() {
		struct local {};
		inner.f(local());
	}
private:
	a_inner inner;
};


namespace {

struct s {
};

} // namespace

void f() {
	a<s> x;
}

import a;

namespace {

struct s {
};

} // namespace

void g() {
	a<s> x;
}

clang fails with

/opt/compiler-explorer/clang-assertions-trunk/bin/clang++ --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot   -fcolor-diagnostics -fno-crash-diagnostics -std=c++20 -MD -MT CMakeFiles/foo.dir/b.cpp.o -MF CMakeFiles/foo.dir/b.cpp.o.d @<!-- -->CMakeFiles/foo.dir/b.cpp.o.modmap -o CMakeFiles/foo.dir/b.cpp.o -c /app/b.cpp
In file included from /app/b.cpp:1:
a.cpp:11:8: error: definition with same mangled name '_ZNW1a1aIN12_GLOBAL__N_11sEED2Ev' as another definition
   11 | struct a {
      |        ^
a.cpp:11:8: note: previous definition is here

See it live: https://godbolt.org/z/vsdznvsbP

[ChuanqiXu9]

Oh, name mangling problem. It looks like a hole in the current mangling system. (But I am not familiar with mangling too much) Let me try to send to this to Itanium CXX ABI Group. They are not so active now. And I really to don't want to touch name mangling without the guidance of Itanium CXX ABI .

[ChuanqiXu9]

After I review it again, I found this is not ABI related. It is forbidden by https://eel.is/c++draft/basic.link#15.4 if the instantiation point is not in a non-inline function body. (So the full explanation in https://eel.is/c++draft/basic.link#14).

To fix this, we have to not write instantiations in non-inline function body.

[jiixyj]

Could it be that this error just fires wrongly? If I comment out the code responsible for throwing a diag::err_duplicate_mangled_name in clang/lib/CodeGen/CodeGenModule.cpp the example compiles fine and looks correct. I.e.

c.cppm

module;
#include <iostream>
export module c;

struct a_inner {
  ~a_inner() {}
  void f(auto) {}
};

export template <typename T> struct a {
  a() {
    struct local {};
    inner.f(local());
    T();
  }

private:
  a_inner inner;
};

namespace {
struct s {
  s() { std::cerr << "s from c.cppm\n"; }
};
} // namespace

export void f() { a<s> x; }

main-119947.cpp

#include <iostream>
import c;

namespace {
struct s {
  s() { std::cerr << "s from main-119947.cpp\n"; }
};
} // namespace

void g() { a<s> x; }

int main() {
  g();
  f();
}

...prints:

s from main-119947.cpp
s from c.cppm

...as one would expect.

After I review it again, I found this is not ABI related. It is forbidden by https://eel.is/c++draft/basic.link#15.4 if the instantiation point is not in a non-inline function body. (So the full explanation in https://eel.is/c++draft/basic.link#14).

Is this program really ill-formed? Both struct s are TU-local, but there is no exposure anywhere, or is there?

[ChuanqiXu9]

Could it be that this error just fires wrongly? If I comment out the code responsible for throwing a diag::err_duplicate_mangled_name in clang/lib/CodeGen/CodeGenModule.cpp the example compiles fine and looks correct. I.e.

c.cppm

module;
#include
export module c;

struct a_inner {
~a_inner() {}
void f(auto) {}
};

export template struct a {
a() {
struct local {};
inner.f(local());
T();
}

private:
a_inner inner;
};

namespace {
struct s {
s() { std::cerr << "s from c.cppm\n"; }
};
} // namespace

export void f() { a x; }
main-119947.cpp

#include
import c;

namespace {
struct s {
s() { std::cerr << "s from main-119947.cpp\n"; }
};
} // namespace

void g() { a x; }

int main() {
g();
f();
}
...prints:

s from main-119947.cpp
s from c.cppm
...as one would expect.

After I review it again, I found this is not ABI related. It is forbidden by https://eel.is/c++draft/basic.link#15.4 if the instantiation point is not in a non-inline function body. (So the full explanation in https://eel.is/c++draft/basic.link#14).

Is this program really ill-formed? Both struct s are TU-local, but there is no exposure anywhere, or is there?

Have you tried reduced BMI?

[jiixyj]

Have you tried reduced BMI?

With -fmodules-reduced-bmi there still is the definition with same mangled name error.

To fix this, we have to not write instantiations in non-inline function body.

Just to better understand: I see there are two CodeGenModules when building the example, one for the .cppm and one for main:

[5/8] Building CXX object CMakeFiles/c.dir/c.cppm.o                                                                                                                                             
CG Context: 0x55d6a52e4ad0                                                                                                                                                                      
CG Context: 0x55d6a52e4ad0                                                                                                                                                                      
CG Context: 0x55d6a52e4ad0                                                                                                                                                                      
CG Context: 0x55d6a52e4ad0
[...]
[7/8] Building CXX object CMakeFiles/main-119947.dir/main-119947.cpp.o
CG Context: 0x5570bfb36420                                                                                                                                                                      
CG Context: 0x5570bfb36420                                                                                                                                                                      
CG Context: 0x5570bfb36420                                                                                                                                                                      
CG Context: 0x5570bfb36420                                                                                                                                                                      
CG Context: 0x5570bfb36420                                                                                                                                                                      
In file included from /home/jan/git/clang-bug/main-119947.cpp:2:                                                                                                                                
/home/jan/git/clang-bug/c.cppm:10:37: error: definition with same mangled name '_ZNW1c1aIN12_GLOBAL__N_11sEED2Ev' as another definition                                                         
   10 | export template <typename T> struct a {                                                                                                                                                 
      |                                     ^                                                                                                                                                   
/home/jan/git/clang-bug/c.cppm:10:37: note: previous definition is here                                                                                                                         
CG Context: 0x5570bfb36420                                                                                                                                                                      
CG Context: 0x5570bfb36420                                                                                                                                                                      
CG Context: 0x5570bfb36420
[...]

So what happens is that in the second CodeGenModule there also is the _ZNW1c1aIN12_GLOBAL__N_11sEED2Ev symbol imported from the BMI of the first compilation, right? And a proper fix would involve that symbol not being in the BMI, is this correct?

[ChuanqiXu9]

Have you tried reduced BMI?

With -fmodules-reduced-bmi there still is the definition with same mangled name error.

To fix this, we have to not write instantiations in non-inline function body.

Just to better understand: I see there are two CodeGenModules when building the example, one for the .cppm and one for main:

[5/8] Building CXX object CMakeFiles/c.dir/c.cppm.o
CG Context: 0x55d6a52e4ad0
CG Context: 0x55d6a52e4ad0
CG Context: 0x55d6a52e4ad0
CG Context: 0x55d6a52e4ad0
[...]
[7/8] Building CXX object CMakeFiles/main-119947.dir/main-119947.cpp.o
CG Context: 0x5570bfb36420
CG Context: 0x5570bfb36420
CG Context: 0x5570bfb36420
CG Context: 0x5570bfb36420
CG Context: 0x5570bfb36420
In file included from /home/jan/git/clang-bug/main-119947.cpp:2:
/home/jan/git/clang-bug/c.cppm:10:37: error: definition with same mangled name '_ZNW1c1aIN12_GLOBAL__N_11sEED2Ev' as another definition
10 | export template struct a {
| ^
/home/jan/git/clang-bug/c.cppm:10:37: note: previous definition is here
CG Context: 0x5570bfb36420
CG Context: 0x5570bfb36420
CG Context: 0x5570bfb36420
[...]
So what happens is that in the second CodeGenModule there also is the _ZNW1c1aIN12_GLOBAL__N_11sEED2Ev symbol imported from the BMI of the first compilation, right? And a proper fix would involve that symbol not being in the BMI, is this correct?

Correct. I think the proper fix is, with Reduced BMI, we shouldn't write all specializations. You can take a look at ASTDeclWriter::VisitClassTemplateDecl in clang/lib/Serialization/ASTWriterDecl.cpp, and you can see in the call to AddTemplateSpecializations(D);, we literally record all BMIs. But we shouldn't.