https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/143739
>From eacaac0ce4177a45183e1f8c878bc2085b443983 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_ols...@apple.com>
Date: Wed, 11 Jun 2025 17:36:29 +0200
Subject: [PATCH 1/2] [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/ASTContext.cpp | 4 +-
clang/lib/AST/Decl.cpp | 25 +++++++++++++
clang/lib/Serialization/ASTReader.cpp | 4 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 4 ++
clang/lib/Serialization/ASTWriterDecl.cpp | 14 ++++---
clang/test/Modules/var-init-side-effects.cpp | 37 +++++++++++++++++++
9 files changed, 95 insertions(+), 9 deletions(-)
create mode 100644 clang/test/Modules/var-init-side-effects.cpp
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..bbbf3a9c369c9 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/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 2836d68b05ff6..16530c560a28b 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12889,9 +12889,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
return true;
// Variables that have initialization with side-effects are required.
- if (VD->getInit() && VD->getInit()->HasSideEffects(*this) &&
- // We can get a value-dependent initializer during error recovery.
- (VD->getInit()->isValueDependent() || !VD->evaluateValue()))
+ if (VD->hasInitWithSideEffects())
return true;
// Likewise, variables with tuple-like bindings are required if their
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 1d9208f0e1c72..93c8abcb894e1 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2434,6 +2434,31 @@ VarDecl *VarDecl::getInitializingDeclaration() {
return Def;
}
+bool VarDecl::hasInitWithSideEffects() const {
+ if (!hasInit())
+ return false;
+
+ // Check if we can get the initializer without deserializing
+ const Expr *E = nullptr;
+ if (auto *S = dyn_cast<Stmt *>(Init)) {
+ E = cast<Expr>(S);
+ } else {
+ auto *Eval = getEvaluatedStmt();
+ if (!Eval->Value.isOffset())
+ E = cast<Expr>(Eval->Value.get(nullptr));
+ }
+
+ if (E)
+ return E->HasSideEffects(getASTContext()) &&
+ // We can get a value-dependent initializer during error recovery.
+ (E->isValueDependent() || !evaluateValue());
+
+ assert(getEvaluatedStmt()->Value.isOffset());
+ // 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..09bb75bc27574 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9712,6 +9712,10 @@ 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) {
return DecodeSelector(getGlobalSelectorID(M, LocalID));
}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 5545cbc8d608c..ec4b2bebf4af5 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..f8dfa07541d88 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();
@@ -1308,10 +1309,11 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
!D->hasExtInfo() && D->getFirstDecl() == D->getMostRecentDecl() &&
D->getKind() == Decl::Var && !D->isInline() && !D->isConstexpr() &&
!D->isInitCapture() && !D->isPreviousDeclInSameBlockScope() &&
- !D->isEscapingByref() && !HasDeducedType &&
- D->getStorageDuration() != SD_Static && !D->getDescribedVarTemplate() &&
- !D->getMemberSpecializationInfo() && !D->isObjCForDecl() &&
- !isa<ImplicitParamDecl>(D) && !D->isEscapingByref())
+ !D->hasInitWithSideEffects() && !D->isEscapingByref() &&
+ !HasDeducedType && D->getStorageDuration() != SD_Static &&
+ !D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+ !D->isObjCForDecl() && !isa<ImplicitParamDecl>(D) &&
+ !D->isEscapingByref())
AbbrevToUse = Writer.getDeclVarAbbrev();
Code = serialization::DECL_VAR;
@@ -2693,12 +2695,12 @@ void ASTWriter::WriteDeclAbbrevs() {
// VarDecl
Abv->Add(BitCodeAbbrevOp(
BitCodeAbbrevOp::Fixed,
- 21)); // Packed Var Decl bits: Linkage, ModulesCodegen,
+ 22)); // Packed Var Decl bits: Linkage, ModulesCodegen,
// SClass, TSCSpec, InitStyle,
// isARCPseudoStrong, IsThisDeclarationADemotedDefinition,
// isExceptionVariable, isNRVOVariable, isCXXForRangeDecl,
// isInline, isInlineSpecified, isConstexpr,
- // isInitCapture, isPrevDeclInSameScope,
+ // isInitCapture, isPrevDeclInSameScope, hasInitWithSideEffects,
// EscapingByref, HasDeducedType, ImplicitParamKind, isObjCForDecl
Abv->Add(BitCodeAbbrevOp(0)); // VarKind (local enum)
// Type Source Info
diff --git a/clang/test/Modules/var-init-side-effects.cpp
b/clang/test/Modules/var-init-side-effects.cpp
new file mode 100644
index 0000000000000..438a9eb204275
--- /dev/null
+++ b/clang/test/Modules/var-init-side-effects.cpp
@@ -0,0 +1,37 @@
+// Tests referencing variable with initializer containing side effect across
module boundary
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/Foo.cppm \
+// RUN: -o %t/Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface \
+// RUN: -fmodule-file=Foo=%t/Foo.pcm \
+// RUN: %t/Bar.cppm \
+// RUN: -o %t/Bar.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj \
+// RUN: -main-file-name Bar.cppm \
+// RUN: -fmodule-file=Foo=%t/Foo.pcm \
+// RUN: -x pcm %t/Bar.pcm \
+// RUN: -o %t/Bar.o
+
+//--- Foo.cppm
+export module Foo;
+
+export {
+class S {};
+
+inline S s = S{};
+}
+
+//--- Bar.cppm
+export module Bar;
+import Foo;
+
+S bar() {
+ return s;
+}
+
>From 5e09306fc47e26c8aeea5330e3112568d1a2eec3 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_ols...@apple.com>
Date: Fri, 20 Jun 2025 12:56:12 +0200
Subject: [PATCH 2/2] address review comments
---
clang/include/clang/AST/ExternalASTSource.h | 11 +++++++++++
.../include/clang/Sema/MultiplexExternalSemaSource.h | 2 ++
clang/include/clang/Serialization/ASTReader.h | 2 ++
clang/lib/AST/Decl.cpp | 9 ++++-----
clang/lib/Sema/MultiplexExternalSemaSource.cpp | 8 ++++++++
clang/lib/Serialization/ASTReaderDecl.cpp | 3 +--
6 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/AST/ExternalASTSource.h
b/clang/include/clang/AST/ExternalASTSource.h
index 4d0812b158a19..e91d5132da10f 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -434,6 +434,17 @@ struct LazyOffsetPtr {
return GetPtr();
}
+ /// Retrieve the pointer to the AST node that this lazy pointer points to,
+ /// if it can be done without triggering deserialization.
+ ///
+ /// \returns a pointer to the AST node, or null if not yet deserialized.
+ T *getWithoutDeserializing() const {
+ if (isOffset()) {
+ return nullptr;
+ }
+ return GetPtr();
+ }
+
/// Retrieve the address of the AST node pointer. Deserializes the pointee if
/// necessary.
T **getAddressOfPointer(ExternalASTSource *Source) const {
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 391c2177d75ec..7c66c26a17a13 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -94,6 +94,8 @@ class MultiplexExternalSemaSource : public ExternalSemaSource
{
bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;
+ bool hasInitializerWithSideEffects(const VarDecl *VD) const override;
+
/// Find all declarations with the given name in the
/// given context.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index bbbf3a9c369c9..ff56065085a1f 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1444,6 +1444,8 @@ class ASTReader
/// VarDecls with initializers containing side effects must be emitted,
/// but DeclMustBeEmitted is not allowed to deserialize the intializer.
+ /// FIXME: Lower memory usage by removing VarDecls once the initializer
+ /// is deserialized.
llvm::SmallPtrSet<Decl *, 16> InitSideEffectVars;
public:
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 93c8abcb894e1..8d5724a4d203e 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2443,9 +2443,7 @@ bool VarDecl::hasInitWithSideEffects() const {
if (auto *S = dyn_cast<Stmt *>(Init)) {
E = cast<Expr>(S);
} else {
- auto *Eval = getEvaluatedStmt();
- if (!Eval->Value.isOffset())
- E = cast<Expr>(Eval->Value.get(nullptr));
+ E =
cast_or_null<Expr>(getEvaluatedStmt()->Value.getWithoutDeserializing());
}
if (E)
@@ -2455,8 +2453,9 @@ bool VarDecl::hasInitWithSideEffects() const {
assert(getEvaluatedStmt()->Value.isOffset());
// ASTReader tracks this without having to deserialize the initializer
- return getASTContext().getExternalSource()->hasInitializerWithSideEffects(
- this);
+ if (auto Source = getASTContext().getExternalSource())
+ return Source->hasInitializerWithSideEffects(this);
+ return false;
}
bool VarDecl::isOutOfLine() const {
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index fbfb242598c24..9f19f13592e86 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -115,6 +115,14 @@ bool
MultiplexExternalSemaSource::wasThisDeclarationADefinition(
return false;
}
+bool MultiplexExternalSemaSource::hasInitializerWithSideEffects(
+ const VarDecl *VD) const {
+ for (const auto &S : Sources)
+ if (S->hasInitializerWithSideEffects(VD))
+ return true;
+ return false;
+}
+
bool MultiplexExternalSemaSource::FindExternalVisibleDeclsByName(
const DeclContext *DC, DeclarationName Name,
const DeclContext *OriginalDC) {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index ec4b2bebf4af5..68f8e86c6facd 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1632,8 +1632,7 @@ RedeclarableResult
ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VD->NonParmVarDeclBits.PreviousDeclInSameBlockScope =
VarDeclBits.getNextBit();
- bool HasInitWithSideEffect = VarDeclBits.getNextBit();
- if (HasInitWithSideEffect)
+ if (VarDeclBits.getNextBit())
Reader.InitSideEffectVars.insert(VD);
VD->NonParmVarDeclBits.EscapingByref = VarDeclBits.getNextBit();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
