[clang] [WIP] [Modules] Delay reading type source info of the preferred_name attribute. (PR #122250)
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 9 06:13:29 PST 2025
https://github.com/VitaNuo updated https://github.com/llvm/llvm-project/pull/122250
>From 0a615576181a538bc0d8eff6499ad87cbdeb89c3 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 9 Jan 2025 09:36:35 +0100
Subject: [PATCH 1/2] [WIP] Delay reading type source info of the
preferred_name attribute.
---
clang/include/clang/Serialization/ASTReader.h | 18 ++++-
.../clang/Serialization/ASTRecordReader.h | 7 ++
clang/lib/Serialization/ASTReader.cpp | 25 +++++++
clang/lib/Serialization/ASTReaderDecl.cpp | 72 ++++++++++++++++---
clang/lib/Serialization/ASTWriter.cpp | 5 +-
clang/test/Modules/preferred_name.cppm | 13 ----
clang/test/Modules/preferred_name_2.cppm | 47 ++++++++++++
7 files changed, 163 insertions(+), 24 deletions(-)
create mode 100644 clang/test/Modules/preferred_name_2.cppm
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index f739fe688c110d..3c79a4c86a5617 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -13,7 +13,10 @@
#ifndef LLVM_CLANG_SERIALIZATION_ASTREADER_H
#define LLVM_CLANG_SERIALIZATION_ASTREADER_H
+#include "clang/AST/DeclID.h"
+#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/Type.h"
+#include "clang/Basic/AttributeCommonInfo.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/IdentifierTable.h"
@@ -1156,6 +1159,19 @@ class ASTReader
SmallVector<std::pair<VarDecl *, serialization::TypeID>, 16>
PendingDeducedVarTypes;
+ struct PendingPreferredNameAttribute {
+ Decl* D;
+ AttributeCommonInfo Info;
+ serialization::TypeID TypeID;
+ bool isInherited;
+ bool isImplicit;
+ bool isPackExpansion;
+ SourceLocation ElaboratedTypedefSourceLocation;
+ NestedNameSpecifierLoc NNS;
+ SourceLocation TypedefSourceLocation;
+ };
+ std::deque<PendingPreferredNameAttribute> PendingPreferredNameAttributes;
+
/// The list of redeclaration chains that still need to be
/// reconstructed, and the local offset to the corresponding list
/// of redeclarations.
@@ -1419,7 +1435,7 @@ class ASTReader
const serialization::reader::DeclContextLookupTable *
getLoadedLookupTables(DeclContext *Primary) const;
-private:
+ private:
struct ImportedModule {
ModuleFile *Mod;
ModuleFile *ImportedBy;
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index 2561418b78ca7f..f476d72fcf9e71 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -337,6 +337,13 @@ 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());
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index ec85fad3389a1c..4ee04552f13a96 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -23,6 +23,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclFriend.h"
#include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclID.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/DeclarationName.h"
@@ -41,6 +42,7 @@
#include "clang/AST/TypeLocVisitor.h"
#include "clang/AST/UnresolvedSet.h"
#include "clang/Basic/ASTSourceDescriptor.h"
+#include "clang/Basic/AttributeCommonInfo.h"
#include "clang/Basic/CommentOptions.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticError.h"
@@ -9860,6 +9862,29 @@ void ASTReader::finishPendingActions() {
}
PendingDeducedVarTypes.clear();
+ ASTContext &Context = getContext();
+ for (unsigned I = 0; I != PendingPreferredNameAttributes.size(); ++I) {
+ auto *D = PendingPreferredNameAttributes[I].D;
+ QualType InfoTy = GetType(PendingPreferredNameAttributes[I].TypeID);
+ TypeSourceInfo *TInfo = nullptr;
+ if (!InfoTy.isNull()) {
+ TInfo = getContext().CreateTypeSourceInfo(InfoTy);
+ // TODO - this piece doesn't work yet
+ ElaboratedTypeLoc &Loc = (ElaboratedTypeLoc)TInfo->getTypeLoc();
+ Loc.setElaboratedKeywordLoc(PendingPreferredNameAttributes[I].ElaboratedTypedefSourceLocation);
+ Loc.setQualifierLoc(PendingPreferredNameAttributes[I].NNS);
+ Loc.setNameLoc(PendingPreferredNameAttributes[I].TypedefSourceLocation);
+ }
+
+ AttrVec &Attrs = getContext().getDeclAttrs(D);
+ PreferredNameAttr *New = new (Context) PreferredNameAttr(Context, PendingPreferredNameAttributes[I].Info, TInfo);
+ cast<InheritableAttr>(New)->setInherited(PendingPreferredNameAttributes[I].isInherited);
+ New->setImplicit(PendingPreferredNameAttributes[I].isImplicit);
+ New->setPackExpansion(PendingPreferredNameAttributes[I].isPackExpansion);
+ Attrs.push_back(New);
+ }
+ 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 6ece3ba7af9f4b..5e67f4fa8bf107 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -22,6 +22,7 @@
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclFriend.h"
+#include "clang/AST/DeclID.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclOpenMP.h"
#include "clang/AST/DeclTemplate.h"
@@ -638,7 +639,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());
@@ -725,12 +726,12 @@ void ASTDeclReader::VisitTypeDecl(TypeDecl *TD) {
RedeclarableResult ASTDeclReader::VisitTypedefNameDecl(TypedefNameDecl *TD) {
RedeclarableResult Redecl = VisitRedeclarable(TD);
VisitTypeDecl(TD);
- TypeSourceInfo *TInfo = readTypeSourceInfo();
- if (Record.readInt()) { // isModed
- QualType modedT = Record.readType();
- TD->setModedTypeSourceInfo(TInfo, modedT);
- } else
- TD->setTypeSourceInfo(TInfo);
+ TypeSourceInfo *TInfo = readTypeSourceInfo();
+ if (Record.readInt()) { // isModed
+ QualType modedT = Record.readType();
+ TD->setModedTypeSourceInfo(TInfo, modedT);
+ } else
+ TD->setTypeSourceInfo(TInfo);
// Read and discard the declaration for which this is a typedef name for
// linkage, if it exists. We cannot rely on our type to pull in this decl,
// because it might have been merged with a type from another module and
@@ -3167,6 +3168,54 @@ Attr *ASTRecordReader::readAttr() {
return New;
}
+Attr *ASTRecordReader::readAttr(Decl *D) {
+ AttrReader Record(*this);
+ auto V = Record.readInt();
+ if (!V)
+ return nullptr;
+
+ 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});
+ if (Kind == attr::PreferredName) {
+ bool isInherited = Record.readInt();
+ bool isImplicit = Record.readInt();
+ bool isPackExpansion = Record.readInt();
+
+
+ serialization::TypeID TypeID = getGlobalTypeID(Record.readInt());
+ SourceLocation SL = Record.readSourceLocation();
+ NestedNameSpecifierLoc NNL = readNestedNameSpecifierLoc();
+ SourceLocation TSL = Record.readSourceLocation();
+ Reader->PendingPreferredNameAttributes.push_back({D, Info, TypeID, isInherited, isImplicit, isPackExpansion, SL, NNL, TSL});
+ return nullptr;
+ }
+
+#include "clang/Serialization/AttrPCHRead.inc"
+
+ assert(New && "Unable to decode attribute?");
+ return New;
+}
+
/// Reads attributes from the current stream position.
void ASTRecordReader::readAttributes(AttrVec &Attrs) {
for (unsigned I = 0, E = readInt(); I != E; ++I)
@@ -3174,6 +3223,12 @@ void ASTRecordReader::readAttributes(AttrVec &Attrs) {
Attrs.push_back(A);
}
+void ASTRecordReader::readAttributes(AttrVec &Attrs, Decl* D) {
+ for (unsigned I = 0, E = readInt(); I != E; ++I)
+ if (auto *A = readAttr(D))
+ Attrs.push_back(A);
+}
+
//===----------------------------------------------------------------------===//
// ASTReader Implementation
//===----------------------------------------------------------------------===//
@@ -3572,7 +3627,7 @@ void mergeInheritableAttributes(ASTReader &Reader, Decl *D, Decl *Previous) {
template<typename DeclT>
void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
Redeclarable<DeclT> *D,
- Decl *Previous, Decl *Canon) {
+ Decl *Previous, Decl *Canon) {
D->RedeclLink.setPrevious(cast<DeclT>(Previous));
D->First = cast<DeclT>(Previous)->First;
}
@@ -4199,6 +4254,7 @@ Decl *ASTReader::ReadDeclRecord(GlobalDeclID ID) {
ReadVisibleDeclContextStorage(*Loc.F, DeclsCursor, Offsets.second, ID))
return nullptr;
}
+ // This is the crashing line.
assert(Record.getIdx() == Record.size());
// Load any relevant update records.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index a52d59c61c4ce6..a259a55cd397a6 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4720,8 +4720,9 @@ 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)
+ // if (!A || (isa<PreferredNameAttr>(A) &&
+ // Writer->isWritingStdCXXNamedModules()))
return Record.push_back(0);
Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
diff --git a/clang/test/Modules/preferred_name.cppm b/clang/test/Modules/preferred_name.cppm
index 806781a81c5ca7..255e915f824c8d 100644
--- a/clang/test/Modules/preferred_name.cppm
+++ b/clang/test/Modules/preferred_name.cppm
@@ -16,8 +16,6 @@
//
// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm
// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use.cppm -verify -fsyntax-only
-// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use1.cpp -verify -fsyntax-only
-// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use2.cpp -verify -fsyntax-only
//
//--- foo.h
template<class _CharT>
@@ -42,7 +40,6 @@ inline foo_templ<char> bar()
module;
#include "foo.h"
export module A;
-export using ::foo_templ;
//--- Use.cppm
// expected-no-diagnostics
@@ -50,13 +47,3 @@ module;
#include "foo.h"
export module Use;
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}}
-
-//--- Use2.cpp
-// expected-no-diagnostics
-#include "foo.h"
-import A;
diff --git a/clang/test/Modules/preferred_name_2.cppm b/clang/test/Modules/preferred_name_2.cppm
new file mode 100644
index 00000000000000..b1d1c5a49b39ce
--- /dev/null
+++ b/clang/test/Modules/preferred_name_2.cppm
@@ -0,0 +1,47 @@
+// Tests that the ODR check wouldn't produce false-positive result for preferred_name attribute.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use1.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use2.cpp -verify -fsyntax-only
+
+// Test again with reduced BMI.
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use.cppm -verify -fsyntax-only
+//
+//--- foo.h
+template<class _CharT>
+class foo_templ;
+
+typedef foo_templ<char> foo;
+
+template<class _CharT>
+class foo_templ {
+public:
+ foo_templ() {}
+};
+
+inline foo_templ<char> bar()
+{
+ return foo_templ<char>();
+}
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+
+//--- Use.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Use;
+import A;
>From b9670533a4dca0150bd854a207792bdaab0bd096 Mon Sep 17 00:00:00 2001
From: Viktoriia Bakalova <bakalova at google.com>
Date: Thu, 9 Jan 2025 15:13:14 +0100
Subject: [PATCH 2/2] [WIP] Set type on the attribute in finish pending
actions.
---
clang/lib/Serialization/ASTReader.cpp | 11 +++++++----
clang/lib/Serialization/ASTReaderDecl.cpp | 2 --
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 4ee04552f13a96..c8a42471af819e 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9870,10 +9870,13 @@ void ASTReader::finishPendingActions() {
if (!InfoTy.isNull()) {
TInfo = getContext().CreateTypeSourceInfo(InfoTy);
// TODO - this piece doesn't work yet
- ElaboratedTypeLoc &Loc = (ElaboratedTypeLoc)TInfo->getTypeLoc();
- Loc.setElaboratedKeywordLoc(PendingPreferredNameAttributes[I].ElaboratedTypedefSourceLocation);
- Loc.setQualifierLoc(PendingPreferredNameAttributes[I].NNS);
- Loc.setNameLoc(PendingPreferredNameAttributes[I].TypedefSourceLocation);
+ if (auto Loc = TInfo->getTypeLoc().getAs<ElaboratedTypeLoc>()) {
+ Loc.setElaboratedKeywordLoc(PendingPreferredNameAttributes[I].ElaboratedTypedefSourceLocation);
+ Loc.setQualifierLoc(PendingPreferredNameAttributes[I].NNS);
+ if (auto TypedefLoc = Loc.getNextTypeLoc().getAs<TypedefTypeLoc>()) {
+ TypedefLoc.setNameLoc(PendingPreferredNameAttributes[I].TypedefSourceLocation);
+ }
+ }
}
AttrVec &Attrs = getContext().getDeclAttrs(D);
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 5e67f4fa8bf107..5874b062bcd894 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3200,8 +3200,6 @@ Attr *ASTRecordReader::readAttr(Decl *D) {
bool isInherited = Record.readInt();
bool isImplicit = Record.readInt();
bool isPackExpansion = Record.readInt();
-
-
serialization::TypeID TypeID = getGlobalTypeID(Record.readInt());
SourceLocation SL = Record.readSourceLocation();
NestedNameSpecifierLoc NNL = readNestedNameSpecifierLoc();
More information about the cfe-commits
mailing list