Skip to content

Commit

Permalink
For P0784R7: compute whether a variable has constant destruction if it
Browse files Browse the repository at this point in the history
has a constexpr destructor.

For constexpr variables, reject if the variable does not have constant
destruction. In all cases, do not emit runtime calls to the destructor
for variables with constant destruction.

llvm-svn: 373159
  • Loading branch information
zygoloid committed Sep 29, 2019
1 parent ac59699 commit 2b4fa53
Show file tree
Hide file tree
Showing 23 changed files with 325 additions and 75 deletions.
28 changes: 24 additions & 4 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -808,12 +808,19 @@ struct EvaluatedStmt {
/// valid if CheckedICE is true.
bool IsICE : 1;

/// Whether this variable is known to have constant destruction. That is,
/// whether running the destructor on the initial value is a side-effect
/// (and doesn't inspect any state that might have changed during program
/// execution). This is currently only computed if the destructor is
/// non-trivial.
bool HasConstantDestruction : 1;

Stmt *Value;
APValue Evaluated;

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

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

/// Represents a variable declaration or definition.
Expand Down Expand Up @@ -1267,6 +1274,14 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
/// to untyped APValue if the value could not be evaluated.
APValue *getEvaluatedValue() const;

/// Evaluate the destruction of this variable to determine if it constitutes
/// constant destruction.
///
/// \pre isInitICE()
/// \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;
Expand Down Expand Up @@ -1505,9 +1520,14 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
// has no definition within this source file.
bool isKnownToBeDefined() const;

/// Do we need to emit an exit-time destructor for this variable?
/// Is destruction of this variable entirely suppressed? If so, the variable
/// need not have a usable destructor at all.
bool isNoDestroy(const ASTContext &) const;

/// Do we need to emit an exit-time destructor for this variable, and if so,
/// what kind?
QualType::DestructionKind needsDestruction(const ASTContext &Ctx) const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K >= firstVar && K <= lastVar; }
Expand Down
6 changes: 4 additions & 2 deletions clang/include/clang/Basic/DiagnosticASTKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,10 @@ def note_constexpr_access_volatile_obj : Note<
"a constant expression">;
def note_constexpr_volatile_here : Note<
"volatile %select{temporary created|object declared|member declared}0 here">;
def note_constexpr_ltor_mutable : Note<
"read of mutable member %0 is not allowed in a constant expression">;
def note_constexpr_access_mutable : Note<
"%select{read of|read of|assignment to|increment of|decrement of|"
"member call on|dynamic_cast of|typeid applied to|destruction of}0 "
"mutable member %1 is not allowed in a constant expression">;
def note_constexpr_ltor_non_const_int : Note<
"read of non-const variable %0 is not allowed in a constant expression">;
def note_constexpr_ltor_non_constexpr : Note<
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -2384,6 +2384,8 @@ def err_constexpr_var_non_literal : Error<
"constexpr variable cannot have non-literal type %0">;
def err_constexpr_var_requires_const_init : Error<
"constexpr variable %0 must be initialized by a constant expression">;
def err_constexpr_var_requires_const_destruction : Error<
"constexpr variable %0 must have constant destruction">;
def err_constexpr_redecl_mismatch : Error<
"%select{non-constexpr|constexpr|consteval}1 declaration of %0"
" follows %select{non-constexpr|constexpr|consteval}2 declaration">;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10064,7 +10064,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
return false;

// Variables that have destruction with side-effects are required.
if (VD->getType().isDestructedType())
if (VD->needsDestruction(*this))
return true;

// Variables that have initialization with side-effects are required.
Expand Down
12 changes: 12 additions & 0 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2592,6 +2592,18 @@ bool VarDecl::isNoDestroy(const ASTContext &Ctx) const {
!hasAttr<AlwaysDestroyAttr>()));
}

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

if (isNoDestroy(Ctx))
return QualType::DK_none;

return getType().isDestructedType();
}

