[clang] [WIP][Modules] Delay deserialization of preferred_name attribute at r… (PR #122726)
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 13 07:35:55 PST 2025
https://github.com/VitaNuo updated https://github.com/llvm/llvm-project/pull/122726
>From 4266533a27b7ecafb96df081682ce0ae8fd32584 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Mon, 13 Jan 2025 16:26:32 +0100
Subject: [PATCH 1/2] [WIP][Modules] Delay deserialization of preferred_name
attribute at record level.
---
clang/include/clang/Serialization/ASTReader.h | 8 ++
.../clang/Serialization/ASTRecordReader.h | 10 +++
clang/lib/Serialization/ASTReader.cpp | 5 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 89 +++++++++++++++++--
clang/lib/Serialization/ASTWriter.cpp | 14 ++-
clang/test/Modules/preferred_name.cppm | 12 ++-
6 files changed, 128 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 9f978762a6fb6b..38f4cf6990296b 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1205,6 +1205,12 @@ class ASTReader
/// been completed.
std::deque<PendingDeclContextInfo> PendingDeclContextInfos;
+ struct PendingPreferredNameAttribute {
+ uint64_t RecordIdx;
+ Decl *D;
+ };
+ SmallVector<PendingPreferredNameAttribute> PendingPreferredNameAttributes;
+
template <typename DeclTy>
using DuplicateObjCDecls = std::pair<DeclTy *, DeclTy *>;
@@ -1551,6 +1557,8 @@ class ASTReader
void loadPendingDeclChain(Decl *D, uint64_t LocalOffset);
void loadObjCCategories(GlobalDeclID ID, ObjCInterfaceDecl *D,
unsigned PreviousGeneration = 0);
+ void loadPreferredNameAttribute(
+ const PendingPreferredNameAttribute &PreferredNameAttribute);
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..252015c472bc00 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 *readAttr(Decl *D);
+
+ /// Reads attributes from the current stream position, advancing Idx.
+ void readAttributes(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 *readAttrImpl(Decl *D);
+ void readAttributesImpl(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..971a2538e9ed74 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 != PendingPreferredNameAttributes.size(); ++I)
+ loadPreferredNameAttribute(PendingPreferredNameAttributes[I]);
+ PendingPreferredNameAttributes.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..7aacfaab532e5e 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.readAttributes(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,22 @@ 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::readAttrImpl(Decl *D) {
AttrReader Record(*this);
auto V = Record.readInt();
if (!V)
@@ -3134,6 +3143,17 @@ 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);
+ if (Kind == attr::PreferredName && D != nullptr) {
+ if (D != nullptr) {
+ Reader->PendingPreferredNameAttributes.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 +3179,27 @@ Attr *ASTRecordReader::readAttr() {
return New;
}
-/// Reads attributes from the current stream position.
-void ASTRecordReader::readAttributes(AttrVec &Attrs) {
+void ASTRecordReader::readAttributesImpl(AttrVec &Attrs, Decl *D) {
for (unsigned I = 0, E = readInt(); I != E; ++I)
- if (auto *A = readAttr())
+ if (auto *A = readAttr(D))
Attrs.push_back(A);
}
+Attr *ASTRecordReader::readAttr() { return readAttrImpl(nullptr); }
+
+/// Reads attributes from the current stream position.
+void ASTRecordReader::readAttributes(AttrVec &Attrs) {
+ readAttributesImpl(Attrs, nullptr);
+}
+
+/// Reads one attribute from the current stream position, advancing Idx.
+Attr *ASTRecordReader::readAttr(Decl *D) { return readAttrImpl(D); }
+
+/// Reads attributes from the current stream position, advancing Idx.
+void ASTRecordReader::readAttributes(AttrVec &Attrs, Decl *D) {
+ readAttributesImpl(Attrs, D);
+}
+
//===----------------------------------------------------------------------===//
// ASTReader Implementation
//===----------------------------------------------------------------------===//
@@ -4424,6 +4458,51 @@ void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
ASTDeclReader::attachLatestDecl(CanonDecl, MostRecent);
}
+void ASTReader::loadPreferredNameAttribute(
+ const PendingPreferredNameAttribute &PreferredNameAttribute) {
+ Decl *D = PreferredNameAttribute.D;
+ 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_CXX_RECORD) {
+ llvm::report_fatal_error(
+ Twine("ASTReader::loadPreferredNameAttribute failed reading rec code: "
+ "expected CXXRecord") +
+ toString(MaybeCode.takeError()));
+ }
+
+ Record.skipInts(PreferredNameAttribute.RecordIdx);
+ Attr *PreferredNameAttr = Record.readAttr(nullptr);
+ AttrVec &Attrs = getContext().getDeclAttrs(D);
+ Attrs.push_back(PreferredNameAttr);
+}
+
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..c96a635592cd8a 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"
@@ -4909,12 +4910,16 @@ void ASTRecordWriter::AddAttr(const Attr *A) {
// 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 +4930,11 @@ 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..4a73961683bdd3 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'}}
\ No newline at end of file
>From bd13df1103f289ebeeb61c53343c8ddfc21ed7b7 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Mon, 13 Jan 2025 16:35:35 +0100
Subject: [PATCH 2/2] [WIP][Modules] Format.
---
clang/lib/Serialization/ASTReaderDecl.cpp | 8 ++------
clang/test/Modules/preferred_name.cppm | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7aacfaab532e5e..aa938b5171dc46 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3118,13 +3118,9 @@ class AttrReader {
return Reader.readVersionTuple();
}
- void skipInts(unsigned N) {
- Reader.skipInts(N);
- }
+ void skipInts(unsigned N) { Reader.skipInts(N); }
- unsigned getCurrentIdx() {
- return Reader.getIdx();
- }
+ unsigned getCurrentIdx() { return Reader.getIdx(); }
OMPTraitInfo *readOMPTraitInfo() { return Reader.readOMPTraitInfo(); }
diff --git a/clang/test/Modules/preferred_name.cppm b/clang/test/Modules/preferred_name.cppm
index 4a73961683bdd3..86ba6ae96db998 100644
--- a/clang/test/Modules/preferred_name.cppm
+++ b/clang/test/Modules/preferred_name.cppm
@@ -65,4 +65,4 @@ import A;
#include "foo.h"
import A;
foo test;
-int size = test.size(); // expected-error {{no member named 'size' in 'foo'}}
\ No newline at end of file
+int size = test.size(); // expected-error {{no member named 'size' in 'foo'}}
More information about the cfe-commits
mailing list