Skip to content

Commit

Permalink
Ensure that checkInitIsICE is called exactly once for every variable
Browse files Browse the repository at this point in the history 
for which it matters.

This is a step towards separating checking for a constant initializer
(in which std::is_constant_evaluated returns true) and any other
evaluation of a variable initializer (in which it returns false).
  • Loading branch information
@zygoloid
zygoloid committed Oct 20, 2020
1 parent a28678e commit 76c0092
Show file tree
Hide file tree
Showing 16 changed files with 180 additions and 193 deletions.
13 changes: 5 additions & 8 deletions clang/include/clang/AST/Decl.h
View file
Expand Up @@ -807,10 +807,6 @@ struct EvaluatedStmt {
/// integral constant expression.
bool CheckedICE : 1;

/// Whether we are checking whether this statement is an
/// integral constant expression.
bool CheckingICE : 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.
Expand All @@ -828,7 +824,7 @@ struct EvaluatedStmt {

EvaluatedStmt()
: WasEvaluated(false), IsEvaluating(false), CheckedICE(false),
CheckingICE(false), IsICE(false), HasConstantDestruction(false) {}
IsICE(false), HasConstantDestruction(false) {}
};

/// Represents a variable declaration or definition.
Expand Down Expand Up @@ -1263,14 +1259,15 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
/// constant expression, according to the relevant language standard.
/// This only checks properties of the declaration, and does not check
/// whether the initializer is in fact a constant expression.
bool mightBeUsableInConstantExpressions(ASTContext &C) const;
bool mightBeUsableInConstantExpressions(const ASTContext &C) const;

/// Determine whether this variable's value can be used in a
/// constant expression, according to the relevant language standard,
/// including checking whether it was initialized by a constant expression.
bool isUsableInConstantExpressions(ASTContext &C) const;
bool isUsableInConstantExpressions(const ASTContext &C) const;

EvaluatedStmt *ensureEvaluatedStmt() const;
EvaluatedStmt *getEvaluatedStmt() const;

/// Attempt to evaluate the value of the initializer attached to this
/// declaration, and produce notes explaining why it cannot be evaluated or is
Expand Down Expand Up @@ -1305,7 +1302,7 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {

/// Determine whether the value of the initializer attached to this
/// declaration is an integral constant expression.
bool checkInitIsICE() const;
bool checkInitIsICE(SmallVectorImpl<PartialDiagnosticAt> &Notes) const;

void setInitStyle(InitializationStyle Style) {
VarDeclBits.InitStyle = Style;
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Serialization/ASTRecordWriter.h
View file
Expand Up @@ -266,6 +266,9 @@ class ASTRecordWriter

void AddCXXDefinitionData(const CXXRecordDecl *D);

/// Emit information about the initializer of a VarDecl.
void AddVarDeclInit(const VarDecl *VD);

/// Write an OMPTraitInfo object.
void writeOMPTraitInfo(const OMPTraitInfo *TI);

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ComparisonCategories.cpp
View file
Expand Up @@ -42,7 +42,7 @@ clang::getComparisonCategoryForBuiltinCmp(QualType T) {

bool ComparisonCategoryInfo::ValueInfo::hasValidIntValue() const {
assert(VD && "must have var decl");
if (!VD->checkInitIsICE())
if (!VD->isUsableInConstantExpressions(VD->getASTContext()))
return false;

// Before we attempt to get the value of the first field, ensure that we
Expand Down
57 changes: 26 additions & 31 deletions clang/lib/AST/Decl.cpp
View file
Expand Up @@ -2277,7 +2277,7 @@ void VarDecl::setInit(Expr *I) {
Init = I;
}

bool VarDecl::mightBeUsableInConstantExpressions(ASTContext &C) const {
bool VarDecl::mightBeUsableInConstantExpressions(const ASTContext &C) const {
const LangOptions &Lang = C.getLangOpts();

if (!Lang.CPlusPlus)
Expand Down Expand Up @@ -2312,7 +2312,7 @@ bool VarDecl::mightBeUsableInConstantExpressions(ASTContext &C) const {
return Lang.CPlusPlus11 && isConstexpr();
}

bool VarDecl::isUsableInConstantExpressions(ASTContext &Context) const {
bool VarDecl::isUsableInConstantExpressions(const ASTContext &Context) const {
// C++2a [expr.const]p3:
// A variable is usable in constant expressions after its initializing
// declaration is encountered...
Expand All @@ -2325,7 +2325,7 @@ bool VarDecl::isUsableInConstantExpressions(ASTContext &Context) const {
if (!DefVD->mightBeUsableInConstantExpressions(Context))
return false;
// ... and its initializer is a constant initializer.
return DefVD->checkInitIsICE();
return DefVD->isInitKnownICE() && DefVD->isInitICE();
}

/// Convert the initializer for this declaration to the elaborated EvaluatedStmt
Expand All @@ -2345,6 +2345,10 @@ EvaluatedStmt *VarDecl::ensureEvaluatedStmt() const {
return Eval;
}

EvaluatedStmt *VarDecl::getEvaluatedStmt() const {
return Init.dyn_cast<EvaluatedStmt *>();
}

APValue *VarDecl::evaluateValue() const {
SmallVector<PartialDiagnosticAt, 8> Notes;
return evaluateValue(Notes);
Expand All @@ -2354,19 +2358,17 @@ APValue *VarDecl::evaluateValue(
SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
EvaluatedStmt *Eval = ensureEvaluatedStmt();

const auto *Init = cast<Expr>(Eval->Value);
assert(!Init->isValueDependent());

// We only produce notes indicating why an initializer is non-constant the
// first time it is evaluated. FIXME: The notes won't always be emitted the
// first time we try evaluation, so might not be produced at all.
if (Eval->WasEvaluated)
return Eval->Evaluated.isAbsent() ? nullptr : &Eval->Evaluated;

const auto *Init = cast<Expr>(Eval->Value);
assert(!Init->isValueDependent());

if (Eval->IsEvaluating) {
// FIXME: Produce a diagnostic for self-initialization.
Eval->CheckedICE = true;
Eval->IsICE = false;
return nullptr;
}

Expand All @@ -2386,26 +2388,19 @@ APValue *VarDecl::evaluateValue(
Eval->IsEvaluating = false;
Eval->WasEvaluated = true;

// In C++11, we have determined whether the initializer was a constant
// expression as a side-effect.
if (getASTContext().getLangOpts().CPlusPlus11 && !Eval->CheckedICE) {
Eval->CheckedICE = true;
Eval->IsICE = Result && Notes.empty();
}

return Result ? &Eval->Evaluated : nullptr;
}

APValue *VarDecl::getEvaluatedValue() const {
if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
if (EvaluatedStmt *Eval = getEvaluatedStmt())
if (Eval->WasEvaluated)
return &Eval->Evaluated;

return nullptr;
}

bool VarDecl::isInitKnownICE() const {
if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
if (EvaluatedStmt *Eval = getEvaluatedStmt())
return Eval->CheckedICE;

return false;
Expand All @@ -2417,34 +2412,34 @@ bool VarDecl::isInitICE() const {
return Init.get<EvaluatedStmt *>()->IsICE;
}

bool VarDecl::checkInitIsICE() const {
bool VarDecl::checkInitIsICE(
SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
EvaluatedStmt *Eval = ensureEvaluatedStmt();
if (Eval->CheckedICE)
// We have already checked whether this subexpression is an
// integral constant expression.
return Eval->IsICE;
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");

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) {
SmallVector<PartialDiagnosticAt, 8> Notes;
evaluateValue(Notes);
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.

if (Eval->CheckingICE)
return false;
Eval->CheckingICE = true;

Eval->IsICE = Init->isIntegerConstantExpr(getASTContext());
Eval->CheckingICE = false;
Eval->IsICE = getType()->isIntegralOrEnumerationType() &&
Init->isIntegerConstantExpr(getASTContext());
Eval->CheckedICE = true;
return Eval->IsICE;
}
Expand Down Expand Up @@ -2599,7 +2594,7 @@ bool VarDecl::isNoDestroy(const ASTContext &Ctx) const {

QualType::DestructionKind
VarDecl::needsDestruction(const ASTContext &Ctx) const {
if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
if (EvaluatedStmt *Eval = getEvaluatedStmt())
if (Eval->HasConstantDestruction)
return QualType::DK_none;

Expand Down
50 changes: 19 additions & 31 deletions clang/lib/AST/ExprConstant.cpp
View file
Expand Up @@ -3278,12 +3278,17 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
return false;
}

// Check that the variable is actually usable in constant expressions.
if (!VD->checkInitIsICE()) {
Info.CCEDiag(E, diag::note_constexpr_var_init_non_constant,
Notes.size() + 1) << VD;
// 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.
//
// 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()) {
Info.CCEDiag(E, diag::note_constexpr_var_init_non_constant, 1) << VD;
NoteLValueLocation(Info, Base);
Info.addNotes(Notes);
}

// Never use the initializer of a weak variable, not even for constant
Expand All @@ -3298,11 +3303,6 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
return true;
}

static bool IsConstNonVolatile(QualType T) {
Qualifiers Quals = T.getQualifiers();
return Quals.hasConst() && !Quals.hasVolatile();
}

/// Get the base index of the given base class within an APValue representing
/// the given derived class.
static unsigned getBaseIndex(const CXXRecordDecl *Derived,
Expand Down Expand Up @@ -8114,6 +8114,12 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
return Success(VD);
}

if (!Info.getLangOpts().CPlusPlus11) {
Info.CCEDiag(E, diag::note_constexpr_ltor_non_integral, 1)
<< VD << VD->getType();
Info.Note(VD->getLocation(), diag::note_declared_at);
}

APValue *V;
if (!evaluateVarDeclInit(Info, E, VD, Frame, Version, V))
return false;
Expand Down Expand Up @@ -15030,30 +15036,12 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
case Expr::DeclRefExprClass: {
if (isa<EnumConstantDecl>(cast<DeclRefExpr>(E)->getDecl()))
return NoDiag();
const ValueDecl *D = cast<DeclRefExpr>(E)->getDecl();
if (Ctx.getLangOpts().CPlusPlus &&
D && IsConstNonVolatile(D->getType())) {
// Parameter variables are never constants. Without this check,
// getAnyInitializer() can find a default argument, which leads
// to chaos.
if (isa<ParmVarDecl>(D))
return ICEDiag(IK_NotICE, cast<DeclRefExpr>(E)->getLocation());

const VarDecl *VD = dyn_cast<VarDecl>(cast<DeclRefExpr>(E)->getDecl());
if (VD && VD->isUsableInConstantExpressions(Ctx)) {
// C++ 7.1.5.1p2
// A variable of non-volatile const-qualified integral or enumeration
// type initialized by an ICE can be used in ICEs.
if (const VarDecl *Dcl = dyn_cast<VarDecl>(D)) {
if (!Dcl->getType()->isIntegralOrEnumerationType())
return ICEDiag(IK_NotICE, cast<DeclRefExpr>(E)->getLocation());

const VarDecl *VD;
// Look for a declaration of this variable that has an initializer, and
// check whether it is an ICE.
if (Dcl->getAnyInitializer(VD) && !VD->isWeak() && VD->checkInitIsICE())
return NoDiag();
else
return ICEDiag(IK_NotICE, cast<DeclRefExpr>(E)->getLocation());
}
return NoDiag();
}
return ICEDiag(IK_NotICE, E->getBeginLoc());
}
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/CodeGen/ItaniumCXXABI.cpp
View file
Expand Up @@ -361,8 +361,9 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI {
return !VD->needsDestruction(getContext()) && InitDecl->evaluateValue();

// Otherwise, we need a thread wrapper unless we know that every
// translation unit will emit the value as a constant. We rely on
// ICE-ness not varying between translation units, which isn't actually
// translation unit will emit the value as a constant. We rely on the
// 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();
}
Expand Down

0 comments on commit 76c0092

Please sign in to comment.