[PATCH] D118018: [RFC] [C+++20] [Modules] Mangling lambda in modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: urnathan, aaron.ballman, dblaikie, rsmith, erichkeane, efriedma, gbenyei.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118018

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/cxx20-lambda-modules.cpp


Index: clang/test/CodeGenCXX/cxx20-lambda-modules.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/cxx20-lambda-modules.cpp
@@ -0,0 +1,18 @@
+// 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"
Index: clang/lib/AST/ItaniumMangle.cpp
===================================================================
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -1537,6 +1537,25 @@
       }
     }
 
+    // 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";
@@ -1544,22 +1563,8 @@
         Out << UnnamedMangle - 2;
       Out << '_';
       writeAbiTags(TD, AdditionalAbiTags);
-      break;
     }
 
-    // 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;
     break;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118018.402420.patch
Type: text/x-patch
Size: 2629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220124/953674a0/attachment.bin>