[clang] [Modules] Delay deserialization of preferred_name attribute at r… (PR #122726)
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 16 06:14:47 PST 2025
https://github.com/VitaNuo updated https://github.com/llvm/llvm-project/pull/122726
>From b61110999596363bafdc94904356840febfcfaa5 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Tue, 14 Jan 2025 14:00:48 +0100
Subject: [PATCH 1/7] [WIP][Modules] Delay deserialization of preferred_name
attribute at record level.
---
clang/include/clang/Serialization/ASTReader.h | 18 ++++
.../clang/Serialization/ASTRecordReader.h | 10 ++
clang/lib/Serialization/ASTReader.cpp | 5 +
clang/lib/Serialization/ASTReaderDecl.cpp | 91 ++++++++++++++++++-
clang/lib/Serialization/ASTWriter.cpp | 19 +++-
clang/test/Modules/preferred_name.cppm | 12 ++-
6 files changed, 142 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 9f978762a6fb6b..4fd51eabe701ba 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/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);
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index 2561418b78ca7f..b1572ecef9028c 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/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
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b53f99732cacce..57d1d4d696290d 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/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) {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index dee5169ae5723a..043bed8dc7f382 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/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
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 39004fd4d4c376..d24ca08e1cc988 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/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.
diff --git a/clang/test/Modules/preferred_name.cppm b/clang/test/Modules/preferred_name.cppm
index 806781a81c5ca7..86ba6ae96db998 100644
--- a/clang/test/Modules/preferred_name.cppm
+++ b/clang/test/Modules/preferred_name.cppm
@@ -53,10 +53,16 @@ import A;
export using ::foo_templ;
//--- Use1.cpp
-import A; // expected-warning at foo.h:8 {{attribute declaration must precede definition}}
-#include "foo.h" // expected-note at foo.h: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'}}
>From e70138755a941693dde0179910b0ecbc427b6cb6 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Tue, 14 Jan 2025 14:13:05 +0100
Subject: [PATCH 2/7] Fix formatting.
---
clang/lib/Serialization/ASTReaderDecl.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 043bed8dc7f382..7dedccdb31caf6 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3194,7 +3194,9 @@ void ASTRecordReader::readAttributes(AttrVec &Attrs) {
/// 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); }
+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
@@ -4461,8 +4463,7 @@ void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
ASTDeclReader::attachLatestDecl(CanonDecl, MostRecent);
}
-void ASTReader::loadDeferredAttribute(
- const DeferredAttribute &DA) {
+void ASTReader::loadDeferredAttribute(const DeferredAttribute &DA) {
Decl *D = DA.ParentDecl;
ModuleFile *M = getOwningModuleFile(D);
>From eb6b91527f890a57e1b454b8cc9a9c50e205f0fb Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 16 Jan 2025 09:28:56 +0100
Subject: [PATCH 3/7] [Modules] Add DeferDeserialization field to attributes.
Address other review comments.
---
clang/include/clang/AST/Attr.h | 14 +++--
clang/include/clang/Basic/Attr.td | 5 ++
clang/include/clang/Serialization/ASTReader.h | 3 +-
.../clang/Serialization/ASTRecordReader.h | 17 ++----
clang/lib/Serialization/ASTReaderDecl.cpp | 55 ++++++++-----------
clang/lib/Serialization/ASTWriter.cpp | 15 ++---
clang/utils/TableGen/ClangAttrEmitter.cpp | 4 ++
7 files changed, 54 insertions(+), 59 deletions(-)
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 3365ebe4d9012b..bed532a84a1bde 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -60,6 +60,8 @@ class Attr : public AttributeCommonInfo {
unsigned IsLateParsed : 1;
LLVM_PREFERRED_TYPE(bool)
unsigned InheritEvenIfAlreadyPresent : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned DeferDeserialization : 1;
void *operator new(size_t bytes) noexcept {
llvm_unreachable("Attrs cannot be allocated with regular 'new'.");
@@ -80,10 +82,11 @@ class Attr : public AttributeCommonInfo {
protected:
Attr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
- attr::Kind AK, bool IsLateParsed)
+ attr::Kind AK, bool IsLateParsed, bool DeferDeserialization = false)
: AttributeCommonInfo(CommonInfo), AttrKind(AK), Inherited(false),
IsPackExpansion(false), Implicit(false), IsLateParsed(IsLateParsed),
- InheritEvenIfAlreadyPresent(false) {}
+ InheritEvenIfAlreadyPresent(false),
+ DeferDeserialization(DeferDeserialization) {}
public:
attr::Kind getKind() const { return static_cast<attr::Kind>(AttrKind); }
@@ -105,6 +108,8 @@ class Attr : public AttributeCommonInfo {
void setPackExpansion(bool PE) { IsPackExpansion = PE; }
bool isPackExpansion() const { return IsPackExpansion; }
+ bool shouldDeferDeserialization() const { return DeferDeserialization; }
+
// Clone this attribute.
Attr *clone(ASTContext &C) const;
@@ -146,8 +151,9 @@ class InheritableAttr : public Attr {
protected:
InheritableAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
attr::Kind AK, bool IsLateParsed,
- bool InheritEvenIfAlreadyPresent)
- : Attr(Context, CommonInfo, AK, IsLateParsed) {
+ bool InheritEvenIfAlreadyPresent,
+ bool DeferDeserialization = false)
+ : Attr(Context, CommonInfo, AK, IsLateParsed, DeferDeserialization) {
this->InheritEvenIfAlreadyPresent = InheritEvenIfAlreadyPresent;
}
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 5039c20d8b73be..c06e060f576dc9 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -713,6 +713,10 @@ class Attr {
// attribute may be documented under multiple categories, more than one
// Documentation entry may be listed.
list<Documentation> Documentation;
+ // Set to true if deserialization of this attribute must be deferred until
+ // the parent Decl is fully deserialized (during header module file
+ // deserialization).
+ bit DeferDeserialization = 0;
}
/// Used to define a set of mutually exclusive attributes.
@@ -3240,6 +3244,7 @@ def PreferredName : InheritableAttr {
let InheritEvenIfAlreadyPresent = 1;
let MeaningfulToClassTemplateDefinition = 1;
let TemplateDependent = 1;
+ let DeferDeserialization = 1;
}
def PreserveMost : DeclOrTypeAttr {
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 4fd51eabe701ba..fc030b820688d3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1212,9 +1212,10 @@ class ASTReader
/// More attributes that store TypeSourceInfo might be potentially affected,
/// see https://github.com/llvm/llvm-project/issues/56490 for details.
struct DeferredAttribute {
+ // Index of the deferred attribute in the Record of the TargetedDecl.
uint64_t RecordIdx;
// Decl to attach a deferred attribute to.
- Decl *ParentDecl;
+ Decl *TargetedDecl;
};
/// The collection of Decls that have been loaded but some of their attributes
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index b1572ecef9028c..a62c033a9045b8 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -335,18 +335,15 @@ class ASTRecordReader
Attr *readAttr();
/// Reads attributes from the current stream position, advancing Idx.
- void readAttributes(AttrVec &Attrs);
+ /// For some attributes (where type depends on itself recursively), defer
+ /// reading the attribute until the type has been read.
+ void readAttributes(AttrVec &Attrs, Decl *D = nullptr);
/// 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);
+ Attr *readOrDeferAttrFor(Decl *D);
/// Read an BTFTypeTagAttr object.
- BTFTypeTagAttr *readBTFTypeTagAttr() {
- return cast<BTFTypeTagAttr>(readAttr());
- }
+ BTFTypeTagAttr *readBTFTypeTagAttr();
/// Reads a token out of a record, advancing Idx.
Token readToken() {
@@ -361,10 +358,6 @@ 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
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7dedccdb31caf6..3c46ced5c3469d 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -612,7 +612,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
if (HasAttrs) {
AttrVec Attrs;
- Record.readOrDeferAttributes(Attrs, D);
+ Record.readAttributes(Attrs, D);
// Avoid calling setAttrs() directly because it uses Decl::getASTContext()
// internally which is unsafe during derialization.
D->setAttrsImpl(Attrs, Reader.getContext());
@@ -3129,7 +3129,7 @@ class AttrReader {
}
/// Reads one attribute from the current stream position, advancing Idx.
-Attr *ASTRecordReader::readOrDeferAttrImpl(Decl *D) {
+Attr *ASTRecordReader::readAttr() {
AttrReader Record(*this);
auto V = Record.readInt();
if (!V)
@@ -3139,20 +3139,6 @@ Attr *ASTRecordReader::readOrDeferAttrImpl(Decl *D) {
// 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();
@@ -3178,31 +3164,34 @@ Attr *ASTRecordReader::readOrDeferAttrImpl(Decl *D) {
return New;
}
-void ASTRecordReader::readOrDeferAttributesImpl(AttrVec &Attrs, Decl *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::readAttributes(AttrVec &Attrs, Decl *D) {
for (unsigned I = 0, E = readInt(); I != E; ++I)
- if (auto *A = readOrDeferAttr(D))
+ if (auto *A = readOrDeferAttrFor(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);
+Attr *ASTRecordReader::readOrDeferAttrFor(Decl *D) {
+ AttrReader Record(*this);
+ unsigned SkipCount = Record.readInt();
+ if (!SkipCount)
+ return readAttr();
+ Reader->PendingDeferredAttributes.push_back({Record.getCurrentIdx(), D});
+ Record.skipInts(SkipCount);
+ return nullptr;
}
-/// 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);
+BTFTypeTagAttr *ASTRecordReader::readBTFTypeTagAttr() {
+ AttrReader Record(*this);
+ // Read and ignore the skip count, since this attribute is not deferred.
+ Record.readInt();
+ return cast<BTFTypeTagAttr>(readAttr());
}
//===----------------------------------------------------------------------===//
@@ -4464,7 +4453,7 @@ void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
}
void ASTReader::loadDeferredAttribute(const DeferredAttribute &DA) {
- Decl *D = DA.ParentDecl;
+ Decl *D = DA.TargetedDecl;
ModuleFile *M = getOwningModuleFile(D);
unsigned LocalDeclIndex = D->getGlobalID().getLocalDeclIndex();
@@ -4502,7 +4491,7 @@ void ASTReader::loadDeferredAttribute(const DeferredAttribute &DA) {
}
Record.skipInts(DA.RecordIdx);
- Attr *A = Record.readOrDeferAttr(nullptr);
+ Attr *A = Record.readAttr();
getContext().getDeclAttrs(D).push_back(A);
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d24ca08e1cc988..cb4ded90182f13 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4910,14 +4910,11 @@ void ASTRecordWriter::AddAttr(const Attr *A) {
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);
- }
+ // Add placeholder for the size of deferred attribute.
+ Record.push_back(0);
+ Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
Record.AddIdentifierRef(A->getAttrName());
Record.AddIdentifierRef(A->getScopeName());
Record.AddSourceRange(A->getRange());
@@ -4929,9 +4926,9 @@ void ASTRecordWriter::AddAttr(const Attr *A) {
#include "clang/Serialization/AttrPCHWrite.inc"
- if (A->getKind() == attr::PreferredName) {
- // Record the actual size of preferred_name attribute (-1 to count the
- // placeholder).
+ if (A->shouldDeferDeserialization()) {
+ // Record the actual size of deferred attribute (-1 to count the size bit
+ // itself).
Record[SkipIdx] = Record.size() - SkipIdx - 1;
}
}
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index cc6a8eaebd44ec..41730eba32ce27 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3043,6 +3043,10 @@ static void emitAttributes(const RecordKeeper &Records, raw_ostream &OS,
<< (R.getValueAsBit("InheritEvenIfAlreadyPresent") ? "true"
: "false");
}
+ if (R.getValueAsBit("DeferDeserialization")) {
+ OS << ", "
+ << "/*DeferDeserialization=*/true";
+ }
OS << ")\n";
for (auto const &ai : Args) {
>From f57f83f93b59c8d8b4f50ed6e4bd24543392f722 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 16 Jan 2025 09:39:54 +0100
Subject: [PATCH 4/7] [Modules] Fix formatting.
---
clang/lib/Serialization/ASTReaderDecl.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 3c46ced5c3469d..5a841ee31bd594 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3173,7 +3173,6 @@ void ASTRecordReader::readAttributes(AttrVec &Attrs, Decl *D) {
Attrs.push_back(A);
}
-
/// 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.
>From 1d8676b102d837d837d9ec316d6edfaca80f4c47 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 16 Jan 2025 13:49:13 +0100
Subject: [PATCH 5/7] [Modules] Address failing tests and review comments.
---
clang/include/clang/Basic/Attr.td | 8 +++++++-
.../clang/Serialization/ASTRecordReader.h | 7 ++++++-
clang/lib/Serialization/ASTReaderDecl.cpp | 19 ++++++++++---------
clang/lib/Serialization/ASTWriter.cpp | 10 +++++-----
4 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index c06e060f576dc9..8668968a7c1b38 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -715,7 +715,9 @@ class Attr {
list<Documentation> Documentation;
// Set to true if deserialization of this attribute must be deferred until
// the parent Decl is fully deserialized (during header module file
- // deserialization).
+ // deserialization). E.g., this is the case for the preferred_name attribute,
+ // since its type deserialization depends on its target Decl type.
+ // (See https://github.com/llvm/llvm-project/issues/56490 for details).
bit DeferDeserialization = 0;
}
@@ -3244,6 +3246,10 @@ def PreferredName : InheritableAttr {
let InheritEvenIfAlreadyPresent = 1;
let MeaningfulToClassTemplateDefinition = 1;
let TemplateDependent = 1;
+ // Type of this attribute depends on the target Decl type.
+ // Therefore, its deserialization must be deferred until
+ // deserialization of the target Decl is complete
+ // (for header modules).
let DeferDeserialization = 1;
}
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index a62c033a9045b8..6aec99f7763265 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -83,6 +83,9 @@ class ASTRecordReader
/// Returns the current value in this record, without advancing.
uint64_t peekInt() { return Record[Idx]; }
+ /// Returns the next value in this record, without advancing.
+ uint64_t peekNextInt() { return Record[Idx + 1]; }
+
/// Skips the specified number of values.
void skipInts(unsigned N) { Idx += N; }
@@ -343,7 +346,9 @@ class ASTRecordReader
Attr *readOrDeferAttrFor(Decl *D);
/// Read an BTFTypeTagAttr object.
- BTFTypeTagAttr *readBTFTypeTagAttr();
+ BTFTypeTagAttr *readBTFTypeTagAttr() {
+ return cast<BTFTypeTagAttr>(readAttr());
+ }
/// Reads a token out of a record, advancing Idx.
Token readToken() {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 5a841ee31bd594..56b63508828738 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3088,6 +3088,10 @@ class AttrReader {
return Reader.readInt();
}
+ uint64_t peekNextInt() {
+ return Reader.peekNextInt();
+ }
+
bool readBool() { return Reader.readBool(); }
SourceRange readSourceRange() {
@@ -3132,9 +3136,13 @@ class AttrReader {
Attr *ASTRecordReader::readAttr() {
AttrReader Record(*this);
auto V = Record.readInt();
- if (!V)
+ if (!V)
return nullptr;
+ // Read and ignore the skip count, since attribute deserialization is not
+ // deferred on this pass.
+ Record.readInt();
+
Attr *New = nullptr;
// Kind is stored as a 1-based integer because 0 is used to indicate a null
// Attr pointer.
@@ -3178,7 +3186,7 @@ void ASTRecordReader::readAttributes(AttrVec &Attrs, Decl *D) {
/// reading the attribute until the type has been read.
Attr *ASTRecordReader::readOrDeferAttrFor(Decl *D) {
AttrReader Record(*this);
- unsigned SkipCount = Record.readInt();
+ unsigned SkipCount = Record.peekNextInt();
if (!SkipCount)
return readAttr();
Reader->PendingDeferredAttributes.push_back({Record.getCurrentIdx(), D});
@@ -3186,13 +3194,6 @@ Attr *ASTRecordReader::readOrDeferAttrFor(Decl *D) {
return nullptr;
}
-BTFTypeTagAttr *ASTRecordReader::readBTFTypeTagAttr() {
- AttrReader Record(*this);
- // Read and ignore the skip count, since this attribute is not deferred.
- Record.readInt();
- return cast<BTFTypeTagAttr>(readAttr());
-}
-
//===----------------------------------------------------------------------===//
// ASTReader Implementation
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index cb4ded90182f13..1c8baa1c64c65a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4910,11 +4910,11 @@ void ASTRecordWriter::AddAttr(const Attr *A) {
if (!A)
return Record.push_back(0);
+ Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
+
auto SkipIdx = Record.size();
// Add placeholder for the size of deferred attribute.
Record.push_back(0);
-
- Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
Record.AddIdentifierRef(A->getAttrName());
Record.AddIdentifierRef(A->getScopeName());
Record.AddSourceRange(A->getRange());
@@ -4927,9 +4927,9 @@ void ASTRecordWriter::AddAttr(const Attr *A) {
#include "clang/Serialization/AttrPCHWrite.inc"
if (A->shouldDeferDeserialization()) {
- // Record the actual size of deferred attribute (-1 to count the size bit
- // itself).
- Record[SkipIdx] = Record.size() - SkipIdx - 1;
+ // Record the actual size of deferred attribute (+ 1 to count the attribute
+ // kind).
+ Record[SkipIdx] = Record.size() - SkipIdx + 1;
}
}
>From f685717bc6b3915599a05efdd692161e326c758f Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 16 Jan 2025 13:53:37 +0100
Subject: [PATCH 6/7] [Modules] Fix formatting.
---
clang/lib/Serialization/ASTReaderDecl.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 56b63508828738..e4f7e710d989e8 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3088,9 +3088,7 @@ class AttrReader {
return Reader.readInt();
}
- uint64_t peekNextInt() {
- return Reader.peekNextInt();
- }
+ uint64_t peekNextInt() { return Reader.peekNextInt(); }
bool readBool() { return Reader.readBool(); }
@@ -3136,7 +3134,7 @@ class AttrReader {
Attr *ASTRecordReader::readAttr() {
AttrReader Record(*this);
auto V = Record.readInt();
- if (!V)
+ if (!V)
return nullptr;
// Read and ignore the skip count, since attribute deserialization is not
>From 9174328df69bb1361e503bee8f7c8b16c3cd1578 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 16 Jan 2025 15:14:31 +0100
Subject: [PATCH 7/7] [Modules] Address review comments.
---
clang/include/clang/Serialization/ASTRecordReader.h | 7 +++++--
clang/lib/Serialization/ASTReaderDecl.cpp | 8 +++++---
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index 6aec99f7763265..a29972fcf73a8d 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -83,8 +83,11 @@ class ASTRecordReader
/// Returns the current value in this record, without advancing.
uint64_t peekInt() { return Record[Idx]; }
- /// Returns the next value in this record, without advancing.
- uint64_t peekNextInt() { return Record[Idx + 1]; }
+ /// Returns the next N values in this record, without advancing.
+ uint64_t peekInts(unsigned N) { return Record[Idx + N]; }
+
+ /// Skips the current value.
+ void skipInt() { Idx += 1; }
/// Skips the specified number of values.
void skipInts(unsigned N) { Idx += N; }
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index e4f7e710d989e8..a48a1b298b8f36 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3088,7 +3088,7 @@ class AttrReader {
return Reader.readInt();
}
- uint64_t peekNextInt() { return Reader.peekNextInt(); }
+ uint64_t peekInts(unsigned N) { return Reader.peekInts(N); }
bool readBool() { return Reader.readBool(); }
@@ -3120,6 +3120,8 @@ class AttrReader {
return Reader.readVersionTuple();
}
+ void skipInt() { Reader.skipInts(1); }
+
void skipInts(unsigned N) { Reader.skipInts(N); }
unsigned getCurrentIdx() { return Reader.getIdx(); }
@@ -3139,7 +3141,7 @@ Attr *ASTRecordReader::readAttr() {
// Read and ignore the skip count, since attribute deserialization is not
// deferred on this pass.
- Record.readInt();
+ Record.skipInt();
Attr *New = nullptr;
// Kind is stored as a 1-based integer because 0 is used to indicate a null
@@ -3184,7 +3186,7 @@ void ASTRecordReader::readAttributes(AttrVec &Attrs, Decl *D) {
/// reading the attribute until the type has been read.
Attr *ASTRecordReader::readOrDeferAttrFor(Decl *D) {
AttrReader Record(*this);
- unsigned SkipCount = Record.peekNextInt();
+ unsigned SkipCount = Record.peekInts(1);
if (!SkipCount)
return readAttr();
Reader->PendingDeferredAttributes.push_back({Record.getCurrentIdx(), D});
More information about the cfe-commits
mailing list