[WIP][Modules] Delay deserialization of preferred_name attribute at record level.

Author: VitaNuo

clang/include/clang/Serialization/ASTReader.h

@@ -1205,6 +1205,23 @@ class ASTReader
   /// been completed.
   std::deque<PendingDeclContextInfo> PendingDeclContextInfos;
 
+  /// Deserialization of some attributes must be deferred since they refer
+  /// to themselves in their type (e.g., preferred_name attribute refers to the
+  /// typedef that refers back to the template specialization of the template
+  /// that the attribute is attached to).
+  /// More attributes that store TypeSourceInfo might be potentially affected,
+  /// see https://github.com/llvm/llvm-project/issues/56490 for details.
+  struct DeferredAttribute {
+    uint64_t RecordIdx;
+    // Decl to attach a deferred attribute to.
+    Decl *ParentDecl;
+  };
+
+  /// The collection of Decls that have been loaded but some of their attributes
+  /// have been deferred, paired with the index inside the record pointing
+  /// at the skipped attribute.
+  SmallVector<DeferredAttribute> PendingDeferredAttributes;
+
   template <typename DeclTy>
   using DuplicateObjCDecls = std::pair<DeclTy *, DeclTy *>;
 
@@ -1551,6 +1568,7 @@ class ASTReader
   void loadPendingDeclChain(Decl *D, uint64_t LocalOffset);
   void loadObjCCategories(GlobalDeclID ID, ObjCInterfaceDecl *D,
                           unsigned PreviousGeneration = 0);
+  void loadDeferredAttribute(const DeferredAttribute &DA);
 
   RecordLocation getLocalBitOffset(uint64_t GlobalOffset);
   uint64_t getGlobalBitOffset(ModuleFile &M, uint64_t LocalOffset);

clang/include/clang/Serialization/ASTRecordReader.h

@@ -337,6 +337,12 @@ class ASTRecordReader
   /// Reads attributes from the current stream position, advancing Idx.
   void readAttributes(AttrVec &Attrs);
 
+  /// Reads one attribute from the current stream position, advancing Idx.
+  Attr *readOrDeferAttr(Decl *D);
+
+  /// Reads attributes from the current stream position, advancing Idx.
+  void readOrDeferAttributes(AttrVec &Attrs, Decl *D);
+
   /// Read an BTFTypeTagAttr object.
   BTFTypeTagAttr *readBTFTypeTagAttr() {
     return cast<BTFTypeTagAttr>(readAttr());
@@ -355,6 +361,10 @@ class ASTRecordReader
   SwitchCase *getSwitchCaseWithID(unsigned ID) {
     return Reader->getSwitchCaseWithID(ID);
   }
+
+private:
+  Attr *readOrDeferAttrImpl(Decl *D);
+  void readOrDeferAttributesImpl(AttrVec &Attrs, Decl *D);
 };
 
 /// Helper class that saves the current stream position and

clang/lib/Serialization/ASTReader.cpp

@@ -10079,6 +10079,11 @@ void ASTReader::finishPendingActions() {
     }
     PendingDeducedVarTypes.clear();
 
+    // Load the delayed preferred name attributes.
+    for (unsigned I = 0; I != PendingDeferredAttributes.size(); ++I)
+      loadDeferredAttribute(PendingDeferredAttributes[I]);
+    PendingDeferredAttributes.clear();
+
     // For each decl chain that we wanted to complete while deserializing, mark
     // it as "still needs to be completed".
     for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -612,7 +612,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
 
   if (HasAttrs) {
     AttrVec Attrs;
-    Record.readAttributes(Attrs);
+    Record.readOrDeferAttributes(Attrs, D);
     // Avoid calling setAttrs() directly because it uses Decl::getASTContext()
     // internally which is unsafe during derialization.
     D->setAttrsImpl(Attrs, Reader.getContext());
@@ -3118,13 +3118,18 @@ class AttrReader {
     return Reader.readVersionTuple();
   }
 
+  void skipInts(unsigned N) { Reader.skipInts(N); }
+
+  unsigned getCurrentIdx() { return Reader.getIdx(); }
+
   OMPTraitInfo *readOMPTraitInfo() { return Reader.readOMPTraitInfo(); }
 
   template <typename T> T *readDeclAs() { return Reader.readDeclAs<T>(); }
 };
 }
 
