r270781 - [Sema] Note when we encounter a problem in ExprConstant.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Wed May 25 15:32:24 PDT 2016
Author: gbiv
Date: Wed May 25 17:31:54 2016
New Revision: 270781
URL: http://llvm.org/viewvc/llvm-project?rev=270781&view=rev
Log:
[Sema] Note when we encounter a problem in ExprConstant.
Currently, the constexpr evaluator is very conservative about unmodeled
side-effects when we're evaluating an expression in a mode that allows
such side-effects.
This patch makes us note when we might have actually encountered an
unmodeled side-effect, which allows us to be more accurate when we know
an unmodeled side-effect couldn't have occurred.
This patch has been split into two commits; this one primarily
introduces the bits necessary to track whether we might have potentially
hit such a side-effect. The one that actually does the tracking (which
boils down to more or less a rename of keepEvaluatingAfterFailure to
noteFailure) is coming soon.
Differential Revision: http://reviews.llvm.org/D18540
Added:
cfe/trunk/test/SemaCXX/builtin-object-size-cxx14.cpp
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=270781&r1=270780&r2=270781&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed May 25 17:31:54 2016
@@ -478,6 +478,9 @@ namespace {
/// fold (not just why it's not strictly a constant expression)?
bool HasFoldFailureDiagnostic;
+ /// \brief Whether or not we're currently speculatively evaluating.
+ bool IsSpeculativelyEvaluating;
+
enum EvaluationMode {
/// Evaluate as a constant expression. Stop if we find that the expression
/// is not a constant expression.
@@ -542,7 +545,8 @@ namespace {
BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
EvaluatingDecl((const ValueDecl *)nullptr),
EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
- HasFoldFailureDiagnostic(false), EvalMode(Mode) {}
+ HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+ EvalMode(Mode) {}
void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
EvaluatingDecl = Base;
@@ -764,6 +768,29 @@ namespace {
llvm_unreachable("Missed EvalMode case");
}
+ /// Notes that we failed to evaluate an expression that other expressions
+ /// directly depend on, and determine if we should keep evaluating. This
+ /// should only be called if we actually intend to keep evaluating.
+ ///
+ /// Call noteSideEffect() instead if we may be able to ignore the value that
+ /// we failed to evaluate, e.g. if we failed to evaluate Foo() in:
+ ///
+ /// (Foo(), 1) // use noteSideEffect
+ /// (Foo() || true) // use noteSideEffect
+ /// Foo() + 1 // use noteFailure
+ LLVM_ATTRIBUTE_UNUSED_RESULT bool noteFailure() {
+ // Failure when evaluating some expression often means there is some
+ // subexpression whose evaluation was skipped. Therefore, (because we
+ // don't track whether we skipped an expression when unwinding after an
+ // evaluation failure) every evaluation failure that bubbles up from a
+ // subexpression implies that a side-effect has potentially happened. We
+ // skip setting the HasSideEffects flag to true until we decide to
+ // continue evaluating after that point, which happens here.
+ bool KeepGoing = keepEvaluatingAfterFailure();
+ EvalStatus.HasSideEffects |= KeepGoing;
+ return KeepGoing;
+ }
+
bool allowInvalidBaseExpr() const {
return EvalMode == EM_DesignatorFold;
}
@@ -812,24 +839,52 @@ namespace {
~FoldOffsetRAII() { Info.EvalMode = OldMode; }
};
- /// RAII object used to suppress diagnostics and side-effects from a
- /// speculative evaluation.
+ /// RAII object used to optionally suppress diagnostics and side-effects from
+ /// a speculative evaluation.
class SpeculativeEvaluationRAII {
- EvalInfo &Info;
+ /// Pair of EvalInfo, and a bit that stores whether or not we were
+ /// speculatively evaluating when we created this RAII.
+ llvm::PointerIntPair<EvalInfo *, 1, bool> InfoAndOldSpecEval;
Expr::EvalStatus Old;
+ void moveFromAndCancel(SpeculativeEvaluationRAII &&Other) {
+ InfoAndOldSpecEval = Other.InfoAndOldSpecEval;
+ Old = Other.Old;
+ Other.InfoAndOldSpecEval.setPointer(nullptr);
+ }
+
+ void maybeRestoreState() {
+ EvalInfo *Info = InfoAndOldSpecEval.getPointer();
+ if (!Info)
+ return;
+
+ Info->EvalStatus = Old;
+ Info->IsSpeculativelyEvaluating = InfoAndOldSpecEval.getInt();
+ }
+
public:
- SpeculativeEvaluationRAII(EvalInfo &Info,
- SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)
- : Info(Info), Old(Info.EvalStatus) {
+ SpeculativeEvaluationRAII() = default;
+
+ SpeculativeEvaluationRAII(
+ EvalInfo &Info, SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)
+ : InfoAndOldSpecEval(&Info, Info.IsSpeculativelyEvaluating),
+ Old(Info.EvalStatus) {
Info.EvalStatus.Diag = NewDiag;
- // If we're speculatively evaluating, we may have skipped over some
- // evaluations and missed out a side effect.
- Info.EvalStatus.HasSideEffects = true;
+ Info.IsSpeculativelyEvaluating = true;
}
- ~SpeculativeEvaluationRAII() {
- Info.EvalStatus = Old;
+
+ SpeculativeEvaluationRAII(const SpeculativeEvaluationRAII &Other) = delete;
+ SpeculativeEvaluationRAII(SpeculativeEvaluationRAII &&Other) {
+ moveFromAndCancel(std::move(Other));
}
+
+ SpeculativeEvaluationRAII &operator=(SpeculativeEvaluationRAII &&Other) {
+ maybeRestoreState();
+ moveFromAndCancel(std::move(Other));
+ return *this;
+ }
+
+ ~SpeculativeEvaluationRAII() { maybeRestoreState(); }
};
/// RAII object wrapping a full-expression or block scope, and handling
@@ -2789,12 +2844,13 @@ static CompleteObject findCompleteObject
}
// In C++1y, we can't safely access any mutable state when we might be
- // evaluating after an unmodeled side effect or an evaluation failure.
+ // evaluating after an unmodeled side effect.
//
// FIXME: Not all local state is mutable. Allow local constant subobjects
// to be read here (but take care with 'mutable' fields).
- if (Frame && Info.getLangOpts().CPlusPlus14 &&
- (Info.EvalStatus.HasSideEffects || Info.keepEvaluatingAfterFailure()))
+ if ((Frame && Info.getLangOpts().CPlusPlus14 &&
+ Info.EvalStatus.HasSideEffects) ||
+ (AK != AK_Read && Info.IsSpeculativelyEvaluating))
return CompleteObject();
return CompleteObject(BaseVal, BaseType);
@@ -4049,14 +4105,16 @@ private:
assert(Info.checkingPotentialConstantExpression());
// Speculatively evaluate both arms.
+ SmallVector<PartialDiagnosticAt, 8> Diag;
{
- SmallVector<PartialDiagnosticAt, 8> Diag;
SpeculativeEvaluationRAII Speculate(Info, &Diag);
-
StmtVisitorTy::Visit(E->getFalseExpr());
if (Diag.empty())
return;
+ }
+ {
+ SpeculativeEvaluationRAII Speculate(Info, &Diag);
Diag.clear();
StmtVisitorTy::Visit(E->getTrueExpr());
if (Diag.empty())
@@ -4071,7 +4129,7 @@ private:
bool HandleConditionalOperator(const ConditionalOperator *E) {
bool BoolResult;
if (!EvaluateAsBooleanCondition(E->getCond(), BoolResult, Info)) {
- if (Info.checkingPotentialConstantExpression())
+ if (Info.checkingPotentialConstantExpression() && Info.noteFailure())
CheckPotentialConstantConditional(E);
return false;
}
@@ -7027,23 +7085,14 @@ class DataRecursiveIntBinOpEvaluator {
Job() = default;
Job(Job &&J)
: E(J.E), LHSResult(J.LHSResult), Kind(J.Kind),
- StoredInfo(J.StoredInfo), OldEvalStatus(J.OldEvalStatus) {
- J.StoredInfo = nullptr;
- }
+ SpecEvalRAII(std::move(J.SpecEvalRAII)) {}
void startSpeculativeEval(EvalInfo &Info) {
- OldEvalStatus = Info.EvalStatus;
- Info.EvalStatus.Diag = nullptr;
- StoredInfo = &Info;
- }
- ~Job() {
- if (StoredInfo) {
- StoredInfo->EvalStatus = OldEvalStatus;
- }
+ SpecEvalRAII = SpeculativeEvaluationRAII(Info);
}
+
private:
- EvalInfo *StoredInfo = nullptr; // non-null if status changed.
- Expr::EvalStatus OldEvalStatus;
+ SpeculativeEvaluationRAII SpecEvalRAII;
};
SmallVector<Job, 16> Queue;
@@ -7145,7 +7194,7 @@ bool DataRecursiveIntBinOpEvaluator::
LHSResult.Failed = true;
// Since we weren't able to evaluate the left hand side, it
- // must have had side effects.
+ // might have had side effects.
if (!Info.noteSideEffect())
return false;
@@ -7313,10 +7362,34 @@ void DataRecursiveIntBinOpEvaluator::pro
llvm_unreachable("Invalid Job::Kind!");
}
+namespace {
+/// Used when we determine that we should fail, but can keep evaluating prior to
+/// noting that we had a failure.
+class DelayedNoteFailureRAII {
+ EvalInfo &Info;
+ bool NoteFailure;
+
+public:
+ DelayedNoteFailureRAII(EvalInfo &Info, bool NoteFailure = true)
+ : Info(Info), NoteFailure(NoteFailure) {}
+ ~DelayedNoteFailureRAII() {
+ if (NoteFailure) {
+ bool ContinueAfterFailure = Info.noteFailure();
+ (void)ContinueAfterFailure;
+ assert(ContinueAfterFailure &&
+ "Shouldn't have kept evaluating on failure.");
+ }
+ }
+};
+}
+
bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
+ // We don't call noteFailure immediately because the assignment happens after
+ // we evaluate LHS and RHS.
if (!Info.keepEvaluatingAfterFailure() && E->isAssignmentOp())
return Error(E);
+ DelayedNoteFailureRAII MaybeNoteFailureLater(Info, E->isAssignmentOp());
if (DataRecursiveIntBinOpEvaluator::shouldEnqueue(E))
return DataRecursiveIntBinOpEvaluator(*this, Result).Traverse(E);
@@ -7638,7 +7711,7 @@ bool IntExprEvaluator::VisitBinaryOperat
MemberPtr LHSValue, RHSValue;
bool LHSOK = EvaluateMemberPointer(E->getLHS(), LHSValue, Info);
- if (!LHSOK && Info.keepEvaluatingAfterFailure())
+ if (!LHSOK && !Info.keepEvaluatingAfterFailure())
return false;
if (!EvaluateMemberPointer(E->getRHS(), RHSValue, Info) || !LHSOK)
Added: cfe/trunk/test/SemaCXX/builtin-object-size-cxx14.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-object-size-cxx14.cpp?rev=270781&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/builtin-object-size-cxx14.cpp (added)
+++ cfe/trunk/test/SemaCXX/builtin-object-size-cxx14.cpp Wed May 25 17:31:54 2016
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
+
+namespace basic {
+// Ensuring that __bos can be used in constexpr functions without anything
+// sketchy going on...
+constexpr int bos0() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 0);
+}
+
+constexpr int bos1() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 1);
+}
+
+constexpr int bos2() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 2);
+}
+
+constexpr int bos3() {
+ int k = 5;
+ char cs[10] = {};
+ return __builtin_object_size(&cs[k], 3);
+}
+
+static_assert(bos0() == sizeof(char) * 5, "");
+static_assert(bos1() == sizeof(char) * 5, "");
+static_assert(bos2() == sizeof(char) * 5, "");
+static_assert(bos3() == sizeof(char) * 5, "");
+}
+
+namespace in_enable_if {
+// The code that prompted these changes was __bos in enable_if
+
+void copy5CharsInto(char *buf) // expected-note{{candidate}}
+ __attribute__((enable_if(__builtin_object_size(buf, 0) != -1 &&
+ __builtin_object_size(buf, 0) > 5,
+ "")));
+
+// We use different EvalModes for __bos with type 0 versus 1. Ensure 1 works,
+// too...
+void copy5CharsIntoStrict(char *buf) // expected-note{{candidate}}
+ __attribute__((enable_if(__builtin_object_size(buf, 1) != -1 &&
+ __builtin_object_size(buf, 1) > 5,
+ "")));
+
+struct LargeStruct {
+ int pad;
+ char buf[6];
+ int pad2;
+};
+
+struct SmallStruct {
+ int pad;
+ char buf[5];
+ int pad2;
+};
+
+void noWriteToBuf() {
+ char buf[6];
+ copy5CharsInto(buf);
+
+ LargeStruct large;
+ copy5CharsIntoStrict(large.buf);
+}
+
+void initTheBuf() {
+ char buf[6] = {};
+ copy5CharsInto(buf);
+
+ LargeStruct large = {0, {}, 0};
+ copy5CharsIntoStrict(large.buf);
+}
+
+int getI();
+void initTheBufWithALoop() {
+ char buf[6] = {};
+ for (unsigned I = getI(); I != sizeof(buf); ++I)
+ buf[I] = I;
+ copy5CharsInto(buf);
+
+ LargeStruct large;
+ for (unsigned I = getI(); I != sizeof(buf); ++I)
+ large.buf[I] = I;
+ copy5CharsIntoStrict(large.buf);
+}
+
+void tooSmallBuf() {
+ char buf[5];
+ copy5CharsInto(buf); // expected-error{{no matching function for call}}
+
+ SmallStruct small;
+ copy5CharsIntoStrict(small.buf); // expected-error{{no matching function for call}}
+}
+}
Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp?rev=270781&r1=270780&r2=270781&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Wed May 25 17:31:54 2016
@@ -179,12 +179,10 @@ namespace string_assign {
static_assert(!test1(100), "");
static_assert(!test1(101), ""); // expected-error {{constant expression}} expected-note {{in call to 'test1(101)'}}
- // FIXME: We should be able to reject this before it's called
- constexpr void f() {
+ constexpr void f() { // expected-error{{constexpr function never produces a constant expression}} expected-note at +2{{assignment to dereferenced one-past-the-end pointer is not allowed in a constant expression}}
char foo[10] = { "z" }; // expected-note {{here}}
- foo[10] = 'x'; // expected-warning {{past the end}} expected-note {{assignment to dereferenced one-past-the-end pointer}}
+ foo[10] = 'x'; // expected-warning {{past the end}}
}
- constexpr int k = (f(), 0); // expected-error {{constant expression}} expected-note {{in call}}
}
namespace array_resize {
@@ -938,3 +936,16 @@ namespace EmptyClass {
constexpr int testb = f(e2, 3); // expected-error {{constant expression}} expected-note {{in call}}
constexpr int testc = f(e3, 3);
}
+
+namespace SpeculativeEvalWrites {
+ // Ensure that we don't try to speculatively evaluate writes.
+ constexpr int f() {
+ int i = 0;
+ int a = 0;
+ // __builtin_object_size speculatively evaluates its first argument.
+ __builtin_object_size((i = 1, &a), 0);
+ return i;
+ }
+
+ static_assert(!f(), "");
+}
More information about the cfe-commits
mailing list