[clang] cf47e9f - [Serialization] Don't try to complete the redeclaration chain in

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-05-12T14:28:58+08:00
New Revision: cf47e9fe86aa65b74b0476a5ad4d036dd7463bfb

URL: https://github.com/llvm/llvm-project/commit/cf47e9fe86aa65b74b0476a5ad4d036dd7463bfb
DIFF: https://github.com/llvm/llvm-project/commit/cf47e9fe86aa65b74b0476a5ad4d036dd7463bfb.diff

LOG: [Serialization] Don't try to complete the redeclaration chain in
ASTReader after we start writing

This is intended to mitigate
https://github.com/llvm/llvm-project/issues/61447.

Before the patch, it takes 5s to compile test.cppm in the above
reproducer. After the patch it takes 3s to compile it. Although this
patch didn't solve the problem completely, it should mitigate the
problem for sure. Noted that the behavior of the patch is consistent
with the comment of the originally empty function
ASTReader::finalizeForWriting. So the change should be consistent with
the original design.

Added: 
    

Modified: 
    clang/include/clang/Serialization/ASTReader.h
    clang/lib/Serialization/ASTReader.cpp
    clang/test/Modules/polluted-operator.cppm

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 1360ee1877c1..af01bacbfdc4 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -988,6 +988,9 @@ class ASTReader
   ///Whether we are currently processing update records.
   bool ProcessingUpdateRecords = false;
 
+  /// Whether we are going to write modules.
+  bool FinalizedForWriting = false;
+
   using SwitchCaseMapTy = llvm::DenseMap<unsigned, SwitchCase *>;
 
   /// Mapping from switch-case IDs in the chain to switch-case statements

diff  --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index bdf476cf128a..93409df3d4fc 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5089,7 +5089,8 @@ void ASTReader::InitializeContext() {
 }
 
 void ASTReader::finalizeForWriting() {
-  // Nothing to do for now.
+  assert(!NumCurrentElementsDeserializing && "deserializing when reading");
+  FinalizedForWriting = true;
 }
 
 /// Reads and return the signature record from \p PCH's control block, or
@@ -7328,6 +7329,12 @@ Decl *ASTReader::GetExternalDecl(uint32_t ID) {
 }
 
 void ASTReader::CompleteRedeclChain(const Decl *D) {
+  // We don't need to complete declaration chain after we start writing.
+  // We loses more chances to find ODR violation in the writing place and
+  // we get more efficient writing process.
+  if (FinalizedForWriting)
+    return;
+
   if (NumCurrentElementsDeserializing) {
     // We arrange to not care about the complete redeclaration chain while we're
     // deserializing. Just remember that the AST has marked this one as complete

diff  --git a/clang/test/Modules/polluted-operator.cppm b/clang/test/Modules/polluted-operator.cppm
index b24464aa6ad2..9b45734432db 100644
--- a/clang/test/Modules/polluted-operator.cppm
+++ b/clang/test/Modules/polluted-operator.cppm
@@ -51,7 +51,20 @@ module;
 export module b;
 import a;
 
+void b() {
+  std::variant<int, double> v;
+}
+
 // expected-error@* {{has 
diff erent definitions in 
diff erent modules; first 
diff erence is defined here found data member '_S_copy_ctor' with an initializer}}
 // expected-note@* {{but in 'a.<global>' found data member '_S_copy_ctor' with a 
diff erent initializer}}
 // expected-error@* {{from module 'a.<global>' is not present in definition of 'variant<_Types...>' provided earlier}}
 // expected-note@* {{declaration of 'swap' does not match}}
+
+//--- c.cppm
+module;
+#include "bar.h"
+export module c;
+import a;
+
+// expected-error@* {{has 
diff erent definitions in 
diff erent modules; first 
diff erence is defined here found data member '_S_copy_ctor' with an initializer}}
+// expected-note@* {{but in 'a.<global>' found data member '_S_copy_ctor' with a 
diff erent initializer}}