Refactor tracking of constant initializers for variables.

Instead of framing the interface around whether the variable is an ICE
(which is only interesting in C++98), primarily track whether the
initializer is a constant initializer (which is interesting in all C++
language modes).

No functionality change intended.

Author: zygoloid

clang/include/clang/AST/Decl.h

@@ -803,14 +803,11 @@ struct EvaluatedStmt {
   /// Whether this statement is being evaluated.
   bool IsEvaluating : 1;
 
-  /// Whether we already checked whether this statement was an
-  /// integral constant expression.
-  bool CheckedICE : 1;
-
-  /// Whether this statement is an integral constant expression,
-  /// or in C++11, whether the statement is a constant expression. Only
-  /// valid if CheckedICE is true.
-  bool IsICE : 1;
+  /// Whether this variable is known to have constant initialization. This is
+  /// currently only computed in C++, for static / thread storage duration
+  /// variables that might have constant initialization and for variables that
+  /// are usable in constant expressions.
+  bool HasConstantInitialization : 1;
 
   /// Whether this variable is known to have constant destruction. That is,
   /// whether running the destructor on the initial value is a side-effect
@@ -819,12 +816,18 @@ struct EvaluatedStmt {
   /// non-trivial.
   bool HasConstantDestruction : 1;
 
+  /// In C++98, whether the initializer is an ICE. This affects whether the
+  /// variable is usable in constant expressions.
+  bool HasICEInit : 1;
+  bool CheckedForICEInit : 1;
+
   Stmt *Value;
   APValue Evaluated;
 
   EvaluatedStmt()
-      : WasEvaluated(false), IsEvaluating(false), CheckedICE(false),
-        IsICE(false), HasConstantDestruction(false) {}
+      : WasEvaluated(false), IsEvaluating(false),
+        HasConstantInitialization(false), HasConstantDestruction(false),
+        HasICEInit(false), CheckedForICEInit(false) {}
 };
 
 /// Represents a variable declaration or definition.
@@ -1284,25 +1287,29 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
   /// Evaluate the destruction of this variable to determine if it constitutes
   /// constant destruction.
   ///
-  /// \pre isInitICE()
+  /// \pre hasConstantInitialization()
   /// \return \c true if this variable has constant destruction, \c false if
   ///         not.
   bool evaluateDestruction(SmallVectorImpl<PartialDiagnosticAt> &Notes) const;
 
-  /// Determines whether it is already known whether the
-  /// initializer is an integral constant expression or not.
-  bool isInitKnownICE() const;
-
-  /// Determines whether the initializer is an integral constant
-  /// expression, or in C++11, whether the initializer is a constant
-  /// expression.
+  /// Determine whether this variable has constant initialization.
   ///
-  /// \pre isInitKnownICE()
-  bool isInitICE() const;
-
-  /// Determine whether the value of the initializer attached to this
-  /// declaration is an integral constant expression.
-  bool checkInitIsICE(SmallVectorImpl<PartialDiagnosticAt> &Notes) const;
+  /// This is only set in two cases: when the language semantics require
+  /// constant initialization (globals in C and some globals in C++), and when
+  /// the variable is usable in constant expressions (constexpr, const int, and
+  /// reference variables in C++).
+  bool hasConstantInitialization() const;
+
+  /// Determine whether the initializer of this variable is an integer constant
+  /// expression. For use in C++98, where this affects whether the variable is
+  /// usable in constant expressions.
+  bool hasICEInitializer(const ASTContext &Context) const;
+
+  /// Evaluate the initializer of this variable to determine whether it's a
+  /// constant initializer. Should only be called once, after completing the
+  /// definition of the variable.
+  bool checkForConstantInitialization(
+      SmallVectorImpl<PartialDiagnosticAt> &Notes) const;
 
   void setInitStyle(InitializationStyle Style) {
     VarDeclBits.InitStyle = Style;

clang/lib/AST/ASTImporter.cpp

@@ -2014,10 +2014,11 @@ Error ASTNodeImporter::ImportInitializer(VarDecl *From, VarDecl *To) {
     return ToInitOrErr.takeError();
 
   To->setInit(*ToInitOrErr);
-  if (From->isInitKnownICE()) {
-    EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
-    Eval->CheckedICE = true;
-    Eval->IsICE = From->isInitICE();
+  if (EvaluatedStmt *FromEval = From->getEvaluatedStmt()) {
+    EvaluatedStmt *ToEval = To->ensureEvaluatedStmt();
+    ToEval->HasConstantInitialization = FromEval->HasConstantInitialization;
+    ToEval->HasConstantDestruction = FromEval->HasConstantDestruction;
+    // FIXME: Also import the initializer value.
   }
 
   // FIXME: Other bits to merge?

clang/lib/AST/Decl.cpp

@@ -2325,7 +2325,16 @@ bool VarDecl::isUsableInConstantExpressions(const ASTContext &Context) const {
   if (!DefVD->mightBeUsableInConstantExpressions(Context))
     return false;
   //   ... and its initializer is a constant initializer.
-  return DefVD->isInitKnownICE() && DefVD->isInitICE();
+  if (!DefVD->hasConstantInitialization())
+    return false;
+  // C++98 [expr.const]p1:
+  //   An integral constant-expression can involve only [...] const variables
+  //   or static data members of integral or enumeration types initialized with
+  //   [integer] constant expressions (dcl.init)
+  if (Context.getLangOpts().CPlusPlus && !Context.getLangOpts().CPlusPlus11 &&
+      !DefVD->hasICEInitializer(Context))
+    return false;
+  return true;
 }
 
 /// Convert the initializer for this declaration to the elaborated EvaluatedStmt
@@ -2399,49 +2408,47 @@ APValue *VarDecl::getEvaluatedValue() const {
   return nullptr;
 }
 
-bool VarDecl::isInitKnownICE() const {
-  if (EvaluatedStmt *Eval = getEvaluatedStmt())
-    return Eval->CheckedICE;
+bool VarDecl::hasICEInitializer(const ASTContext &Context) const {
+  const Expr *Init = getInit();
+  assert(Init && "no initializer");
 
-  return false;
+  EvaluatedStmt *Eval = ensureEvaluatedStmt();
+  if (!Eval->CheckedForICEInit) {
+    Eval->CheckedForICEInit = true;
+    Eval->HasICEInit = Init->isIntegerConstantExpr(Context);
+  }
+  return Eval->HasICEInit;
 }
 
-bool VarDecl::isInitICE() const {
-  assert(isInitKnownICE() &&
-         "Check whether we already know that the initializer is an ICE");
-  return Init.get<EvaluatedStmt *>()->IsICE;
+bool VarDecl::hasConstantInitialization() const {
+  // In C, all globals (and only globals) have constant initialization.
+  if (hasGlobalStorage() && !getASTContext().getLangOpts().CPlusPlus)
+    return true;
+
+  // In C++, it depends on whether the evaluation at the point of definition
+  // was evaluatable as a constant initializer.
+  if (EvaluatedStmt *Eval = getEvaluatedStmt())
+    return Eval->HasConstantInitialization;
+
+  return false;
 }
 
-bool VarDecl::checkInitIsICE(
+bool VarDecl::checkForConstantInitialization(
     SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
   EvaluatedStmt *Eval = ensureEvaluatedStmt();
-  assert(!Eval->CheckedICE &&
-         "should check whether var has constant init at most once");
   // If we ask for the value before we know whether we have a constant
   // initializer, we can compute the wrong value (for example, due to
   // std::is_constant_evaluated()).
   assert(!Eval->WasEvaluated &&
          "already evaluated var value before checking for constant init");
+  assert(getASTContext().getLangOpts().CPlusPlus && "only meaningful in C++");
 
   const auto *Init = cast<Expr>(Eval->Value);
   assert(!Init->isValueDependent());
 
-  // In C++11, evaluate the initializer to check whether it's a constant
-  // expression.
-  if (getASTContext().getLangOpts().CPlusPlus11) {
-    Eval->IsICE = evaluateValue(Notes) && Notes.empty();
-    Eval->CheckedICE = true;
-    return Eval->IsICE;
-  }
-
-  // It's an ICE whether or not the definition we found is
-  // out-of-line.  See DR 721 and the discussion in Clang PR
-  // 6206 for details.
-
-  Eval->IsICE = getType()->isIntegralOrEnumerationType() &&
-                Init->isIntegerConstantExpr(getASTContext());
-  Eval->CheckedICE = true;
-  return Eval->IsICE;
+  // Evaluate the initializer to check whether it's a constant expression.
+  Eval->HasConstantInitialization = evaluateValue(Notes) && Notes.empty();
+  return Eval->HasConstantInitialization;
 }
 
 bool VarDecl::isParameterPack() const {

clang/lib/AST/ExprConstant.cpp

@@ -3281,12 +3281,16 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
   // Check that the variable is actually usable in constant expressions. For a
   // const integral variable or a reference, we might have a non-constant
   // initializer that we can nonetheless evaluate the initializer for. Such
-  // variables are not usable in constant expressions.
+  // variables are not usable in constant expressions. In C++98, the
+  // initializer also syntactically needs to be an ICE.
   //
-  // FIXME: It would be cleaner to check VD->isUsableInConstantExpressions
-  // here, but that regresses diagnostics for things like reading from a
-  // volatile constexpr variable.
-  if (VD->isInitKnownICE() && !VD->isInitICE()) {
+  // FIXME: We don't diagnose cases that aren't potentially usable in constant
+  // expressions here; doing so would regress diagnostics for things like
+  // reading from a volatile constexpr variable.
+  if ((!VD->hasConstantInitialization() &&
+       VD->mightBeUsableInConstantExpressions(Info.Ctx)) ||
+      (Info.getLangOpts().CPlusPlus && !Info.getLangOpts().CPlusPlus11 &&
+       !VD->hasICEInitializer(Info.Ctx))) {
     Info.CCEDiag(E, diag::note_constexpr_var_init_non_constant, 1) << VD;
     NoteLValueLocation(Info, Base);
   }

clang/lib/CodeGen/ItaniumCXXABI.cpp

@@ -365,7 +365,7 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI {
     // variable being constant-initialized in every translation unit if it's
     // constant-initialized in any translation unit, which isn't actually
     // guaranteed by the standard but is necessary for sanity.
-    return InitDecl->isInitKnownICE() && InitDecl->isInitICE();
+    return InitDecl->hasConstantInitialization();
   }
 
   bool usesThreadWrapperFunction(const VarDecl *VD) const override {

clang/lib/Sema/SemaDecl.cpp

@@ -12983,21 +12983,28 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
     // do this lazily, because the result might depend on things that change
     // later, such as which constexpr functions happen to be defined.
     SmallVector<PartialDiagnosticAt, 8> Notes;
-    bool HasConstInit = var->checkInitIsICE(Notes);
-
-    // Prior to C++11, in contexts where a constant initializer is required,
-    // additional kinds of constant expression are permitted beyond ICEs, as
-    // described in [expr.const]p2-6.
-    // FIXME: Stricter checking for these rules would be useful for constinit /
-    // -Wglobal-constructors.
-    if (!getLangOpts().CPlusPlus11 && !HasConstInit) {
+    bool HasConstInit;
+    if (!getLangOpts().CPlusPlus11) {
+      // Prior to C++11, in contexts where a constant initializer is required,
+      // the set of valid constant initializers is described by syntactic rules
+      // in [expr.const]p2-6.
+      // FIXME: Stricter checking for these rules would be useful for constinit /
+      // -Wglobal-constructors.
       HasConstInit = checkConstInit();
-      Notes.clear();
-      if (CacheCulprit) {
+
+      // Compute and cache the constant value, and remember that we have a
+      // constant initializer.
+      if (HasConstInit) {
+        (void)var->checkForConstantInitialization(Notes);
+        Notes.clear();
+      } else if (CacheCulprit) {
         Notes.emplace_back(CacheCulprit->getExprLoc(),
                            PDiag(diag::note_invalid_subexpr_in_const_expr));
         Notes.back().second << CacheCulprit->getSourceRange();
       }
+    } else {
+      // Evaluate the initializer to see if it's a constant initializer.
+      HasConstInit = var->checkForConstantInitialization(Notes);
     }
 
     if (HasConstInit) {

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -1423,10 +1423,9 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
 
   if (uint64_t Val = Record.readInt()) {
     VD->setInit(Record.readExpr());
-    if (Val > 1) {
+    if (Val != 1) {
       EvaluatedStmt *Eval = VD->ensureEvaluatedStmt();
-      Eval->CheckedICE = (Val & 2) != 0;
-      Eval->IsICE = (Val & 3) == 3;
+      Eval->HasConstantInitialization = (Val & 2) != 0;
       Eval->HasConstantDestruction = (Val & 4) != 0;
     }
   }
@@ -4440,8 +4439,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
         VD->setInit(Record.readExpr());
         if (Val != 1) {
           EvaluatedStmt *Eval = VD->ensureEvaluatedStmt();
-          Eval->CheckedICE = (Val & 2) != 0;
-          Eval->IsICE = (Val & 3) == 3;
+          Eval->HasConstantInitialization = (Val & 2) != 0;
           Eval->HasConstantDestruction = (Val & 4) != 0;
         }
       }

clang/lib/Serialization/ASTWriter.cpp

@@ -5747,15 +5747,11 @@ void ASTRecordWriter::AddVarDeclInit(const VarDecl *VD) {
     return;
   }
 
-  // Bottom two bits are as follows:
-  //  01 -- initializer not checked for ICE
-  //  10 -- initializer not ICE
-  //  11 -- initializer ICE
   unsigned Val = 1;
   if (EvaluatedStmt *ES = VD->getEvaluatedStmt()) {
-    if (ES->CheckedICE)
-      Val = 2 | ES->IsICE;
+    Val |= (ES->HasConstantInitialization ? 2 : 0);
     Val |= (ES->HasConstantDestruction ? 4 : 0);
+    // FIXME: Also emit the constant initializer value.
   }
   push_back(Val);
   writeStmtRef(Init);

clang/lib/Serialization/ASTWriterDecl.cpp

@@ -2187,7 +2187,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));                         // ImplicitParamKind
   Abv->Add(BitCodeAbbrevOp(0));                         // EscapingByref
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Linkage
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // IsInitICE (local)
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // HasConstant*
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // VarKind (local enum)
   // Type Source Info
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));