Revert "[RFC][C++20][Modules] Fix crash when function and lambda insi…

[pranavk]

…de loaded from different modules (#104512)"

This reverts commit d778689.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Pranav Kant (pranavk)

Changes

…de loaded from different modules (#104512)"

This reverts commit d778689.

---

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 --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

[pranavk]

Breaks multiple of our internal workloads and chromium open-source project (https://issues.chromium.org/issues/366045258)