[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added reviewers: clang-vendors, rsmith, probinson, rjmccall.
Herald added a project: All.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://llvm.org/pr58819 - clang is giving an externally visible lambda in a static data member internal linkage and the wrong linkage name.

Looks like we should be classifying this case the same as a non-static data member, so far as I can tell from the ABI docs and template examples (seems like the non-template inline-defined case should be the same).

This is a change in ABI, but not sure it qualifies as an ABI break as far as Apple and Sony are concerned - do you folks want this change? (it should fix the example in the bug where a static member in such a lambda ends up bifurcated, and I don't /think/ it'll break existing code since the symbol was previously internal anyway)

Looks like GCC has got this mangling slightly wrong (so we'd still end up with GCC+Clang bifurcation of the local static in the lambda, function address inequality, etc) in that they miss the variable name in the mangling in the non-template case. GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107741


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138247

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCXX/mangle-lambdas.cpp


Index: clang/test/CodeGenCXX/mangle-lambdas.cpp
===================================================================
--- clang/test/CodeGenCXX/mangle-lambdas.cpp
+++ clang/test/CodeGenCXX/mangle-lambdas.cpp
@@ -290,6 +290,17 @@
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZZ3ft4IiEvvEN2lc2mfEiEd_NKUlvE_clEv
 
 
+extern int ExternalVariable;
+struct StaticInlineMember {
+  static constexpr auto x = [] { return ExternalVariable; };
+};
+
+// CHECK-LABEL: define void @_Z23test_StaticInlineMemberv
+// CHECK: call {{.*}} @_ZNK18StaticInlineMember1xMUlvE_clEv
+void test_StaticInlineMember() {
+  StaticInlineMember::x();
+}
+
 // Check linkage of the various lambdas.
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZ11inline_funciENKUlvE_clEv
 // CHECK: ret i32 1
Index: clang/lib/Sema/SemaLambda.cpp
===================================================================
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -296,10 +296,10 @@
         if (LexicalDC->isRecord())
           Kind = DefaultArgument;
     } else if (VarDecl *Var = dyn_cast<VarDecl>(ManglingContextDecl)) {
-      if (Var->getDeclContext()->isRecord())
-        Kind = StaticDataMember;
-      else if (Var->getMostRecentDecl()->isInline())
+      if (Var->getMostRecentDecl()->isInline())
         Kind = InlineVariable;
+      else if (Var->getDeclContext()->isRecord())
+        Kind = StaticDataMember;
       else if (Var->getDescribedVarTemplate())
         Kind = VariableTemplate;
       else if (auto *VTS = dyn_cast<VarTemplateSpecializationDecl>(Var)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138247.476287.patch
Type: text/x-patch
Size: 1573 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221118/f11de666/attachment.bin>