https://github.com/pranavk created https://github.com/llvm/llvm-project/pull/108311
…de loaded from different modules (#104512)" This reverts commit d778689fdc812033e7142ed87e4ee13c4997b3f9. >From 2bce8f5555fa0ede2e21c590768f0b533c7e793b Mon Sep 17 00:00:00 2001 From: Pranav Kant <p...@google.com> Date: Thu, 12 Sep 2024 00:18:28 +0000 Subject: [PATCH] Revert "[RFC][C++20][Modules] Fix crash when function and lambda inside loaded from different modules (#104512)" This reverts commit d778689fdc812033e7142ed87e4ee13c4997b3f9. --- clang/include/clang/Serialization/ASTReader.h | 9 --- clang/lib/Serialization/ASTReader.cpp | 8 +- clang/lib/Serialization/ASTReaderDecl.cpp | 10 --- clang/lib/Serialization/ASTWriterDecl.cpp | 42 ---------- ...rash-instantiated-in-scope-cxx-modules.cpp | 76 ------------------- ...ash-instantiated-in-scope-cxx-modules2.cpp | 30 -------- 6 files changed, 1 insertion(+), 174 deletions(-) delete mode 100644 clang/test/Headers/crash-instantiated-in-scope-cxx-modules.cpp delete mode 100644 clang/test/Headers/crash-instantiated-in-scope-cxx-modules2.cpp diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 7331bcf249266d..898f4392465fdf 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1188,15 +1188,6 @@ class ASTReader /// once recursing loading has been completed. llvm::SmallVector<NamedDecl *, 16> PendingOdrMergeChecks; - /// Lambdas that need to be loaded right after the function they belong to. - /// It is required to have canonical declaration for lambda class from the - /// same module as enclosing function. This is required to correctly resolve - /// captured variables in the lambda. Without this, due to lazy - /// deserialization canonical declarations for the function and lambdas can - /// be from different modules and DeclRefExprs may refer to the AST nodes - /// that don't exist in the function. - SmallVector<GlobalDeclID, 4> PendingLambdas; - using DataPointers = std::pair<CXXRecordDecl *, struct CXXRecordDecl::DefinitionData *>; using ObjCInterfaceDataPointers = diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 0ee53e43dff39c..e5a1e20a265616 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9782,8 +9782,7 @@ void ASTReader::finishPendingActions() { !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() || !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() || - !PendingObjCExtensionIvarRedeclarations.empty() || - !PendingLambdas.empty()) { + !PendingObjCExtensionIvarRedeclarations.empty()) { // If any identifiers with corresponding top-level declarations have // been loaded, load those declarations now. using TopLevelDeclsMap = @@ -9928,11 +9927,6 @@ void ASTReader::finishPendingActions() { } PendingObjCExtensionIvarRedeclarations.pop_back(); } - - // Load any pendiong lambdas. - for (auto ID : PendingLambdas) - GetDecl(ID); - PendingLambdas.clear(); } // At this point, all update records for loaded decls are in place, so any diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 20e577404d997d..9272e23c7da3fc 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1155,16 +1155,6 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { for (unsigned I = 0; I != NumParams; ++I) Params.push_back(readDeclAs<ParmVarDecl>()); FD->setParams(Reader.getContext(), Params); - - // For the first decl add all lambdas inside for loading them later, - // otherwise skip them. - unsigned NumLambdas = Record.readInt(); - if (FD->isFirstDecl()) { - for (unsigned I = 0; I != NumLambdas; ++I) - Reader.PendingLambdas.push_back(Record.readDeclID()); - } else { - Record.skipInts(NumLambdas); - } } void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl *MD) { diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 732a6e21f340d6..555f6325da646b 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -18,7 +18,6 @@ #include "clang/AST/Expr.h" #include "clang/AST/OpenMPClause.h" #include "clang/AST/PrettyDeclStackTrace.h" -#include "clang/AST/StmtVisitor.h" #include "clang/Basic/SourceManager.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/ASTRecordWriter.h" @@ -626,33 +625,6 @@ void ASTDeclWriter::VisitDeclaratorDecl(DeclaratorDecl *D) { : QualType()); } -static llvm::SmallVector<const Decl *, 2> collectLambdas(FunctionDecl *D) { - struct LambdaCollector : public ConstStmtVisitor<LambdaCollector> { - llvm::SmallVectorImpl<const Decl *> &Lambdas; - - LambdaCollector(llvm::SmallVectorImpl<const Decl *> &Lambdas) - : Lambdas(Lambdas) {} - - void VisitLambdaExpr(const LambdaExpr *E) { - VisitStmt(E); - Lambdas.push_back(E->getLambdaClass()); - } - - void VisitStmt(const Stmt *S) { - if (!S) - return; - for (const Stmt *Child : S->children()) - if (Child) - Visit(Child); - } - }; - - llvm::SmallVector<const Decl *, 2> Lambdas; - if (D->hasBody()) - LambdaCollector(Lambdas).VisitStmt(D->getBody()); - return Lambdas; -} - void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { static_assert(DeclContext::NumFunctionDeclBits == 44, "You need to update the serializer after you change the " @@ -792,19 +764,6 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { Record.push_back(D->param_size()); for (auto *P : D->parameters()) Record.AddDeclRef(P); - - // Store references to all lambda decls inside function to load them - // immediately after loading the function to make sure that canonical - // decls for lambdas will be from the same module. - if (D->isCanonicalDecl()) { - llvm::SmallVector<const Decl *, 2> Lambdas = collectLambdas(D); - Record.push_back(Lambdas.size()); - for (const auto *L : Lambdas) - Record.AddDeclRef(L); - } else { - Record.push_back(0); - } - Code = serialization::DECL_FUNCTION; } @@ -2280,7 +2239,6 @@ getFunctionDeclAbbrev(serialization::DeclCode Code) { // // This is: // NumParams and Params[] from FunctionDecl, and - // NumLambdas, Lambdas[] from FunctionDecl, and // NumOverriddenMethods, OverriddenMethods[] from CXXMethodDecl. // // Add an AbbrevOp for 'size then elements' and use it here. diff --git a/clang/test/Headers/crash-instantiated-in-scope-cxx-modules.cpp b/clang/test/Headers/crash-instantiated-in-scope-cxx-modules.cpp deleted file mode 100644 index 80844a58ad825a..00000000000000 --- a/clang/test/Headers/crash-instantiated-in-scope-cxx-modules.cpp +++ /dev/null @@ -1,76 +0,0 @@ -// RUN: rm -fR %t -// RUN: split-file %s %t -// RUN: cd %t -// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header -Werror=uninitialized folly-conv.h -// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header -Werror=uninitialized thrift_cpp2_base.h -// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header -Werror=uninitialized -fmodule-file=folly-conv.pcm -fmodule-file=thrift_cpp2_base.pcm logger_base.h - -//--- Conv.h -#pragma once - -template <typename _Tp, typename _Up = _Tp&&> -_Up __declval(int); - -template <typename _Tp> -auto declval() noexcept -> decltype(__declval<_Tp>(0)); - -namespace folly { - -template <class Value, class Error> -struct Expected { - template <class Yes> - auto thenOrThrow() -> decltype(declval<Value&>()) { - return 1; - } -}; - -struct ExpectedHelper { - template <class Error, class T> - static constexpr Expected<T, Error> return_(T) { - return Expected<T, Error>(); - } - - template <class This, class Fn, class E = int, class T = ExpectedHelper> - static auto then_(This&&, Fn&&) - -> decltype(T::template return_<E>((declval<Fn>()(true), 0))) { - return Expected<int, int>(); - } -}; - -template <class Tgt> -inline Expected<Tgt, const char*> tryTo() { - Tgt result = 0; - // In build with asserts: - // clang/lib/Sema/SemaTemplateInstantiate.cpp: llvm::PointerUnion<Decl *, LocalInstantiationScope::DeclArgumentPack *> *clang::LocalInstantiationScope::findInstantiationOf(const Decl *): Assertion `isa<LabelDecl>(D) && "declaration not instantiated in this scope"' failed. - // In release build compilation error on the line below inside lambda: - // error: variable 'result' is uninitialized when used here [-Werror,-Wuninitialized] - ExpectedHelper::then_(Expected<bool, int>(), [&](bool) { return result; }); - return {}; -} - -} // namespace folly - -inline void bar() { - folly::tryTo<int>(); -} -// expected-no-diagnostics - -//--- folly-conv.h -#pragma once -#include "Conv.h" -// expected-no-diagnostics - -//--- thrift_cpp2_base.h -#pragma once -#include "Conv.h" -// expected-no-diagnostics - -//--- logger_base.h -#pragma once -import "folly-conv.h"; -import "thrift_cpp2_base.h"; - -inline void foo() { - folly::tryTo<unsigned>(); -} -// expected-no-diagnostics diff --git a/clang/test/Headers/crash-instantiated-in-scope-cxx-modules2.cpp b/clang/test/Headers/crash-instantiated-in-scope-cxx-modules2.cpp deleted file mode 100644 index 5b1a904e928a68..00000000000000 --- a/clang/test/Headers/crash-instantiated-in-scope-cxx-modules2.cpp +++ /dev/null @@ -1,30 +0,0 @@ -// RUN: rm -fR %t -// RUN: split-file %s %t -// RUN: cd %t -// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header header.h -// RUN: %clang_cc1 -std=c++20 -fmodule-file=header.pcm main.cpp - -//--- header.h -template <typename T> -void f(T) {} - -class A { - virtual ~A(); -}; - -inline A::~A() { - f([](){}); -} - -struct B { - void g() { - f([](){ - [](){}; - }); - } -}; -// expected-no-diagnostics - -//--- main.cpp -import "header.h"; -// expected-no-diagnostics _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits