https://github.com/Sirraide updated
https://github.com/llvm/llvm-project/pull/110446
>From a61f3e94d0636f4294f3be440d79b83889771800 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalm...@gmail.com>
Date: Mon, 30 Sep 2024 02:58:31 +0200
Subject: [PATCH 1/3] [Clang] Instantiate the correct lambda call operator
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/AST/DeclCXX.cpp | 27 ++++++++++++++++++--
clang/test/Modules/gh110401.cppm | 44 ++++++++++++++++++++++++++++++++
3 files changed, 70 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Modules/gh110401.cppm
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 14907e7db18de3..17a0c45d98c5c8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -426,6 +426,7 @@ Bug Fixes to C++ Support
- Fixed an assertion failure in debug mode, and potential crashes in release
mode, when
diagnosing a failed cast caused indirectly by a failed implicit conversion
to the type of the constructor parameter.
- Fixed an assertion failure by adjusting integral to boolean vector
conversions (#GH108326)
+- 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 01143391edab40..af9c644337c84d 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1631,9 +1631,32 @@ 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);
+
+ // 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 wants A's here.
+ // }
+ //
+ // To mitigate this, search our own decls first.
+ // FIXME: This feels like a hack; is there a better way of doing this?
+ for (CXXMethodDecl *M : RD.methods())
+ if (M->getDeclName() == Name)
+ return M;
+ // Otherwise, do a full lookup to find external decls too.
+ DeclContext::lookup_result Calls = RD.lookup(Name);
assert(!Calls.empty() && "Missing lambda call operator!");
assert(allLookupResultsAreTheSame(Calls) &&
"More than one lambda call operator!");
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);
+}
>From d3b5acc0a43bf227d57b3d7f1c68acedb89e3123 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalm...@gmail.com>
Date: Mon, 30 Sep 2024 17:34:43 +0200
Subject: [PATCH 2/3] Walk redecl chain to find the right operator
---
clang/lib/AST/DeclCXX.cpp | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index af9c644337c84d..da7a8ac0e5fe0f 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1633,6 +1633,11 @@ static NamedDecl* getLambdaCallOperatorHelper(const
CXXRecordDecl &RD) {
DeclarationName 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!");
+
// 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).
@@ -1646,21 +1651,19 @@ static NamedDecl* getLambdaCallOperatorHelper(const
CXXRecordDecl &RD) {
// template <typename>
// void f() {
// using T = int; // We only instantiate B's version of this.
- // auto L = [](T) { }; // But A's call operator wants A's here.
+ // auto L = [](T) { }; // But A's call operator would want A's here.
// }
//
- // To mitigate this, search our own decls first.
- // FIXME: This feels like a hack; is there a better way of doing this?
- for (CXXMethodDecl *M : RD.methods())
- if (M->getDeclName() == Name)
- return M;
+ // Walk the call operator’s redecl chain to find the one that belongs
+ // to this module.
+ Module *M = RD.getOwningModule();
+ for (Decl *D : Calls.front()->redecls()) {
+ auto *MD = cast<NamedDecl>(D);
+ if (MD->getOwningModule() == M)
+ return MD;
+ }
- // Otherwise, do a full lookup to find external decls too.
- 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();
+ llvm_unreachable("Couldn't find our call operator!");
}
FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator() const {
>From 1cc387a50b24237ce4b96b466a9d00ba8524b51f Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalm...@gmail.com>
Date: Tue, 8 Oct 2024 10:29:22 +0200
Subject: [PATCH 3/3] Add FIXME and TODO
---
clang/lib/AST/DeclCXX.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index a522627b694fb3..1364ccc745ba01 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1638,7 +1638,7 @@ static NamedDecl* getLambdaCallOperatorHelper(const
CXXRecordDecl &RD) {
assert(allLookupResultsAreTheSame(Calls) &&
"More than one lambda call operator!");
- // If we have multiple call operators, we might be in a situation
+ // 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).
//
@@ -1656,6 +1656,9 @@ static NamedDecl* getLambdaCallOperatorHelper(const
CXXRecordDecl &RD) {
//
// 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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
