[clang] Revert "[RFC][C++20][Modules] Fix crash when function and lambda insi… (PR #108311)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 11 17:19:38 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: Pranav Kant (pranavk)
<details>
<summary>Changes</summary>
…de loaded from different modules (#<!-- -->104512)"
This reverts commit d778689fdc812033e7142ed87e4ee13c4997b3f9.
---
Full diff: https://github.com/llvm/llvm-project/pull/108311.diff
6 Files Affected:
- (modified) clang/include/clang/Serialization/ASTReader.h (-9)
- (modified) clang/lib/Serialization/ASTReader.cpp (+1-7)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (-10)
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (-42)
- (removed) clang/test/Headers/crash-instantiated-in-scope-cxx-modules.cpp (-76)
- (removed) clang/test/Headers/crash-instantiated-in-scope-cxx-modules2.cpp (-30)
``````````diff
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/108311
More information about the cfe-commits
mailing list