[Modules] Record side effect info in EvaluatedStmt (#146468)

hnrklssn · web-flow · commit 2910c24638fc · 2025-07-03T15:37:55.000-07:00
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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -2444,24 +2444,16 @@ 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
@@ -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
@@ -9733,10 +9733,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
@@ -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
@@ -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
@@ -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
@@ -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"
+}
+

Original file line number	Diff line number	Diff line change
`@@ -9733,10 +9733,6 @@ bool ASTReader::wasThisDeclarationADefinition(const FunctionDecl *FD) {`
`9733`	`9733`	`return ThisDeclarationWasADefinitionSet.contains(FD);`
`9734`	`9734`	`}`
`9735`	`9735`
`9736`		`-bool ASTReader::hasInitializerWithSideEffects(const VarDecl *VD) const {`
`9737`		`- return InitSideEffectVars.count(VD);`
`9738`		`-}`
`9739`		`-`
`9740`	`9736`	`Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {`
`9741`	`9737`	`return DecodeSelector(getGlobalSelectorID(M, LocalID));`
`9742`	`9738`	`}`