[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

Author: hnrklssn

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

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.
   ///

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);

clang/lib/AST/Decl.cpp

@@ -2434,6 +2434,21 @@ VarDecl *VarDecl::getInitializingDeclaration() {
   return Def;
 }
 
+bool VarDecl::hasInitWithSideEffects() const {
+  if (!hasInit())
+    return false;
+
+  if (auto *S = dyn_cast<Stmt *>(Init)) {
+    const Expr *E = cast<Expr>(S);
+    return E->HasSideEffects(getASTContext()) &&
+      // We can get a value-dependent initializer during error recovery.
+      (E->isValueDependent() || !evaluateValue());
+  }
+
+  // ASTReader tracks this without having to deserialize the initializer
+  return getASTContext().getExternalSource()->hasInitializerWithSideEffects(this);
+}
+
 bool VarDecl::isOutOfLine() const {
   if (Decl::isOutOfLine())
     return true;

clang/lib/Serialization/ASTReader.cpp

@@ -8310,8 +8310,6 @@ Stmt *ASTReader::GetExternalDeclStmt(uint64_t Offset) {
     Error(std::move(Err));
     return nullptr;
   }
-  assert(NumCurrentElementsDeserializing == 0 &&
-         "should not be called while already deserializing");
   Deserializing D(this);
   return ReadStmtFromStream(*Loc.F);
 }
@@ -9710,6 +9708,9 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) {
 
 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) {

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 =

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();