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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
