[clang] [WIP] MicrosoftCXXABI: save and load the RecordToCopyCtor table (#53486)

glebov-andrey · glebov-andrey · commit 0c8b9cbfa0a7 · 2025-01-28T01:34:55.000+03:00
- Replaces the previous attempt at restoring the table
- The implementation is currently broken
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
@@ -3289,6 +3289,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
       const FunctionDecl *FD,
       llvm::function_ref<void(FunctionDecl *)> Pred) const;
 
+  const llvm::SmallDenseMap<CXXRecordDecl *, CXXConstructorDecl *> *
+  getRecordToCopyCtor() const;
+
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD);
 
diff --git a/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def b/clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -249,11 +249,6 @@ FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR)
 /// base classes or fields have a no-return destructor
 FIELD(IsAnyDestructorNoReturn, 1, NO_MERGE)
 
-/// Microsoft CXX ABI specific:
-/// Whether the copy constructor is used by a `throw` expression.
-/// Used by ASTReader to restore the sidecar RecordToCopyCtor LUT.
-FIELD(HasCopyConstructorForExceptionObject, 1, MERGE_OR)
-
 /// Whether the record type is intangible (if any base classes or fields have
 /// type that is intangible). HLSL only.
 FIELD(IsHLSLIntangible, 1, NO_MERGE)
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
@@ -1558,14 +1558,6 @@ class CXXRecordDecl : public RecordDecl {
   /// a field or in base class.
   bool isHLSLIntangible() const { return data().IsHLSLIntangible; }
 
-  bool hasCopyConstructorForExceptionObject() const {
-    return data().HasCopyConstructorForExceptionObject;
-  }
-
-  void setHasCopyConstructorForExceptionObject() {
-    data().HasCopyConstructorForExceptionObject = true;
-  }
-
   /// If the class is a local class [class.local], returns
   /// the enclosing function declaration.
   const FunctionDecl *isLocalClass() const {
diff --git a/clang/include/clang/Sema/ExternalSemaSource.h b/clang/include/clang/Sema/ExternalSemaSource.h
@@ -200,6 +200,9 @@ class ExternalSemaSource : public ExternalASTSource {
   virtual void
   ReadDeclsToCheckForDeferredDiags(llvm::SmallSetVector<Decl *, 4> &Decls) {}
 
+  virtual void ReadRecordExceptionCopyingConstructors(
+      llvm::MapVector<CXXRecordDecl *, CXXConstructorDecl *> &RecordToCtor) {}
+
   /// \copydoc Sema::CorrectTypo
   /// \note LookupKind must correspond to a valid Sema::LookupNameKind
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -345,6 +345,10 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
   void ReadDeclsToCheckForDeferredDiags(
       llvm::SmallSetVector<Decl *, 4> &Decls) override;
 
+  void ReadRecordExceptionCopyingConstructors(
+      llvm::MapVector<CXXRecordDecl *,
+                      CXXConstructorDecl *> &RecordToCtor) override;
+
   /// \copydoc ExternalSemaSource::CorrectTypo
   /// \note Returns the first nonempty correction.
   TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -5427,6 +5427,8 @@ class Sema final : public SemaBase {
   /// \returns true if any work was done, false otherwise.
   bool DefineUsedVTables();
 
+  void LoadExternalRecordToExceptionCopyingCtor();
+
   /// AddImplicitlyDeclaredMembersToClass - Adds any implicitly-declared
   /// special functions, such as the default constructor, copy
   /// constructor, or destructor, to the given C++ class (C++
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -742,6 +742,8 @@ enum ASTRecordTypes {
   UPDATE_MODULE_LOCAL_VISIBLE = 76,
 
   UPDATE_TU_LOCAL_VISIBLE = 77,
+
+  MSCXXABI_EXCEPTION_COPYING_CONSTRUCTORS = 78,
 };
 
 /// Record types used within a source manager block.
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
@@ -1044,6 +1044,12 @@ class ASTReader
   /// The IDs of all decls with function effects to be checked.
   SmallVector<GlobalDeclID> DeclsWithEffectsToVerify;
 
+  struct RecordAndCopyingCtor {
+    GlobalDeclID RecordID;
+    GlobalDeclID CtorID;
+  };
+  SmallVector<RecordAndCopyingCtor> RecordToCopyingCtor;
+
 private:
   struct ImportedSubmodule {
     serialization::SubmoduleID ID;
@@ -2289,6 +2295,10 @@ class ASTReader
       llvm::MapVector<const FunctionDecl *, std::unique_ptr<LateParsedTemplate>>
           &LPTMap) override;
 
+  void ReadRecordExceptionCopyingConstructors(
+      llvm::MapVector<CXXRecordDecl *,
+                      CXXConstructorDecl *> &RecordToCtor) override;
+
   void AssignedLambdaNumbering(CXXRecordDecl *Lambda) override;
 
   /// Load a selector from disk, registering its ID if it exists.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
@@ -13103,6 +13103,11 @@ ASTContext::createMangleNumberingContext() const {
   return ABI->createMangleNumberingContext();
 }
 
+const llvm::SmallDenseMap<CXXRecordDecl *, CXXConstructorDecl *> *
+ASTContext::getRecordToCopyCtor() const {
+  return ABI->getRecordToCopyCtor();
+}
+
 const CXXConstructorDecl *
 ASTContext::getCopyConstructorForExceptionObject(CXXRecordDecl *RD) {
   return ABI->getCopyConstructorForExceptionObject(
diff --git a/clang/lib/AST/CXXABI.h b/clang/lib/AST/CXXABI.h
@@ -56,6 +56,9 @@ class CXXABI {
   virtual void addCopyConstructorForExceptionObject(CXXRecordDecl *,
                                                     CXXConstructorDecl *) = 0;
 
+  virtual const llvm::SmallDenseMap<CXXRecordDecl *, CXXConstructorDecl *> *
+  getRecordToCopyCtor() const = 0;
+
   /// Retrieves the mapping from class to copy constructor for this C++ ABI.
   virtual const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *) = 0;
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
@@ -109,10 +109,9 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
       ImplicitCopyAssignmentHasConstParam(true),
       HasDeclaredCopyConstructorWithConstParam(false),
       HasDeclaredCopyAssignmentWithConstParam(false),
-      IsAnyDestructorNoReturn(false),
-      HasCopyConstructorForExceptionObject(false), IsHLSLIntangible(false),
-      IsLambda(false), IsParsingBaseSpecifiers(false),
-      ComputedVisibleConversions(false), HasODRHash(false), Definition(D) {}
+      IsAnyDestructorNoReturn(false), IsHLSLIntangible(false), IsLambda(false),
+      IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
+      HasODRHash(false), Definition(D) {}
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
   return Bases.get(Definition->getASTContext().getExternalSource());
diff --git a/clang/lib/AST/ItaniumCXXABI.cpp b/clang/lib/AST/ItaniumCXXABI.cpp
@@ -256,6 +256,11 @@ class ItaniumCXXABI : public CXXABI {
     return Layout.getNonVirtualSize() == PointerSize;
   }
 
+  const llvm::SmallDenseMap<CXXRecordDecl *, CXXConstructorDecl *> *
+  getRecordToCopyCtor() const override {
+    return nullptr;
+  }
+
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD) override {
     return nullptr;
diff --git a/clang/lib/AST/MicrosoftCXXABI.cpp b/clang/lib/AST/MicrosoftCXXABI.cpp
@@ -146,6 +146,11 @@ class MicrosoftCXXABI : public CXXABI {
     llvm_unreachable("unapplicable to the MS ABI");
   }
 
+  const llvm::SmallDenseMap<CXXRecordDecl *, CXXConstructorDecl *> *
+  getRecordToCopyCtor() const override {
+    return &RecordToCopyCtor;
+  }
+
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD) override {
     return RecordToCopyCtor[RD];
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -336,6 +336,12 @@ void MultiplexExternalSemaSource::ReadLateParsedTemplates(
     Sources[i]->ReadLateParsedTemplates(LPTMap);
 }
 
+void MultiplexExternalSemaSource::ReadRecordExceptionCopyingConstructors(
+    llvm::MapVector<CXXRecordDecl *, CXXConstructorDecl *> &RecordToCtor) {
+  for (size_t i = 0; i < Sources.size(); ++i)
+    Sources[i]->ReadRecordExceptionCopyingConstructors(RecordToCtor);
+}
+
 TypoCorrection MultiplexExternalSemaSource::CorrectTypo(
                                      const DeclarationNameInfo &Typo,
                                      int LookupKind, Scope *S, CXXScopeSpec *SS,
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
@@ -1101,6 +1101,8 @@ void Sema::ActOnStartOfTranslationUnit() {
   if (getLangOpts().CPlusPlusModules &&
       getLangOpts().getCompilingModule() == LangOptions::CMK_HeaderUnit)
     HandleStartOfHeaderUnit();
+
+  LoadExternalRecordToExceptionCopyingCtor();
 }
 
 void Sema::ActOnEndOfTranslationUnitFragment(TUFragmentKind Kind) {
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18660,6 +18660,19 @@ bool Sema::DefineUsedVTables() {
   return DefinedAnything;
 }
 
+void Sema::LoadExternalRecordToExceptionCopyingCtor() {
+  if (!ExternalSource)
+    return;
+
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+    llvm::MapVector<CXXRecordDecl *, CXXConstructorDecl *> RecordToCtorMap;
+    ExternalSource->ReadRecordExceptionCopyingConstructors(RecordToCtorMap);
+    for (const auto &[RD, CD] : RecordToCtorMap) {
+      Context.addCopyConstructorForExceptionObject(RD, CD);
+    }
+  }
+}
+
 void Sema::MarkVirtualMemberExceptionSpecsNeeded(SourceLocation Loc,
                                                  const CXXRecordDecl *RD) {
   for (const auto *I : RD->methods())
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1082,10 +1082,6 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc,
       // friendship or any other means).
       Context.addCopyConstructorForExceptionObject(Subobject, CD);
 
-      // Store the bit in CXXRecordDecl so that ASTReader can restore this
-      // mapping later.
-      Subobject->setHasCopyConstructorForExceptionObject();
-
       // We don't keep the instantiated default argument expressions around so
       // we must rebuild them here.
       for (unsigned I = 1, E = CD->getNumParams(); I != E; ++I) {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
@@ -4313,6 +4313,19 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
       for (unsigned I = 0, N = Record.size(); I != N; /*in loop*/)
         DeclsToCheckForDeferredDiags.insert(ReadDeclID(F, Record, I));
       break;
+
+    case MSCXXABI_EXCEPTION_COPYING_CONSTRUCTORS: {
+      if (Record.size() % 2 != 0)
+        return llvm::createStringError(
+            std::errc::illegal_byte_sequence,
+            "Invalid MSCXXABI_EXCEPTION_COPYING_CONSTRUCTORS record");
+      for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) {
+        RecordToCopyingCtor.push_back(
+            {ReadDeclID(F, Record, I),
+             ReadDeclID(F, Record, I)});
+      }
+      break;
+    }
     }
   }
 }
@@ -9372,6 +9385,16 @@ void ASTReader::ReadLateParsedTemplates(
   LateParsedTemplates.clear();
 }
 
+void ASTReader::ReadRecordExceptionCopyingConstructors(
+    llvm::MapVector<CXXRecordDecl *, CXXConstructorDecl *> &RecordToCtor) {
+  for (const auto &[RecordID, CtorID] : RecordToCopyingCtor) {
+    auto *RD = cast<CXXRecordDecl>(GetDecl(RecordID));
+    auto *CD = cast<CXXConstructorDecl>(GetDecl(CtorID));
+    RecordToCtor.insert({RD, CD});
+  }
+  RecordToCopyingCtor.clear();
+}
+
 void ASTReader::AssignedLambdaNumbering(CXXRecordDecl *Lambda) {
   if (!Lambda->getLambdaContextDecl())
     return;
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2327,20 +2327,6 @@ void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
   }
 
   VisitCXXMethodDecl(D);
-
-  // Microsoft CXX ABI specific:
-  // Restore the RecordToCopyCtor sidecar LUT entry so that `throw` expressions
-  // find the correct copy constructor for exceptions during codegen.
-  // There is no need to check the target info because the
-  // HasCopyConstructorForExceptionObject bit only gets set for the MS ABI.
-  if (D->isCopyConstructor()) {
-    // TODO What if this is not the same copy constructor which was chosen by
-    //      LookupCopyingConstructor() in SemaExprCXX? Is there a better way?
-    auto *R = cast<CXXRecordDecl>(D->getDeclContext());
-    if (R->hasCopyConstructorForExceptionObject()) {
-      Reader.getContext().addCopyConstructorForExceptionObject(R, D);
-    }
-  }
 }
 
 void ASTDeclReader::VisitCXXDestructorDecl(CXXDestructorDecl *D) {
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
@@ -5847,6 +5847,19 @@ void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) {
 
   if (!VTablesToEmit.empty())
     Stream.EmitRecord(VTABLES_TO_EMIT, VTablesToEmit);
+
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+    RecordData ExceptionCopyingConstructors;
+    const auto *RecordToCopyCtor = Context.getRecordToCopyCtor();
+    assert(RecordToCopyCtor);
+    for (const auto [RD, CD] : *RecordToCopyCtor) {
+      AddDeclRef(RD, ExceptionCopyingConstructors);
+      AddDeclRef(CD, ExceptionCopyingConstructors);
+    }
+    if (!ExceptionCopyingConstructors.empty())
+      Stream.EmitRecord(MSCXXABI_EXCEPTION_COPYING_CONSTRUCTORS,
+                        ExceptionCopyingConstructors);
+  }
 }
 
 ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot,
