https://github.com/hnrklssn updated
https://github.com/llvm/llvm-project/pull/146468
>From 39fa1a4610cf14d6288abab07f25c7f735c62508 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_ols...@apple.com>
Date: Mon, 30 Jun 2025 21:26:19 -0700
Subject: [PATCH] [Modules] Record side effect info in EvaluatedStmt
All deserialized VarDecl initializers are EvaluatedStmt, but not all
EvaluatedStmt initializers are from a PCH. Calling
`VarDecl::hasInitWithSideEffects` can trigger constant evaluation, but
it's hard to know ahead of time whether that will trigger
deserialization - even if the initializer is fully deserialized, it may
contain a call to a constructor whose body is not deserialized. By
caching the result of `VarDecl::hasInitWithSideEffects` and populating
that cache during deserialization we can guarantee that calling it won't
trigger deserialization regardless of the state of the initializer.
This also reduces memory usage by removing the `InitSideEffectVars` set
in `ASTReader`.
rdar://154717930
---
clang/include/clang/AST/Decl.h | 14 +++--
clang/include/clang/AST/ExternalASTSource.h | 15 ------
.../clang/Sema/MultiplexExternalSemaSource.h | 2 -
clang/include/clang/Serialization/ASTReader.h | 8 ---
clang/lib/AST/Decl.cpp | 27 ++++------
.../lib/Sema/MultiplexExternalSemaSource.cpp | 8 ---
clang/lib/Serialization/ASTReader.cpp | 4 --
clang/lib/Serialization/ASTReaderDecl.cpp | 5 +-
clang/lib/Serialization/ASTWriter.cpp | 4 ++
clang/lib/Serialization/ASTWriterDecl.cpp | 1 -
.../var-init-side-effects-modulemap.cpp | 51 +++++++++++++++++++
11 files changed, 76 insertions(+), 63 deletions(-)
create mode 100644 clang/test/Modules/var-init-side-effects-modulemap.cpp
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index c4202f1f3d07e..f70a039bf3517 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -888,13 +888,17 @@ struct EvaluatedStmt {
bool HasICEInit : 1;
bool CheckedForICEInit : 1;
+ bool HasSideEffects : 1;
+ bool CheckedForSideEffects : 1;
+
LazyDeclStmtPtr Value;
APValue Evaluated;
EvaluatedStmt()
: WasEvaluated(false), IsEvaluating(false),
HasConstantInitialization(false), HasConstantDestruction(false),
- HasICEInit(false), CheckedForICEInit(false) {}
+ HasICEInit(false), CheckedForICEInit(false), HasSideEffects(false),
+ CheckedForSideEffects(false) {}
};
/// Represents a variable declaration or definition.
@@ -1353,9 +1357,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.
+ /// Checks whether this declaration has an initializer with side effects.
+ /// The result is cached. If the result hasn't been computed this can trigger
+ /// deserialization and constant evaluation. By running this during
+ /// serialization and serializing the result all clients can safely call this
+ /// without triggering further deserialization.
bool hasInitWithSideEffects() const;
/// Determine whether this variable's value might be usable in a
diff --git a/clang/include/clang/AST/ExternalASTSource.h
b/clang/include/clang/AST/ExternalASTSource.h
index e91d5132da10f..8fc490b1d8471 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -196,10 +196,6 @@ 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.
///
@@ -434,17 +430,6 @@ 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 7c66c26a17a13..391c2177d75ec 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -94,8 +94,6 @@ 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 7d4b4467eb97d..6e64a435af3de 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1453,12 +1453,6 @@ class ASTReader
const StringRef &operator*() && = delete;
};
- /// 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:
/// Get the buffer for resolving paths.
SmallString<0> &getPathBuf() { return PathBuf; }
@@ -2410,8 +2404,6 @@ 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 5cdf75d71e4d7..a22721d7e5277 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2444,24 +2444,15 @@ 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 {
- E =
cast_or_null<Expr>(getEvaluatedStmt()->Value.getWithoutDeserializing());
- }
-
- 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
- if (auto Source = getASTContext().getExternalSource())
- return Source->hasInitializerWithSideEffects(this);
- return false;
+ EvaluatedStmt *ES = ensureEvaluatedStmt();
+ if (!ES->CheckedForSideEffects) {
+ const Expr *E = getInit();
+ ES->HasSideEffects = E->HasSideEffects(getASTContext()) &&
+ // We can get a value-dependent initializer during error recovery.
+ (E->isValueDependent() || !evaluateValue());
+ ES->CheckedForSideEffects = true;
+ }
+ return ES->HasSideEffects;
}
bool VarDecl::isOutOfLine() const {
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 9f19f13592e86..fbfb242598c24 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -115,14 +115,6 @@ 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/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 486971818f109..156b5bfdaddf6 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9725,10 +9725,6 @@ 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 0ffd78424be0d..8bde213dc4cb3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1628,9 +1628,6 @@ RedeclarableResult
ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VD->NonParmVarDeclBits.PreviousDeclInSameBlockScope =
VarDeclBits.getNextBit();
- if (VarDeclBits.getNextBit())
- Reader.InitSideEffectVars.insert(VD);
-
VD->NonParmVarDeclBits.EscapingByref = VarDeclBits.getNextBit();
HasDeducedType = VarDeclBits.getNextBit();
VD->NonParmVarDeclBits.ImplicitParamKind =
@@ -1701,6 +1698,8 @@ void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) {
Eval->HasConstantInitialization = (Val & 2) != 0;
Eval->HasConstantDestruction = (Val & 4) != 0;
Eval->WasEvaluated = (Val & 8) != 0;
+ Eval->HasSideEffects = (Val & 16) != 0;
+ Eval->CheckedForSideEffects = true;
if (Eval->WasEvaluated) {
Eval->Evaluated = Record.readAPValue();
if (Eval->Evaluated.needsCleanup())
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 06cd6c7305114..dc4b2f475c323 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -7320,6 +7320,10 @@ void ASTRecordWriter::AddVarDeclInit(const VarDecl *VD) {
uint64_t Val = 1;
if (EvaluatedStmt *ES = VD->getEvaluatedStmt()) {
+ // This may trigger evaluation, so run it first
+ if (VD->hasInitWithSideEffects())
+ Val |= 16;
+ assert(ES->CheckedForSideEffects);
Val |= (ES->HasConstantInitialization ? 2 : 0);
Val |= (ES->HasConstantDestruction ? 4 : 0);
APValue *Evaluated = VD->getEvaluatedValue();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index 7f1b39c242e01..a4474bbd37690 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1305,7 +1305,6 @@ 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();
diff --git a/clang/test/Modules/var-init-side-effects-modulemap.cpp
b/clang/test/Modules/var-init-side-effects-modulemap.cpp
new file mode 100644
index 0000000000000..750a6f1731405
--- /dev/null
+++ b/clang/test/Modules/var-init-side-effects-modulemap.cpp
@@ -0,0 +1,51 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodules-cache-path=%t
-fmodule-map-file=%t/module.modulemap %t/test.cppm -I%t
+//
+
+//--- test.cppm
+#pragma clang module import Baz
+
+//--- Foo.h
+#pragma once
+class foo {
+ char dummy = 1;
+
+public:
+ static foo var;
+
+};
+
+inline foo foo::var;
+
+//--- Bar.h
+#pragma once
+#include <Foo.h>
+
+void bar() {
+ (void) foo::var;
+}
+
+//--- Baz.h
+#pragma once
+#include <Foo.h>
+
+void baz() {
+ (void) foo::var;
+}
+
+#include <Bar.h>
+
+//--- module.modulemap
+module Foo {
+ header "Foo.h"
+}
+module Bar {
+ header "Bar.h"
+}
+module Baz {
+ header "Baz.h"
+}
+
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
