Revert "[Modules] Record whether VarDecl initializers contain side effects"

[JDevlieghere]

Reverts #143739 because it triggers an assert:

Assertion failed: (!isNull() && "Cannot retrieve a NULL type pointer"), function getCommonPtr, file Type.h, line 952.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Jonas Devlieghere (JDevlieghere)

Changes

Reverts llvm/llvm-project#143739

---

Full diff: https://github.com/llvm/llvm-project/pull/145407.diff

11 Files Affected:

• (modified) clang/include/clang/AST/Decl.h (-5)
• (modified) clang/include/clang/AST/ExternalASTSource.h (-16)
• (modified) clang/include/clang/Sema/MultiplexExternalSemaSource.h (-2)
• (modified) clang/include/clang/Serialization/ASTReader.h (-8)
• (modified) clang/lib/AST/ASTContext.cpp (+3-1)
• (modified) clang/lib/AST/Decl.cpp (-24)
• (modified) clang/lib/Sema/MultiplexExternalSemaSource.cpp (-8)
• (modified) clang/lib/Serialization/ASTReader.cpp (-4)
• (modified) clang/lib/Serialization/ASTReaderDecl.cpp (-3)
• (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+6-8)
• (removed) clang/test/Modules/var-init-side-effects.cpp (-37)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 0da940883b6f5..58209f4601422 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -1352,11 +1352,6 @@ 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 e91d5132da10f..f45e3af7602c1 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -51,7 +51,6 @@ class RecordDecl;
 class Selector;
 class Stmt;
 class TagDecl;
-class VarDecl;
 
 /// Abstract interface for external sources of AST nodes.
 ///
@@ -196,10 +195,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 +429,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 7a4b7d21bb20e..be1c6e0817593 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1455,12 +1455,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; }
@@ -2412,8 +2406,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/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index eba3c3de3d092..e851e8c3d8143 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13110,7 +13110,9 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
     return true;
 
   // Variables that have initialization with side-effects are required.
-  if (VD->hasInitWithSideEffects())
+  if (VD->getInit() && VD->getInit()->HasSideEffects(*this) &&
+      // We can get a value-dependent initializer during error recovery.
+      (VD->getInit()->isValueDependent() || !VD->evaluateValue()))
     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 35c41859595da..2f01c715ae4ba 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2441,30 +2441,6 @@ 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 {
-    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;
-}
-
 bool VarDecl::isOutOfLine() const {
   if (Decl::isOutOfLine())
     return true;
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 6f082fe840b4c..a3fbc3d25acab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9722,10 +9722,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..7f7882654b9d1 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 =
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 2e390dbe79ec6..2d93832a9ac31 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1306,7 +1306,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();
@@ -1356,11 +1355,10 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
       !D->hasExtInfo() && D->getFirstDecl() == D->getMostRecentDecl() &&
       D->getKind() == Decl::Var && !D->isInline() && !D->isConstexpr() &&
       !D->isInitCapture() && !D->isPreviousDeclInSameBlockScope() &&
-      !D->hasInitWithSideEffects() && !D->isEscapingByref() &&
-      !HasDeducedType && D->getStorageDuration() != SD_Static &&
-      !D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
-      !D->isObjCForDecl() && !isa<ImplicitParamDecl>(D) &&
-      !D->isEscapingByref())
+      !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;
@@ -2733,12 +2731,12 @@ void ASTWriter::WriteDeclAbbrevs() {
   // VarDecl
   Abv->Add(BitCodeAbbrevOp(
       BitCodeAbbrevOp::Fixed,
-      22)); // Packed Var Decl bits:  Linkage, ModulesCodegen,
+      21)); // Packed Var Decl bits:  Linkage, ModulesCodegen,
             // SClass, TSCSpec, InitStyle,
             // isARCPseudoStrong, IsThisDeclarationADemotedDefinition,
             // isExceptionVariable, isNRVOVariable, isCXXForRangeDecl,
             // isInline, isInlineSpecified, isConstexpr,
-            // isInitCapture, isPrevDeclInSameScope, hasInitWithSideEffects,
+            // isInitCapture, isPrevDeclInSameScope,
             // 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
deleted file mode 100644
index 438a9eb204275..0000000000000
--- a/clang/test/Modules/var-init-side-effects.cpp
+++ /dev/null
@@ -1,37 +0,0 @@
-// 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;
-}
-