Skip to content

Commit

Permalink
[Serialization] Don't try to complete the redeclaration chain in
Browse files Browse the repository at this point in the history
ASTReader after we start writing

This is intended to mitigate
#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.
  • Loading branch information
ChuanqiXu9 committed May 12, 2023
1 parent 5becf54 commit cf47e9f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
3 changes: 3 additions & 0 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions clang/test/Modules/polluted-operator.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,20 @@ module;
export module b;
import a;

void b() {
std::variant<int, double> v;
}

// expected-error@* {{has different definitions in different modules; first difference 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 different 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 different definitions in different modules; first difference 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 different initializer}}

0 comments on commit cf47e9f

Please sign in to comment.