Author: Richard Smith Date: 2020-10-19T21:31:19-07:00 New Revision: 3692d20d2b994ce865ffb97096b05218279e1ebd
URL: https://github.com/llvm/llvm-project/commit/3692d20d2b994ce865ffb97096b05218279e1ebd DIFF: https://github.com/llvm/llvm-project/commit/3692d20d2b994ce865ffb97096b05218279e1ebd.diff LOG: 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. Added: Modified: clang/include/clang/AST/Decl.h clang/lib/AST/ASTImporter.cpp clang/lib/AST/Decl.cpp clang/lib/AST/ExprConstant.cpp clang/lib/CodeGen/ItaniumCXXABI.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/ASTWriterDecl.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index e309819400f1..fe906fc8c6d6 100644 --- a/clang/include/clang/AST/Decl.h +++ b/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; diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 9efbcf757e99..5a70d0ee9590 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/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? diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index ee7f51c5218e..5331ed8f0a9e 100644 --- a/clang/lib/AST/Decl.cpp +++ b/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 { diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 3014f948f9b1..34893302f2e7 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/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); } diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 40cd5c54185f..8ab80e66dec3 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/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 { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 1a27667fc106..283d663c5d26 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/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) { diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 41f2db1ef5f0..4d50e4af31f7 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/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; } } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 6056ed623c69..ed00a3bc6281 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/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); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 8778f0c02671..5b0a6ffb9566 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/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)); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits