[Serialization] Add a callback to register new created predefined decls for DeserializationListener #102855
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ls for DeserializationListener Close llvm#102684 The root cause of the issue is, it is possible that the predefined decl is not registered at the beginning of writing a module file but got created during the process of writing from reading. This is incorrect. The predefined decls should always be predefined decls. Another deep thought about the issue is, we shouldn't read any new things after we start to write the module file. But this is another deeper question.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Chuanqi Xu (ChuanqiXu9) ChangesClose #102684 The root cause of the issue is, it is possible that the predefined decl is not registered at the beginning of writing a module file but got created during the process of writing from reading. This is incorrect. The predefined decls should always be predefined decls. Another deep thought about the issue is, we shouldn't read any new things after we start to write the module file. But this is another deeper question. Full diff: https://github.com/llvm/llvm-project/pull/102855.diff 8 Files Affected:
diff --git a/clang/include/clang/Frontend/MultiplexConsumer.h b/clang/include/clang/Frontend/MultiplexConsumer.h
index e49e3392d1f317..3a7670d7a51aa6 100644
--- a/clang/include/clang/Frontend/MultiplexConsumer.h
+++ b/clang/include/clang/Frontend/MultiplexConsumer.h
@@ -36,6 +36,7 @@ class MultiplexASTDeserializationListener : public ASTDeserializationListener {
void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
void TypeRead(serialization::TypeIdx Idx, QualType T) override;
void DeclRead(GlobalDeclID ID, const Decl *D) override;
+ void PredefinedDeclBuilt(PredefinedDeclIDs ID, const Decl *D) override;
void SelectorRead(serialization::SelectorID iD, Selector Sel) override;
void MacroDefinitionRead(serialization::PreprocessedEntityID,
MacroDefinitionRecord *MD) override;
diff --git a/clang/include/clang/Serialization/ASTDeserializationListener.h b/clang/include/clang/Serialization/ASTDeserializationListener.h
index 1d81a9ae3fe2eb..ea96faa07c1917 100644
--- a/clang/include/clang/Serialization/ASTDeserializationListener.h
+++ b/clang/include/clang/Serialization/ASTDeserializationListener.h
@@ -45,6 +45,8 @@ class ASTDeserializationListener {
virtual void TypeRead(serialization::TypeIdx Idx, QualType T) { }
/// A decl was deserialized from the AST file.
virtual void DeclRead(GlobalDeclID ID, const Decl *D) {}
+ /// A predefined decl was built during the serialization.
+ virtual void PredefinedDeclBuilt(PredefinedDeclIDs ID, const Decl *D) {}
/// A selector was read from the AST file.
virtual void SelectorRead(serialization::SelectorID iD, Selector Sel) {}
/// A macro definition was read from the AST file.
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 549396a3b6b387..a0e90e62bd60ec 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1559,6 +1559,9 @@ class ASTReader
std::pair<ModuleFile *, unsigned>
translateTypeIDToIndex(serialization::TypeID ID) const;
+ /// Get a predefined Decl from ASTContext.
+ Decl *getPredefinedDecl(PredefinedDeclIDs ID);
+
public:
/// Load the AST file and validate its contents against the given
/// Preprocessor.
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 71a7c28047e318..a4cc95cd1373fa 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -869,6 +869,7 @@ class ASTWriter : public ASTDeserializationListener,
void IdentifierRead(serialization::IdentifierID ID, IdentifierInfo *II) override;
void MacroRead(serialization::MacroID ID, MacroInfo *MI) override;
void TypeRead(serialization::TypeIdx Idx, QualType T) override;
+ void PredefinedDeclBuilt(PredefinedDeclIDs ID, const Decl *D) override;
void SelectorRead(serialization::SelectorID ID, Selector Sel) override;
void MacroDefinitionRead(serialization::PreprocessedEntityID ID,
MacroDefinitionRecord *MD) override;
diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp
index 651c55aeed5408..2158d176d18929 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -58,6 +58,11 @@ void MultiplexASTDeserializationListener::DeclRead(GlobalDeclID ID,
Listeners[i]->DeclRead(ID, D);
}
+void MultiplexASTDeserializationListener::PredefinedDeclBuilt(PredefinedDeclIDs ID, const Decl *D) {
+ for (size_t i = 0, e = Listeners.size(); i != e; ++i)
+ Listeners[i]->PredefinedDeclBuilt(ID, D);
+}
+
void MultiplexASTDeserializationListener::SelectorRead(
serialization::SelectorID ID, Selector Sel) {
for (size_t i = 0, e = Listeners.size(); i != e; ++i)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 5f9cfa3cf4bfaa..511e2df7ad3230 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7787,7 +7787,10 @@ SourceLocation ASTReader::getSourceLocationForDeclID(GlobalDeclID ID) {
return Loc;
}
-static Decl *getPredefinedDecl(ASTContext &Context, PredefinedDeclIDs ID) {
+Decl *ASTReader::getPredefinedDecl(PredefinedDeclIDs ID) {
+ assert(ContextObj && "reading predefined decl without AST context");
+ ASTContext &Context = *ContextObj;
+ Decl *NewLoaded = nullptr;
switch (ID) {
case PREDEF_DECL_NULL_ID:
return nullptr;
@@ -7796,54 +7799,106 @@ static Decl *getPredefinedDecl(ASTContext &Context, PredefinedDeclIDs ID) {
return Context.getTranslationUnitDecl();
case PREDEF_DECL_OBJC_ID_ID:
- return Context.getObjCIdDecl();
+ if (Context.ObjCIdDecl)
+ return Context.ObjCIdDecl;
+ NewLoaded = Context.getObjCIdDecl();
+ break;
case PREDEF_DECL_OBJC_SEL_ID:
- return Context.getObjCSelDecl();
+ if (Context.ObjCSelDecl)
+ return Context.ObjCSelDecl;
+ NewLoaded = Context.getObjCSelDecl();
+ break;
case PREDEF_DECL_OBJC_CLASS_ID:
- return Context.getObjCClassDecl();
+ if (Context.ObjCClassDecl)
+ return Context.ObjCClassDecl;
+ NewLoaded = Context.getObjCClassDecl();
+ break;
case PREDEF_DECL_OBJC_PROTOCOL_ID:
- return Context.getObjCProtocolDecl();
+ if (Context.ObjCProtocolClassDecl)
+ return Context.ObjCProtocolClassDecl;
+ NewLoaded = Context.getObjCProtocolDecl();
+ break;
case PREDEF_DECL_INT_128_ID:
- return Context.getInt128Decl();
+ if (Context.Int128Decl)
+ return Context.Int128Decl;
+ NewLoaded = Context.getInt128Decl();
+ break;
case PREDEF_DECL_UNSIGNED_INT_128_ID:
- return Context.getUInt128Decl();
+ if (Context.UInt128Decl)
+ return Context.UInt128Decl;
+ NewLoaded = Context.getUInt128Decl();
+ break;
case PREDEF_DECL_OBJC_INSTANCETYPE_ID:
- return Context.getObjCInstanceTypeDecl();
+ if (Context.ObjCInstanceTypeDecl)
+ return Context.ObjCInstanceTypeDecl;
+ NewLoaded = Context.getObjCInstanceTypeDecl();
+ break;
case PREDEF_DECL_BUILTIN_VA_LIST_ID:
- return Context.getBuiltinVaListDecl();
+ if (Context.BuiltinVaListDecl)
+ return Context.BuiltinVaListDecl;
+ NewLoaded = Context.getBuiltinVaListDecl();
+ break;
case PREDEF_DECL_VA_LIST_TAG:
- return Context.getVaListTagDecl();
+ if (Context.VaListTagDecl)
+ return Context.VaListTagDecl;
+ NewLoaded = Context.getVaListTagDecl();
+ break;
case PREDEF_DECL_BUILTIN_MS_VA_LIST_ID:
- return Context.getBuiltinMSVaListDecl();
+ if (Context.BuiltinMSVaListDecl)
+ return Context.BuiltinMSVaListDecl;
+ NewLoaded = Context.getBuiltinMSVaListDecl();
+ break;
case PREDEF_DECL_BUILTIN_MS_GUID_ID:
+ // ASTContext::getMSGuidTagDecl won't create MSGuidTagDecl conditionally.
return Context.getMSGuidTagDecl();
case PREDEF_DECL_EXTERN_C_CONTEXT_ID:
- return Context.getExternCContextDecl();
+ if (Context.ExternCContext)
+ return Context.ExternCContext;
+ NewLoaded = Context.getExternCContextDecl();
+ break;
case PREDEF_DECL_MAKE_INTEGER_SEQ_ID:
- return Context.getMakeIntegerSeqDecl();
+ if (Context.MakeIntegerSeqDecl)
+ return Context.MakeIntegerSeqDecl;
+ NewLoaded = Context.getMakeIntegerSeqDecl();
+ break;
case PREDEF_DECL_CF_CONSTANT_STRING_ID:
- return Context.getCFConstantStringDecl();
+ if (Context.CFConstantStringTypeDecl)
+ return Context.CFConstantStringTypeDecl;
+ NewLoaded = Context.getCFConstantStringDecl();
+ break;
case PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID:
- return Context.getCFConstantStringTagDecl();
+ if (Context.CFConstantStringTagDecl)
+ return Context.CFConstantStringTagDecl;
+ NewLoaded = Context.getCFConstantStringTagDecl();
+ break;
case PREDEF_DECL_TYPE_PACK_ELEMENT_ID:
- return Context.getTypePackElementDecl();
+ if (Context.TypePackElementDecl)
+ return Context.TypePackElementDecl;
+ NewLoaded = Context.getTypePackElementDecl();
+ break;
}
- llvm_unreachable("PredefinedDeclIDs unknown enum value");
+
+ assert(NewLoaded && "Failed to load predefined decl?");
+
+ if (DeserializationListener)
+ DeserializationListener->PredefinedDeclBuilt(ID, NewLoaded);
+
+ return NewLoaded;
}
unsigned ASTReader::translateGlobalDeclIDToIndex(GlobalDeclID GlobalID) const {
@@ -7860,7 +7915,7 @@ Decl *ASTReader::GetExistingDecl(GlobalDeclID ID) {
assert(ContextObj && "reading decl with no AST context");
if (ID < NUM_PREDEF_DECL_IDS) {
- Decl *D = getPredefinedDecl(*ContextObj, (PredefinedDeclIDs)ID);
+ Decl *D = getPredefinedDecl((PredefinedDeclIDs)ID);
if (D) {
// Track that we have merged the declaration with ID \p ID into the
// pre-existing predefined declaration \p D.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index b5d487465541b8..1455f8e4145cb8 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6754,6 +6754,12 @@ void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
StoredIdx = Idx;
}
+void ASTWriter::PredefinedDeclBuilt(PredefinedDeclIDs ID, const Decl *D) {
+ assert(D->isCanonicalDecl() && "predefined decl is not canonical");
+ DeclIDs[D] = LocalDeclID(ID);
+ PredefinedDecls.insert(D);
+}
+
void ASTWriter::SelectorRead(SelectorID ID, Selector S) {
// Always keep the highest ID. See \p TypeRead() for more information.
SelectorID &StoredID = SelectorIDs[S];
diff --git a/clang/test/Modules/pr102684.cppm b/clang/test/Modules/pr102684.cppm
new file mode 100644
index 00000000000000..113edbf548a5d6
--- /dev/null
+++ b/clang/test/Modules/pr102684.cppm
@@ -0,0 +1,38 @@
+// 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 %t/b.cppm -emit-module-interface -o %t/b.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/test.cpp -fsyntax-only -verify \
+// RUN: -fprebuilt-module-path=%t
+
+//--- a.cppm
+export module a;
+
+namespace n {
+template<typename, int...>
+struct integer_sequence {
+
+};
+
+export template<typename>
+using make_integer_sequence = __make_integer_seq<integer_sequence, int, 0>;
+}
+
+//--- b.cppm
+export module b;
+import a;
+
+export template<typename T>
+void b() {
+ n::make_integer_sequence<T>();
+}
+
+//--- test.cpp
+// expected-no-diagnostics
+import b;
+void test() {
+ b<int>();
+}
|
You can test this locally with the following command:git-clang-format --diff e607360fcde2994080bb8cec9d4be3a4091fe9a9 b81f9d33e2d96477af8d49f8a174991dcbbc22ef --extensions cpp,cppm,h -- clang/test/Modules/pr102684.cppm clang/include/clang/Frontend/MultiplexConsumer.h clang/include/clang/Serialization/ASTDeserializationListener.h clang/include/clang/Serialization/ASTReader.h clang/include/clang/Serialization/ASTWriter.h clang/lib/Frontend/MultiplexConsumer.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cppView the diff from clang-format here.diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp
index 2158d176d1..8c3effbd00 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -58,7 +58,8 @@ void MultiplexASTDeserializationListener::DeclRead(GlobalDeclID ID,
Listeners[i]->DeclRead(ID, D);
}
-void MultiplexASTDeserializationListener::PredefinedDeclBuilt(PredefinedDeclIDs ID, const Decl *D) {
+void MultiplexASTDeserializationListener::PredefinedDeclBuilt(
+ PredefinedDeclIDs ID, const Decl *D) {
for (size_t i = 0, e = Listeners.size(); i != e; ++i)
Listeners[i]->PredefinedDeclBuilt(ID, D);
}
|
ChuanqiXu9
added
the
skip-precommit-approval
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #102684
The root cause of the issue is, it is possible that the predefined decl is not registered at the beginning of writing a module file but got created during the process of writing from reading.
This is incorrect. The predefined decls should always be predefined decls.
Another deep thought about the issue is, we shouldn't read any new things after we start to write the module file. But this is another deeper question.