MemberSpecializationInfo *VarDecl::getMemberSpecializationInfo() const {
if (isStaticDataMember())
// FIXME: Remove ?
Expand Down
136 changes: 104 additions & 32 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,15 @@ namespace {
/// evaluated, if any.
APValue::LValueBase EvaluatingDecl;

enum class EvaluatingDeclKind {
None,
/// We're evaluating the construction of EvaluatingDecl.
Ctor,
/// We're evaluating the destruction of EvaluatingDecl.
Dtor,
};
EvaluatingDeclKind IsEvaluatingDecl = EvaluatingDeclKind::None;

/// EvaluatingDeclValue - This is the value being constructed for the
/// declaration whose initializer is being evaluated, if any.
APValue *EvaluatingDeclValue;
Expand Down Expand Up @@ -902,8 +911,10 @@ namespace {
discardCleanups();
}

void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value,
EvaluatingDeclKind EDK = EvaluatingDeclKind::Ctor) {
EvaluatingDecl = Base;
IsEvaluatingDecl = EDK;
EvaluatingDeclValue = &Value;
}

Expand Down Expand Up @@ -2913,8 +2924,8 @@ static bool isReadByLvalueToRvalueConversion(QualType T) {

/// Diagnose an attempt to read from any unreadable field within the specified
/// type, which might be a class type.
static bool diagnoseUnreadableFields(EvalInfo &Info, const Expr *E,
QualType T) {
static bool diagnoseMutableFields(EvalInfo &Info, const Expr *E, AccessKinds AK,
QualType T) {
CXXRecordDecl *RD = T->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
if (!RD)
return false;
Expand All @@ -2929,25 +2940,26 @@ static bool diagnoseUnreadableFields(EvalInfo &Info, const Expr *E,
// FIXME: Add core issue number for the union case.
if (Field->isMutable() &&
(RD->isUnion() || isReadByLvalueToRvalueConversion(Field->getType()))) {
Info.FFDiag(E, diag::note_constexpr_ltor_mutable, 1) << Field;
Info.FFDiag(E, diag::note_constexpr_access_mutable, 1) << AK << Field;
Info.Note(Field->getLocation(), diag::note_declared_at);
return true;
}

if (diagnoseUnreadableFields(Info, E, Field->getType()))
if (diagnoseMutableFields(Info, E, AK, Field->getType()))
return true;
}

for (auto &BaseSpec : RD->bases())
if (diagnoseUnreadableFields(Info, E, BaseSpec.getType()))
if (diagnoseMutableFields(Info, E, AK, BaseSpec.getType()))
return true;

// All mutable fields were empty, and thus not actually read.
return false;
}

static bool lifetimeStartedInEvaluation(EvalInfo &Info,
APValue::LValueBase Base) {
APValue::LValueBase Base,
bool MutableSubobject = false) {
// A temporary we created.
if (Base.getCallIndex())
return true;
Expand All @@ -2956,19 +2968,42 @@ static bool lifetimeStartedInEvaluation(EvalInfo &Info,
if (!Evaluating)
return false;

// The variable whose initializer we're evaluating.
if (auto *BaseD = Base.dyn_cast<const ValueDecl*>())
if (declaresSameEntity(Evaluating, BaseD))
return true;
auto *BaseD = Base.dyn_cast<const ValueDecl*>();

// A temporary lifetime-extended by the variable whose initializer we're
// evaluating.
if (auto *BaseE = Base.dyn_cast<const Expr *>())
if (auto *BaseMTE = dyn_cast<MaterializeTemporaryExpr>(BaseE))
if (declaresSameEntity(BaseMTE->getExtendingDecl(), Evaluating))
return true;
switch (Info.IsEvaluatingDecl) {
case EvalInfo::EvaluatingDeclKind::None:
return false;

return false;
case EvalInfo::EvaluatingDeclKind::Ctor:
// The variable whose initializer we're evaluating.
if (BaseD)
return declaresSameEntity(Evaluating, BaseD);

// A temporary lifetime-extended by the variable whose initializer we're
// evaluating.
if (auto *BaseE = Base.dyn_cast<const Expr *>())
if (auto *BaseMTE = dyn_cast<MaterializeTemporaryExpr>(BaseE))
return declaresSameEntity(BaseMTE->getExtendingDecl(), Evaluating);
return false;

case EvalInfo::EvaluatingDeclKind::Dtor:
// C++2a [expr.const]p6:
// [during constant destruction] the lifetime of a and its non-mutable
// subobjects (but not its mutable subobjects) [are] considered to start
// within e.
//
// FIXME: We can meaningfully extend this to cover non-const objects, but
// we will need special handling: we should be able to access only
// subobjects of such objects that are themselves declared const.
if (!BaseD ||
!(BaseD->getType().isConstQualified() ||
BaseD->getType()->isReferenceType()) ||
MutableSubobject)
return false;
return declaresSameEntity(Evaluating, BaseD);
}

llvm_unreachable("unknown evaluating decl kind");
}

namespace {
Expand All @@ -2986,13 +3021,13 @@ struct CompleteObject {
CompleteObject(APValue::LValueBase Base, APValue *Value, QualType Type)
: Base(Base), Value(Value), Type(Type) {}

bool mayReadMutableMembers(EvalInfo &Info) const {
bool mayAccessMutableMembers(EvalInfo &Info, AccessKinds AK) const {
// In C++14 onwards, it is permitted to read a mutable member whose
// lifetime began within the evaluation.
// FIXME: Should we also allow this in C++11?
if (!Info.getLangOpts().CPlusPlus14)
return false;
return lifetimeStartedInEvaluation(Info, Base);
return lifetimeStartedInEvaluation(Info, Base, /*MutableSubobject*/true);
}

explicit operator bool() const { return !Type.isNull(); }
Expand Down Expand Up @@ -3097,9 +3132,9 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
// things we need to check: if there are any mutable subobjects, we
// cannot perform this read. (This only happens when performing a trivial
// copy or assignment.)
if (ObjType->isRecordType() && isRead(handler.AccessKind) &&
!Obj.mayReadMutableMembers(Info) &&
diagnoseUnreadableFields(Info, E, ObjType))
if (ObjType->isRecordType() &&
!Obj.mayAccessMutableMembers(Info, handler.AccessKind) &&
diagnoseMutableFields(Info, E, handler.AccessKind, ObjType))
return handler.failed();
}

Expand Down Expand Up @@ -3167,10 +3202,10 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
: O->getComplexFloatReal(), ObjType);
}
} else if (const FieldDecl *Field = getAsField(Sub.Entries[I])) {
if (Field->isMutable() && isRead(handler.AccessKind) &&
!Obj.mayReadMutableMembers(Info)) {
Info.FFDiag(E, diag::note_constexpr_ltor_mutable, 1)
<< Field;
if (Field->isMutable() &&
!Obj.mayAccessMutableMembers(Info, handler.AccessKind)) {
Info.FFDiag(E, diag::note_constexpr_access_mutable, 1)
<< handler.AccessKind << Field;
Info.Note(Field->getLocation(), diag::note_declared_at);
return handler.failed();
}
Expand Down Expand Up @@ -3427,8 +3462,7 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
// the variable we're reading must be const.
if (!Frame) {
if (Info.getLangOpts().CPlusPlus14 &&
declaresSameEntity(
VD, Info.EvaluatingDecl.dyn_cast<const ValueDecl *>())) {
lifetimeStartedInEvaluation(Info, LVal.Base)) {
// OK, we can read and modify an object if we're in the process of
// evaluating its initializer, because its lifetime began in this
// evaluation.
Expand Down Expand Up @@ -3518,11 +3552,14 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
// int x = ++r;
// constexpr int k = r;
// Therefore we use the C++14 rules in C++11 too.
const ValueDecl *VD = Info.EvaluatingDecl.dyn_cast<const ValueDecl*>();
const ValueDecl *ED = MTE->getExtendingDecl();
//
// Note that temporaries whose lifetimes began while evaluating a
// variable's constructor are not usable while evaluating the
// corresponding destructor, not even if they're of const-qualified
// types.
if (!(BaseType.isConstQualified() &&
BaseType->isIntegralOrEnumerationType()) &&
!(VD && VD->getCanonicalDecl() == ED->getCanonicalDecl())) {
!lifetimeStartedInEvaluation(Info, LVal.Base)) {
if (!IsAccess)
return CompleteObject(LVal.getLValueBase(), nullptr, BaseType);
Info.FFDiag(E, diag::note_constexpr_access_static_temporary, 1) << AK;
Expand Down Expand Up @@ -13282,6 +13319,41 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
CheckMemoryLeaks(Info);
}

bool VarDecl::evaluateDestruction(
SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
assert(getEvaluatedValue() && !getEvaluatedValue()->isAbsent() &&
"cannot evaluate destruction of non-constant-initialized variable");

Expr::EvalStatus EStatus;
EStatus.Diag = &Notes;

// Make a copy of the value for the destructor to mutate.
APValue DestroyedValue = *getEvaluatedValue();

EvalInfo Info(getASTContext(), EStatus, EvalInfo::EM_ConstantExpression);
Info.setEvaluatingDecl(this, DestroyedValue,
EvalInfo::EvaluatingDeclKind::Dtor);
Info.InConstantContext = true;

SourceLocation DeclLoc = getLocation();
QualType DeclTy = getType();

LValue LVal;
LVal.set(this);

// FIXME: Consider storing whether this variable has constant destruction in
// the EvaluatedStmt so that CodeGen can query it.
if (!HandleDestruction(Info, DeclLoc, LVal.Base, DestroyedValue, DeclTy) ||
EStatus.HasSideEffects)
return false;

if (!Info.discardCleanups())
llvm_unreachable("Unhandled cleanup; missing full expression marker?");

ensureEvaluatedStmt()->HasConstantDestruction = true;
return true;
}

/// isEvaluatable - Call EvaluateAsRValue to see if this expression can be
/// constant folded, but discard the result.
bool Expr::isEvaluatable(const ASTContext &Ctx, SideEffectsKind SEK) const {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/Interp/Interp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {

const SourceInfo &Loc = S.Current->getSource(OpPC);
const FieldDecl *Field = Ptr.getField();
S.FFDiag(Loc, diag::note_constexpr_ltor_mutable, 1) << Field;
S.FFDiag(Loc, diag::note_constexpr_access_mutable, 1) << AK_Read << Field;
S.Note(Field->getLocation(), diag::note_declared_at);
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/AST/TextNodeDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,8 @@ void TextNodeDumper::VisitVarDecl(const VarDecl *D) {
break;
}
}
if (D->needsDestruction(D->getASTContext()))
OS << " destroyed";
if (D->isParameterPack())
OS << " pack";
}
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/CGCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3093,7 +3093,7 @@ void CodeGenFunction::EmitDelegateCallArg(CallArgList &args,
// Deactivate the cleanup for the callee-destructed param that was pushed.
if (hasAggregateEvaluationKind(type) && !CurFuncIsThunk &&
type->getAs<RecordType>()->getDecl()->isParamDestroyedInCallee() &&
type.isDestructedType()) {
param->needsDestruction(getContext())) {
EHScopeStack::stable_iterator cleanup =
CalleeDestructedParamCleanups.lookup(cast<ParmVarDecl>(param));
assert(cleanup.isValid() &&
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/CGClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2083,7 +2083,7 @@ static bool canEmitDelegateCallArgs(CodeGenFunction &CGF,
if (CGF.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) {
// If the parameters are callee-cleanup, it's not safe to forward.
for (auto *P : Ctor->parameters())
if (P->getType().isDestructedType())
if (P->needsDestruction(CGF.getContext()))
return false;

// Likewise if they're inalloca.
Expand Down
Loading

0 comments on commit 2b4fa53

Please sign in to comment.