[clang] [Serialization] Add a callback to register new created predefined decls for DeserializationListener (PR #102850)

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 12 00:07:12 PDT 2024


https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/102850

>From 598ce1867b5ac36aa6da48be0556bb637ce1ddb0 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Mon, 12 Aug 2024 13:31:35 +0800
Subject: [PATCH] [Serialization] Add a callback to register new created
 predefined decls for DeserializationListener

Close https://github.com/llvm/llvm-project/issues/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.
---
 .../clang/Frontend/MultiplexConsumer.h        |  1 +
 .../ASTDeserializationListener.h              |  2 +
 clang/include/clang/Serialization/ASTReader.h |  3 +
 clang/include/clang/Serialization/ASTWriter.h |  1 +
 clang/lib/Frontend/MultiplexConsumer.cpp      |  5 +
 clang/lib/Serialization/ASTReader.cpp         | 91 +++++++++++++++----
 clang/lib/Serialization/ASTWriter.cpp         |  6 ++
 clang/test/Modules/pr102684.cppm              | 38 ++++++++
 8 files changed, 129 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/Modules/pr102684.cppm

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>();
+}



More information about the cfe-commits mailing list