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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
