[clang] [WIP] [Modules] Delay reading type source info of the preferred_name attribute. (PR #122250)

[Ilya Biryukov via cfe-commits]

================
@@ -3129,42 +3129,86 @@ class AttrReader {
   OMPTraitInfo *readOMPTraitInfo() { return Reader.readOMPTraitInfo(); }
 
   template <typename T> T *readDeclAs() { return Reader.readDeclAs<T>(); }
+
+  AttributeCommonInfo readAttributeCommonInfo() {
+    IdentifierInfo *AttrName = readIdentifier();
+    IdentifierInfo *ScopeName = readIdentifier();
+    SourceRange AttrRange = readSourceRange();
+    SourceLocation ScopeLoc = readSourceLocation();
+    unsigned ParsedKind = readInt();
+    unsigned Syntax = readInt();
+    unsigned SpellingIndex = readInt();
+    bool IsAlignas = (ParsedKind == AttributeCommonInfo::AT_Aligned &&
+                      Syntax == AttributeCommonInfo::AS_Keyword &&
+                      SpellingIndex == AlignedAttr::Keyword_alignas);
+    bool IsRegularKeywordAttribute = readBool();
+
+    AttributeCommonInfo Info(AttrName, ScopeName, AttrRange, ScopeLoc,
+                             AttributeCommonInfo::Kind(ParsedKind),
+                             {AttributeCommonInfo::Syntax(Syntax),
+                              SpellingIndex, IsAlignas,
+                              IsRegularKeywordAttribute});
+    return Info;
+  }
+
+  std::optional<attr::Kind> readAttrKind() {
+    auto V = readInt();
+    if (!V)
+      return {};
+
+    // Kind is stored as a 1-based integer because 0 is used to indicate a null
+    // Attr pointer.
+    return static_cast<attr::Kind>(V - 1);
+  }
+
+  Attr *createAttribute(attr::Kind Kind, AttributeCommonInfo Info) {
+    ASTContext &Context = Reader.getContext();
+    Attr *New = nullptr;
+    auto Record = *this;
+#include "clang/Serialization/AttrPCHRead.inc"
+
+    assert(New && "Unable to decode attribute?");
+    return New;
+  }
 };
-}
+} // namespace
 
 Attr *ASTRecordReader::readAttr() {
   AttrReader Record(*this);
-  auto V = Record.readInt();
-  if (!V)
+  attr::Kind Kind;
+  if (auto KindOpt = Record.readAttrKind(); !KindOpt)
     return nullptr;
+  else
+    Kind = *KindOpt;
 
-  Attr *New = nullptr;
-  // 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);
-  ASTContext &Context = getContext();
-
-  IdentifierInfo *AttrName = Record.readIdentifier();
-  IdentifierInfo *ScopeName = Record.readIdentifier();
-  SourceRange AttrRange = Record.readSourceRange();
-  SourceLocation ScopeLoc = Record.readSourceLocation();
-  unsigned ParsedKind = Record.readInt();
-  unsigned Syntax = Record.readInt();
-  unsigned SpellingIndex = Record.readInt();
-  bool IsAlignas = (ParsedKind == AttributeCommonInfo::AT_Aligned &&
-                    Syntax == AttributeCommonInfo::AS_Keyword &&
-                    SpellingIndex == AlignedAttr::Keyword_alignas);
-  bool IsRegularKeywordAttribute = Record.readBool();
-
-  AttributeCommonInfo Info(AttrName, ScopeName, AttrRange, ScopeLoc,
-                           AttributeCommonInfo::Kind(ParsedKind),
-                           {AttributeCommonInfo::Syntax(Syntax), SpellingIndex,
-                            IsAlignas, IsRegularKeywordAttribute});
+  AttributeCommonInfo Info = Record.readAttributeCommonInfo();
+  return Record.createAttribute(Kind, Info);
+}
 
-#include "clang/Serialization/AttrPCHRead.inc"
+Attr *ASTRecordReader::readAttr(Decl *D) {
+  AttrReader Record(*this);
+  attr::Kind Kind;
+  if (auto KindOpt = Record.readAttrKind(); !KindOpt)
+    return nullptr;
+  else
+    Kind = *KindOpt;
+
+  AttributeCommonInfo Info = Record.readAttributeCommonInfo();
+  if (Kind == attr::PreferredName) {
+    bool isInherited = Record.readInt();
+    bool isImplicit = Record.readInt();
+    bool isPackExpansion = Record.readInt();
+    serialization::TypeID TypeID = getGlobalTypeID(Record.readInt());
+    SourceLocation ElaboratedTypedefSL = Record.readSourceLocation();
----------------
ilya-biryukov wrote:

This code assumes a specific structure of the `TypeLoc` we store for `PreferredName`.
It is a major assumption that we will not change the representation of `TypeLoc`s and that the particular `TypeLoc`s used for `PreferredName` stay the same over time.

We could probably make the code here more robust by **also** updating serialization so that it writes only the source locations we read here, rather than call the generic method for serializing `TypeLoc`.
However, I don't think this will lead to better code.

Could we instead try postponing reading the whole attribute?
As chatted outside of this thread, this would entail updating the serialization of `PreferredName` to store the number of record entries that it takes.
Later, when deserializing, we can simply remember the position at which the attribute starts, skip it and store the position in the "pending attributes". In `finishPendingAction`, we can temporarily switch the position back and call `readAttr` to re-read the whole attribute.

I bet this approach would end up being much simpler and will allow avoiding the duplication of serialization/deseriazliation logic. Moreover, it can be generalized to more attribute if the need arises.

https://github.com/llvm/llvm-project/pull/122250