https://github.com/hnrklssn updated 
https://github.com/llvm/llvm-project/pull/143739

>From 89179c0d4ebcbc5311af73ce293d9297e196b786 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] [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     |  1 +
 clang/test/Modules/var-init-side-effects.cpp  | 37 +++++++++++++++++++
 9 files changed, 88 insertions(+), 3 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..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();
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;
+}
+

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to