[RFC] [C+++20] [Modules] Mangling lambda in modules

[ChuanqiXu]

Summary

This tries to solve https://github.com/llvm/llvm-project/issues/52857.

Simply, the key reason is the unnamed class might be externally visible too in c++ modules! It is a little bit surprising to me at the first sight. Since how can an entity without a name to be visible. However, I recognize that it should be so otherwise the optimizer would lose a chance to do IPO (interprocedural optimization, like inlining). The corresponding code would be: https://github.com/llvm/llvm-project/blob/d29e319263de17516f50cd46edbf1e62c1289dd4/clang/lib/AST/Decl.cpp#L1258-L1259.

So I think we didn't met the problem before we introduces C++ modules since we didn't met externally visible unnamed class before.

To my understanding, changing the behavior of mangler is not trivial. It matters to ABI compatibility and other many tools like demangler and debugger. So I marked this one as RFC although it doesn't contain many codes.

[urnathan]

I don't think this is wanted. there are two cases:
a) lambda is attached to some ODR-visible entitity. Something like (in module-purview at namespace-scope) '[maybe-export] auto var = []{};'. Here the lambda acquires 'var' as its context.
b) lambda is not attached like that. p1815 makes it ill-formed for the lambda to be exposed externally in that case.

see https://github.com/itanium-cxx-abi/cxx-abi/issues/84

FWIW I'm redoing the module mangling scheme, the current one is undemangleable

[ChuanqiXu]

In D118018#3265810, @urnathan wrote:

I don't think this is wanted. there are two cases:
a) lambda is attached to some ODR-visible entitity. Something like (in module-purview at namespace-scope) '[maybe-export] auto var = []{};'. Here the lambda acquires 'var' as its context.
b) lambda is not attached like that. p1815 makes it ill-formed for the lambda to be exposed externally in that case.

see https://github.com/itanium-cxx-abi/cxx-abi/issues/84

FWIW I'm redoing the module mangling scheme, the current one is undemangleable

Oh, great to hear you are willing to take this. I know you must know much more about mangling than me. If you are going to refactor the mangling system, this one would be meaning less. I would abandon it later.

BTW, do you mind to be assigned for issue https://github.com/llvm/llvm-project/issues/52857 and https://github.com/llvm/llvm-project/issues/53232?

[urnathan]

sure, please reassign them

clang/lib/AST/ItaniumMangle.cpp

if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(TD)) {
            (!DeviceNumber && Record->getLambdaManglingNumber() > 0))) {
         assert(!AdditionalAbiTags &&
                "Lambda type cannot have additional abi tags");
         mangleLambda(Record);
         break;
       }
     }
 
-    if (TD->isExternallyVisible()) {
-      unsigned UnnamedMangle = getASTContext().getManglingNumber(TD);
-      Out << "Ut";
-      if (UnnamedMangle > 1)
-        Out << UnnamedMangle - 2;
-      Out << '_';
-      writeAbiTags(TD, AdditionalAbiTags);
-      break;
-    }
-
+    // Handle modules specially to avoid breaking ABI.
+    // Otherwise it is possible to mangle two different lambda to the same name,
+    // See issue 52857 for example.
+    if (!TD->isExternallyVisible() || TD->getOwningModuleForLinkage()) {
       // Get a unique id for the anonymous struct. If it is not a real output
       // ID doesn't matter so use fake one.
       unsigned AnonStructId = NullOut ? 0 : Context.getAnonymousStructId(TD);
 
       // Mangle it as a source name in the form
       // [n] $_<id>
       // where n is the length of the string.
       SmallString<8> Str;
       Str += "$_";
       Str += llvm::utostr(AnonStructId);
 
       Out << Str.size();
       Out << Str;
+    }
+
+    if (TD->isExternallyVisible()) {
+      unsigned UnnamedMangle = getASTContext().getManglingNumber(TD);
+      Out << "Ut";
+      if (UnnamedMangle > 1)
+        Out << UnnamedMangle - 2;
+      Out << '_';
+      writeAbiTags(TD, AdditionalAbiTags);
+    }
+
     break;
   }
 
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
     llvm_unreachable("Can't mangle Objective-C selector names here!");
 

clang/test/CodeGenCXX/cxx20-lambda-modules.cpp

+// Tests that the lambda in modules would be mangled properly.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "export module test;" >> %t/test.cppm
+// RUN: echo "export namespace test {" >> %t/test.cppm
+// RUN: echo "auto foo = [] {return 43;};" >> %t/test.cppm
+// RUN: echo "auto bar = [] {return 'a';};" >> %t/test.cppm
+// RUN: echo "}" >> %t/test.cppm
+// RUN: %clang_cc1  -std=c++20 %t/test.cppm -emit-module-interface -o %t/test.pcm
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fprebuilt-module-path=%t -std=c++20 %s -emit-llvm -o - | FileCheck %s
+import test;
+void use() {
+  test::foo();
+  test::bar();
+}
+// CHECK: call noundef i32 @"_ZW4testENK4test3$_0Ut_clEv"
+// CHECK: call noundef signext i8 @"_ZW4testENK4test3$_1Ut_clEv"