[clang] [ASTReader] Remove assert in GetExternalDeclStmt (PR #143739)
Henrik G. Olsson via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 16 07:24:56 PDT 2025
https://github.com/hnrklssn updated https://github.com/llvm/llvm-project/pull/143739
>From d239fee7d5fd002edb08a24b4de1db84433413fe Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Wed, 11 Jun 2025 17:36:29 +0200
Subject: [PATCH] [Modules] Record whether VarDecl initializers contain side
effects
Calling `DeclMustBeEmitted` should not lead to more deserialization, as
it may occur before previous deserialization has finished.
When passed a `VarDecl` with an initializer however, `DeclMustBeEmitted`
needs to know whether that initializer contains side effects. When the
`VarDecl` is deserialized but the initializer is not, this triggers
deserialization of the initializer. To avoid this we add a bit to the
serialization format for `VarDecl`s, indicating whether its initializer
contains side effects or not, so that the `ASTReader` can query this
information directly without deserializing the initializer.
rdar://153085264
---
clang/include/clang/AST/Decl.h | 5 +++++
clang/include/clang/AST/ExternalASTSource.h | 5 +++++
clang/include/clang/Serialization/ASTReader.h | 6 ++++++
clang/lib/AST/Decl.cpp | 15 +++++++++++++++
clang/lib/Serialization/ASTReader.cpp | 5 +++--
clang/lib/Serialization/ASTReaderDecl.cpp | 4 ++++
clang/lib/Serialization/ASTWriterDecl.cpp | 1 +
7 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 3faf63e395a08..008c97c18d63a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -1351,6 +1351,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
return const_cast<VarDecl *>(this)->getInitializingDeclaration();
}
+ /// Checks whether this declaration has an initializer with side effects,
+ /// without triggering deserialization if the initializer is not yet
+ /// deserialized.
+ bool hasInitWithSideEffects() const;
+
/// Determine whether this variable's value might be usable in a
/// constant expression, according to the relevant language standard.
/// This only checks properties of the declaration, and does not check
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index f45e3af7602c1..4d0812b158a19 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -51,6 +51,7 @@ class RecordDecl;
class Selector;
class Stmt;
class TagDecl;
+class VarDecl;
/// Abstract interface for external sources of AST nodes.
///
@@ -195,6 +196,10 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
/// module.
virtual bool wasThisDeclarationADefinition(const FunctionDecl *FD);
+ virtual bool hasInitializerWithSideEffects(const VarDecl *VD) const {
+ return false;
+ }
+
/// Finds all declarations lexically contained within the given
/// DeclContext, after applying an optional filter predicate.
///
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 2765c827ece2b..6e76097c6fae8 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1442,6 +1442,10 @@ class ASTReader
const StringRef &operator*() && = delete;
};
+ /// VarDecls with initializers containing side effects must be emitted,
+ /// but DeclMustBeEmitted is not allowed to deserialize the intializer.
+ llvm::SmallPtrSet<Decl*, 16> InitSideEffectVars;
+
public:
/// Get the buffer for resolving paths.
SmallString<0> &getPathBuf() { return PathBuf; }
@@ -2392,6 +2396,8 @@ class ASTReader
bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;
+ bool hasInitializerWithSideEffects(const VarDecl *VD) const override;
+
/// Retrieve a selector from the given module with its local ID
/// number.
Selector getLocalSelector(ModuleFile &M, unsigned LocalID);
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 1d9208f0e1c72..1444135a59a2e 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2434,6 +2434,21 @@ VarDecl *VarDecl::getInitializingDeclaration() {
return Def;
}
+bool VarDecl::hasInitWithSideEffects() const {
+ if (!hasInit())
+ return false;
+
+ if (auto *S = dyn_cast<Stmt *>(Init)) {
+ const Expr *E = cast<Expr>(S);
+ return E->HasSideEffects(getASTContext()) &&
+ // We can get a value-dependent initializer during error recovery.
+ (E->isValueDependent() || !evaluateValue());
+ }
+
+ // ASTReader tracks this without having to deserialize the initializer
+ return getASTContext().getExternalSource()->hasInitializerWithSideEffects(this);
+}
+
bool VarDecl::isOutOfLine() const {
if (Decl::isOutOfLine())
return true;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 70b54b7296882..00dec12a1daa4 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8310,8 +8310,6 @@ Stmt *ASTReader::GetExternalDeclStmt(uint64_t Offset) {
Error(std::move(Err));
return nullptr;
}
- assert(NumCurrentElementsDeserializing == 0 &&
- "should not be called while already deserializing");
Deserializing D(this);
return ReadStmtFromStream(*Loc.F);
}
@@ -9710,6 +9708,9 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) {
bool ASTReader::wasThisDeclarationADefinition(const FunctionDecl *FD) {
return ThisDeclarationWasADefinitionSet.contains(FD);
+
+bool ASTReader::hasInitializerWithSideEffects(const VarDecl *VD) const {
+ return InitSideEffectVars.count(VD);
}
Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 5545cbc8d608c..1ad37728e907f 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1632,6 +1632,10 @@ RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VD->NonParmVarDeclBits.PreviousDeclInSameBlockScope =
VarDeclBits.getNextBit();
+ bool hasInitWithSideEffect = VarDeclBits.getNextBit();
+ if (hasInitWithSideEffect)
+ Reader.InitSideEffectVars.insert(VD);
+
VD->NonParmVarDeclBits.EscapingByref = VarDeclBits.getNextBit();
HasDeducedType = VarDeclBits.getNextBit();
VD->NonParmVarDeclBits.ImplicitParamKind =
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 3a7a23481ea98..876131db461df 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1259,6 +1259,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
VarDeclBits.addBit(D->isConstexpr());
VarDeclBits.addBit(D->isInitCapture());
VarDeclBits.addBit(D->isPreviousDeclInSameBlockScope());
+ VarDeclBits.addBit(D->hasInitWithSideEffects());
VarDeclBits.addBit(D->isEscapingByref());
HasDeducedType = D->getType()->getContainedDeducedType();
More information about the cfe-commits
mailing list