-Attr *ASTRecordReader::readAttr() {
+/// Reads one attribute from the current stream position, advancing Idx.
+Attr *ASTRecordReader::readOrDeferAttrImpl(Decl *D) {
   AttrReader Record(*this);
   auto V = Record.readInt();
   if (!V)
@@ -3134,6 +3139,20 @@ Attr *ASTRecordReader::readAttr() {
   // Kind is stored as a 1-based integer because 0 is used to indicate a null
   // Attr pointer.
   auto Kind = static_cast<attr::Kind>(V - 1);
+  // Some attributes refer to themselves during deserialization, thus
+  // their deserialization must be deferred until their underlying type
+  // is resolved.
+  if (Kind == attr::PreferredName) {
+    if (D != nullptr) {
+      Reader->PendingDeferredAttributes.push_back(
+          {Record.getCurrentIdx() - 1, D});
+      auto SkipCount = Record.readInt();
+      Record.skipInts(SkipCount);
+      return nullptr;
+    }
+    // Ignore the skip count when resolving pending actions.
+    Record.readInt();
+  }
   ASTContext &Context = getContext();
 
   IdentifierInfo *AttrName = Record.readIdentifier();
@@ -3159,13 +3178,31 @@ Attr *ASTRecordReader::readAttr() {
   return New;
 }
 
-/// Reads attributes from the current stream position.
-void ASTRecordReader::readAttributes(AttrVec &Attrs) {
+void ASTRecordReader::readOrDeferAttributesImpl(AttrVec &Attrs, Decl *D) {
   for (unsigned I = 0, E = readInt(); I != E; ++I)
-    if (auto *A = readAttr())
+    if (auto *A = readOrDeferAttr(D))
       Attrs.push_back(A);
 }
 
+Attr *ASTRecordReader::readAttr() { return readOrDeferAttrImpl(nullptr); }
+
+/// Reads attributes from the current stream position.
+void ASTRecordReader::readAttributes(AttrVec &Attrs) {
+  readOrDeferAttributesImpl(Attrs, nullptr);
+}
+
+/// Reads one attribute from the current stream position, advancing Idx.
+/// For some attributes (where type depends on itself recursively), defer
+/// reading the attribute until the type has been read.
+Attr *ASTRecordReader::readOrDeferAttr(Decl *D) { return readOrDeferAttrImpl(D); }
+
+/// Reads attributes from the current stream position, advancing Idx.
+/// For some attributes (where type depends on itself recursively), defer
+/// reading the attribute until the type has been read.
+void ASTRecordReader::readOrDeferAttributes(AttrVec &Attrs, Decl *D) {
+  readOrDeferAttributesImpl(Attrs, D);
+}
+
 //===----------------------------------------------------------------------===//
 // ASTReader Implementation
 //===----------------------------------------------------------------------===//
@@ -4424,6 +4461,50 @@ void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
   ASTDeclReader::attachLatestDecl(CanonDecl, MostRecent);
 }
 
+void ASTReader::loadDeferredAttribute(
+    const DeferredAttribute &DA) {
+  Decl *D = DA.ParentDecl;
+  ModuleFile *M = getOwningModuleFile(D);
+
+  unsigned LocalDeclIndex = D->getGlobalID().getLocalDeclIndex();
+  const DeclOffset &DOffs = M->DeclOffsets[LocalDeclIndex];
+  RecordLocation Loc(M, DOffs.getBitOffset(M->DeclsBlockStartOffset));
+
+  llvm::BitstreamCursor &Cursor = Loc.F->DeclsCursor;
+  SavedStreamPosition SavedPosition(Cursor);
+  if (llvm::Error Err = Cursor.JumpToBit(Loc.Offset)) {
+    Error(std::move(Err));
+  }
+
+  Expected<unsigned> MaybeCode = Cursor.ReadCode();
+  if (!MaybeCode) {
+    llvm::report_fatal_error(
+        Twine("ASTReader::loadPreferredNameAttribute failed reading code: ") +
+        toString(MaybeCode.takeError()));
+  }
+  unsigned Code = MaybeCode.get();
+
+  ASTRecordReader Record(*this, *Loc.F);
+  Expected<unsigned> MaybeRecCode = Record.readRecord(Cursor, Code);
+  if (!MaybeRecCode) {
+    llvm::report_fatal_error(
+        Twine(
+            "ASTReader::loadPreferredNameAttribute failed reading rec code: ") +
+        toString(MaybeCode.takeError()));
+  }
+  unsigned RecCode = MaybeRecCode.get();
+  if (RecCode < DECL_TYPEDEF || RecCode > DECL_LAST) {
+    llvm::report_fatal_error(
+        Twine("ASTReader::loadPreferredNameAttribute failed reading rec code: "
+              "expected valid DeclCode") +
+        toString(MaybeCode.takeError()));
+  }
+
+  Record.skipInts(DA.RecordIdx);
+  Attr *A = Record.readOrDeferAttr(nullptr);
+  getContext().getDeclAttrs(D).push_back(A);
+}
+
 namespace {
 
   /// Given an ObjC interface, goes through the modules and links to the

clang/lib/Serialization/ASTWriter.cpp

@@ -37,6 +37,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
+#include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileEntry.h"
@@ -4906,15 +4907,17 @@ void ASTWriter::WriteModuleFileExtension(Sema &SemaRef,
 
 void ASTRecordWriter::AddAttr(const Attr *A) {
   auto &Record = *this;
-  // FIXME: Clang can't handle the serialization/deserialization of
-  // preferred_name properly now. See
-  // https://github.com/llvm/llvm-project/issues/56490 for example.
-  if (!A || (isa<PreferredNameAttr>(A) &&
-             Writer->isWritingStdCXXNamedModules()))
+  if (!A)
     return Record.push_back(0);
 
   Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
 
+  auto SkipIdx = Record.size();
+  if (A->getKind() == attr::PreferredName) {
+    // Add placeholder for the size of preferred_name attribute.
+    Record.push_back(0);
+  }
+
   Record.AddIdentifierRef(A->getAttrName());
   Record.AddIdentifierRef(A->getScopeName());
   Record.AddSourceRange(A->getRange());
@@ -4925,6 +4928,12 @@ void ASTRecordWriter::AddAttr(const Attr *A) {
   Record.push_back(A->isRegularKeywordAttribute());
 
 #include "clang/Serialization/AttrPCHWrite.inc"
+
+  if (A->getKind() == attr::PreferredName) {
+    // Record the actual size of preferred_name attribute (-1 to count the
+    // placeholder).
+    Record[SkipIdx] = Record.size() - SkipIdx - 1;
+  }
 }
 
 /// Emit the list of attributes to the specified record.

clang/test/Modules/preferred_name.cppm

@@ -53,10 +53,16 @@ import A;
 export using ::foo_templ;
 
 //--- Use1.cpp
-import A;         // expected-[email protected]:8 {{attribute declaration must precede definition}}
-#include "foo.h"  // [email protected]:9 {{previous definition is here}}
-
+// expected-no-diagnostics
+import A;
+#include "foo.h"
 //--- Use2.cpp
 // expected-no-diagnostics
 #include "foo.h"
 import A;
+
+//--- Use3.cpp
+#include "foo.h"
+import A;
+foo test;
+int size = test.size(); // expected-error {{no member named 'size' in 'foo'}}