[clang] f01364e - [Clang] Instantiate the correct lambda call operator (#110446)

[via cfe-commits]

Author: Sirraide
Date: 2024-10-08T13:34:00+02:00
New Revision: f01364ebc88d875fbfc0e00413538465aaa8a2b3

URL: https://github.com/llvm/llvm-project/commit/f01364ebc88d875fbfc0e00413538465aaa8a2b3
DIFF: https://github.com/llvm/llvm-project/commit/f01364ebc88d875fbfc0e00413538465aaa8a2b3.diff

LOG: [Clang] Instantiate the correct lambda call operator (#110446)

This is a fix for the following issue: when a lambda’s class type is
merged across modules (e.g. because it is defined in a template in the
GMF of some module `A`, and some other module `B` both imports `A` and
has the same template in its GMF), then `getLambdaCallOperator()` might
return the wrong operator (e.g. while compiling `B`, the lambda’s class
type would be the one attached to `B`’s GMF, but the call operator ends
up being the one attached to `A`’s GMF).

This causes issues in situations where the call operator is in a
template and accesses declarations in the surrounding context: when
those declarations are instantated, a mapping is introduced from the
original node in the template to that of the instantiation. If such an
instantiation happens in `B`, and we then try to instantiate `A`’s call
operator, any nodes in that call operator refer to declarations in the
template in `A`, but the `LocalInstantiationScope` only contains
mappings for declarations in `B`! This causes the following assertion
(for godbolt links and more, see the issue below):

```
Assertion `isa<LabelDecl>(D) && "declaration not instantiated in this scope"' failed.
```

We now walk the redecl chain of the call operator to find the
one that is in the same module as the record decl.

This fixes #110401.

Added: 
    clang/test/Modules/gh110401.cppm

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/AST/DeclCXX.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 05bd74710dd7c8..21c6e3c98e87a4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -479,6 +479,8 @@ Bug Fixes to C++ Support
   conformance of explicit instantiation behaviour with MSVC. (#GH111266)
 - Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly
   in certain friend declarations. (#GH93099)
+- Clang now instantiates the correct lambda call operator when a lambda's class type is
+  merged across modules. (#GH110401)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index f5a0aa8f82512e..1364ccc745ba01 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1631,13 +1631,42 @@ static bool allLookupResultsAreTheSame(const DeclContext::lookup_result &R) {
 static NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl &RD) {
   if (!RD.isLambda()) return nullptr;
   DeclarationName Name =
-    RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
-  DeclContext::lookup_result Calls = RD.lookup(Name);
+      RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
 
+  DeclContext::lookup_result Calls = RD.lookup(Name);
   assert(!Calls.empty() && "Missing lambda call operator!");
   assert(allLookupResultsAreTheSame(Calls) &&
          "More than one lambda call operator!");
-  return Calls.front();
+
+  // FIXME: If we have multiple call operators, we might be in a situation
+  // where we merged this lambda with one from another module; in that
+  // case, return our method (instead of that of the other lambda).
+  //
+  // This avoids situations where, given two modules A and B, if we
+  // try to instantiate A's call operator in a function in B, anything
+  // in the call operator that relies on local decls in the surrounding
+  // function will crash because it tries to find A's decls, but we only
+  // instantiated B's:
+  //
+  //   template <typename>
+  //   void f() {
+  //     using T = int;      // We only instantiate B's version of this.
+  //     auto L = [](T) { }; // But A's call operator would want A's here.
+  //   }
+  //
+  // Walk the call operator’s redecl chain to find the one that belongs
+  // to this module.
+  //
+  // TODO: We need to fix this properly (see
+  // https://github.com/llvm/llvm-project/issues/90154).
+  Module *M = RD.getOwningModule();
+  for (Decl *D : Calls.front()->redecls()) {
+    auto *MD = cast<NamedDecl>(D);
+    if (MD->getOwningModule() == M)
+      return MD;
+  }
+
+  llvm_unreachable("Couldn't find our call operator!");
 }
 
 FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator() const {

diff  --git a/clang/test/Modules/gh110401.cppm b/clang/test/Modules/gh110401.cppm
new file mode 100644
index 00000000000000..6b335eb5ba9d55
--- /dev/null
+++ b/clang/test/Modules/gh110401.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux -emit-module-interface %t/a.cppm -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux -emit-module-interface -fprebuilt-module-path=%t %t/b.cppm -o %t/B.pcm
+
+// Just check that this doesn't crash.
+
+//--- a.cppm
+module;
+
+template <typename _Visitor>
+void __do_visit(_Visitor &&__visitor) {
+  using _V0 = int;
+  [](_V0 __v) -> _V0 { return __v; } (1);
+}
+
+export module A;
+
+void g() {
+  struct Visitor { };
+  __do_visit(Visitor());
+}
+
+//--- b.cppm
+module;
+
+template <typename _Visitor>
+void __do_visit(_Visitor &&__visitor) {
+  using _V0 = int;
+
+  // Check that we instantiate this lambda's call operator in 'f' below
+  // instead of the one in 'a.cppm' here; otherwise, we won't find a
+  // corresponding instantiation of the using declaration above.
+  [](_V0 __v) -> _V0 { return __v; } (1);
+}
+
+export module B;
+import A;
+
+void f() {
+  __do_visit(1);
+}