[Sema] Note when we've actually encountered a failure in ExprConstant, and take that into account when looking up objects.

[george.burgess.iv]

Summary

This patch aims to fix a bug encountered by compiling code with C++14.

Specifically, when evaluating p in __builtin_object_size(p, n), we start speculatively evaluating. Currently, clang pretends that side-effects have always occurred when we start speculatively evaluating. This prevents us from reading any locals in C++14. So, the following code compiles fine in C++11 mode, but fails to compile with C++14:

void foo(char *p) __attribute__((enable_if(__builtin_object_size(p, 0) != -1, "")));

void runFoo() {
  char buf[5];
  foo(buf);
}

This patch makes ExprConstant note when we've actually failed (read: may have an unmodeled side-effect), and makes us act as conservatively before if a failure happened. If a failure hasn't happened, we can be more aggressive.

[george.burgess.iv]

test/SemaCXX/constant-expression-cxx1y.cpp

182

Aside: This test was fixed because we no longer give up on trying to look up foo, because we know an unmodeled side-effect hasn't happened.

[rsmith]

include/clang/AST/Expr.h

541–546

I don't think this provides enough guarantees to be useful as part of the external EvalStatus results. Can you sink this into EvalInfo? Alternatively, can you merge this with HasSideEffects? I don't think we need to maintain a distinction between the two different ways we can get an evaluation result after skipping a subexpression.

[george.burgess.iv]

Addressed all feedback.

include/clang/AST/Expr.h

541–546

I don't think this provides enough guarantees to be useful as part of the external EvalStatus results. Can you sink this into EvalInfo

WFM

Alternatively, can you merge this with HasSideEffects.

The result is a bit more subtle (because some things depend on HasSideEffects being accurate, and not being set whenever we fail), but sure.

[george.burgess.iv]

Ping :)

[rsmith]

lib/AST/ExprConstant.cpp

782

unchange -> unchanged

784–785

I think this is the right behavior regardless of the current expectations of any particular caller. After this patch:

• HasSideEffects == false indicates that the evaluation (if evaluation was successful, then up to the current point, otherwise up to the point where evaluation failed) has definitely not skipped any side-effects
• HasSideEffects == true means nothing is known about side-effects (in particular, it does not guarantee that a side-effect has definitely happened, just that one /might/ have happened)

(If any caller expects these flags to mean something else, they are wrong -- and note that HasSideEffects is meaningless if evaluation fails.) Therefore, if we transition from an "evaluation has failed" to an "evaluation was successful" state, we must set HasSideEffects to true to model the subexpressions we might have skipped. And if we just unwind (because keepEvaluatingAfterFailure() returns false), then we don't need to set HasSideEffects at all.

Perhaps you could say something like this:

"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."

855–857

I think this comment is now redundant / incorrect.

2645

IsSpeculativelyEvaluating is always false. Presumably it should be set to true in the SpeculativeEvaluationRAII constructor. (Please also add a testcase to catch this...)

3283

Can we give this function a name that reads more naturally at the call site? It's locally unclear what this condition is testing. Maybe keep the old name?

4077–4079

These RAII objects are cheap; maybe just create two of them regardless?

4108–4109

I think this should be an if rather than an assert. If we hit something that's known to be non-constant while evaluating the condition of a ?:, we should bail out of evaluation in checkingPotentialConstantExpression mode. keepEvaluatingAfterFailure should return false in that case (even though it doesn't do so today).

7373–7374

The side-effect here should be delayed until after we've evaluated the left and right hand sides of the assignment.

[george.burgess.iv]

Addressed feedback

[george.burgess.iv]

lib/AST/ExprConstant.cpp

855–857

My bad :)

3283

The best alternative I can come up with is noteFailureAndGetShouldContinueEvaluating(), which occupies 44 characters.

...I think I'll go back to the old name.

4108–4109

I'm not sure that I fully understand what you mean by this. Are you saying that we (ultimately) only want to check the arms of a conditional when we're evaluating for overflow?

Either way, took a stab at fixing it.

7103

I'm happy to make SpeculativeEvalRAII a type that optionally restores state, if you think that would be better. That way, we don't need to duplicate this logic...

Also, I can't figure out how to test this, because it's only ever run after HasSideEffects = true, and the only thing that (I think) checks for if we're speculatively evaluating *also* fails in exactly the same way if we have side effects.

[george.burgess.iv]

Ping :)

[george.burgess.iv]

Ping :)

[rsmith]

LGTM with or without the SpeculativeEvaluationRAII refactor (which is ultimately pre-existing duplication).

Please commit the rename of keepEvaluatingAfterFailure -> noteFailure separately.

lib/AST/ExprConstant.cpp

4108–4109

The relevant case is when we're checking the body of a constexpr function to see whether it could possibly be a constant expression. In that case, what we do next should depend on whether we've already found something that is known to never be constant. If so (which is indicated by us having a diagnostic), we should bail out. If not, we should check both arms of the conditional operator and report a problem if neither of them can be constant.

I think the upshot of all of that is that keepEvaluatingAfterFailure should return false for EM_PotentialConstantExpression* if we have already produced a diagnostic. But that's not relevant for this particular patch, so let's leave it as you have it for now. We can revisit this.

7103

Yes, if we can reuse SpeculativeEvaluationRAII here, or otherwise factor out the duplication, that would be better. Also I think you're probably right that this is not observable right now.

test/SemaCXX/constant-expression-cxx1y.cpp

942

Unused?

include/clang/AST/Expr.h

struct EvalStatus {
     /// For example, (f() && 0) can be folded, but it still has side effects.
     bool HasSideEffects;
 
     /// \brief Whether the evaluation hit undefined behavior.
     /// For example, 1.0 / 0.0 can be folded to Inf, but has undefined behavior.
     /// Likewise, INT_MAX + 1 can be folded to INT_MIN, but has UB.
     bool HasUndefinedBehavior;
 
+    /// \brief Whether the evaluation was unable to reduce a construct to a
+    /// value.
+    ///
+    /// For example, in foo(bar, baz, qux), if evaluating bar fails, we may
+    /// try to evaluate baz and qux anyway.
+    bool HasFailure;
+
     /// Diag - If this is non-null, it will be filled in with a stack of notes
     /// indicating why evaluation failed (or why it failed to produce a constant
     /// expression).
     /// If the expression is unfoldable, the notes will indicate why it's not
     /// foldable. If the expression is foldable, but not a constant expression,
     /// the notes will describes why it isn't a constant expression. If the
     /// expression *is* a constant expression, no notes will be produced.
     SmallVectorImpl<PartialDiagnosticAt> *Diag;
 
     EvalStatus()
-        : HasSideEffects(false), HasUndefinedBehavior(false), Diag(nullptr) {}
+        : HasSideEffects(false), HasUndefinedBehavior(false), HasFailure(false),
+          Diag(nullptr) {}
 
     // hasSideEffects - Return true if the evaluated expression has
     // side effects.
     bool hasSideEffects() const {
       return HasSideEffects;
     }
   };
 

lib/AST/ExprConstant.cpp

struct EvalInfo {
     /// HasActiveDiagnostic - Was the previous diagnostic stored? If so, further
     /// notes attached to it will also be stored, otherwise they will not be.
     bool HasActiveDiagnostic;
 
     /// \brief Have we emitted a diagnostic explaining why we couldn't constant
     /// 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.
       EM_ConstantExpression,
 
       /// Evaluate as a potential constant expression. Keep going if we hit a
       /// construct that we can't evaluate yet (because we don't yet know the
       /// value of something) but stop if we hit something that could never be
struct EvalInfo {
 
     EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
       : Ctx(const_cast<ASTContext &>(C)), EvalStatus(S), CurrentCall(nullptr),
         CallStackDepth(0), NextCallIndex(1),
         StepsLeft(getLangOpts().ConstexprStepLimit),
         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;
       EvaluatingDeclValue = &Value;
     }
 
     const LangOptions &getLangOpts() const { return Ctx.getLangOpts(); }
 
bool keepEvaluatingAfterFailure() {
       case EM_ConstantFold:
       case EM_IgnoreSideEffects:
       case EM_DesignatorFold:
         return false;
       }
       llvm_unreachable("Missed EvalMode case");
     }
 
+    /// Notes that we failed to evaluate an expression, and determine if we
+    /// should keep evaluating.
+    ///
+    /// Currently, this is used as a fancy way of noting that there *may* have
+    /// been a side-effect that we didn't see (and can't model).
+    ///
+    /// Note that this is sometimes *not* called when a failure happens. The
+    /// only time we really care about this case is when we're going to try to
+    /// evaluate things after the failure. It's therefore the responsibility of
+    /// the caller that wants to continue evaluating after a failure to call
+    /// this.
+    bool noteFailure() {
+      EvalStatus.HasFailure = true;
+      return keepEvaluatingAfterFailure();
+    }
+
+    /// Returns whether a side-effect that we can't model might have occurred.
+    /// Moreover, whether we've seen a side-effect, or if we failed to evaluate
+    /// something that might produce a side-effect.
+    bool sideEffectsMightHaveOccurred() const {
+      return EvalStatus.HasSideEffects || EvalStatus.HasFailure;
+    }
+
     bool allowInvalidBaseExpr() const {
       return EvalMode == EM_DesignatorFold;
     }
   };
 
   /// Object used to treat all foldable expressions as constant expressions.
   struct FoldConstant {
     EvalInfo &Info;
struct FoldOffsetRAII {
     ~FoldOffsetRAII() { Info.EvalMode = OldMode; }
   };
 
   /// RAII object used to suppress diagnostics and side-effects from a
   /// speculative evaluation.
   class SpeculativeEvaluationRAII {
     EvalInfo &Info;
     Expr::EvalStatus Old;
+    bool OldSpecEval;
 
   public:
-    SpeculativeEvaluationRAII(EvalInfo &Info,
-                        SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)
-      : Info(Info), Old(Info.EvalStatus) {
+    SpeculativeEvaluationRAII(
+        EvalInfo &Info, SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)
+        : Info(Info), Old(Info.EvalStatus),
+          OldSpecEval(Info.IsSpeculativelyEvaluating) {
       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;
     }
     ~SpeculativeEvaluationRAII() {
       Info.EvalStatus = Old;
+      Info.IsSpeculativelyEvaluating = OldSpecEval;
     }
   };
 
   /// RAII object wrapping a full-expression or block scope, and handling
   /// the ending of the lifetime of temporaries created within it.
   template<bool IsFullExpression>
   class ScopeRAII {
     EvalInfo &Info;

 static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
                                          AccessKinds AK, const LValue &LVal,
                                          QualType LValType) {
   if (!LVal.Base) {
     Info.Diag(E, diag::note_constexpr_access_null) << AK;
     return CompleteObject();
   }
 
+  // Don't allow the speculative evaluation of writes.
+  if (AK != AK_Read && Info.IsSpeculativelyEvaluating)
+    return CompleteObject();
+
   CallStackFrame *Frame = nullptr;
   if (LVal.CallIndex) {
     Frame = Info.getCallFrame(LVal.CallIndex);
     if (!Frame) {
       Info.Diag(E, diag::note_constexpr_lifetime_ended, 1)
         << AK << LVal.Base.is<const ValueDecl*>();
       NoteLValueLocation(Info, LVal.Base);
       return CompleteObject();
static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
   // and this doesn't do quite the right thing for const subobjects of the
   // object under construction.
   if (LVal.getLValueBase() == Info.EvaluatingDecl) {
     BaseType = Info.Ctx.getCanonicalType(BaseType);
     BaseType.removeLocalConst();
   }
 
   // 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()))
+      Info.sideEffectsMightHaveOccurred())
     return CompleteObject();
 
   return CompleteObject(BaseVal, BaseType);
 }
 
 /// \brief Perform an lvalue-to-rvalue conversion on the given glvalue. This
 /// can also be used for 'lvalue-to-lvalue' conversions for looking up the
 /// glvalue referred to by an entity of reference type.

 
 static const ValueDecl *HandleMemberPointerAccess(EvalInfo &Info,
                                                   const BinaryOperator *BO,
                                                   LValue &LV,
                                                   bool IncludeMember = true) {
   assert(BO->getOpcode() == BO_PtrMemD || BO->getOpcode() == BO_PtrMemI);
 
   if (!EvaluateObjectArgument(Info, BO->getLHS(), LV)) {
-    if (Info.keepEvaluatingAfterFailure()) {
+    if (Info.noteFailure()) {
       MemberPtr MemPtr;
       EvaluateMemberPointer(BO->getRHS(), MemPtr, Info);
     }
     return nullptr;
   }
 
   return HandleMemberPointerAccess(Info, BO->getLHS()->getType(), LV,
                                    BO->getRHS(), IncludeMember);
static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info,
 
   case Stmt::DeclStmtClass: {
     const DeclStmt *DS = cast<DeclStmt>(S);
     for (const auto *DclIt : DS->decls()) {
       // Each declaration initialization is its own full-expression.
       // FIXME: This isn't quite right; if we're performing aggregate
       // initialization, each braced subexpression is its own full-expression.
       FullExpressionRAII Scope(Info);
-      if (!EvaluateDecl(Info, DclIt) && !Info.keepEvaluatingAfterFailure())
+      if (!EvaluateDecl(Info, DclIt) && !Info.noteFailure())
         return ESR_Failed;
     }
     return ESR_Succeeded;
   }
 
   case Stmt::ReturnStmtClass: {
     const Expr *RetExpr = cast<ReturnStmt>(S)->getRetValue();
     FullExpressionRAII Scope(Info);

 static bool EvaluateArgs(ArrayRef<const Expr*> Args, ArgVector &ArgValues,
                          EvalInfo &Info) {
   bool Success = true;
   for (ArrayRef<const Expr*>::iterator I = Args.begin(), E = Args.end();
        I != E; ++I) {
     if (!Evaluate(ArgValues[I - Args.begin()], Info, *I)) {
       // If we're checking for a potential constant expression, evaluate all
       // initializers even if some of them fail.
-      if (!Info.keepEvaluatingAfterFailure())
+      if (!Info.noteFailure())
         return false;
       Success = false;
     }
   }
   return Success;
 }
 
 /// Evaluate a function call.
#endif
     }
 
     FullExpressionRAII InitScope(Info);
     if (!EvaluateInPlace(*Value, Info, Subobject, I->getInit()) ||
         (FD && FD->isBitField() && !truncateBitfieldValue(Info, I->getInit(),
                                                           *Value, FD))) {
       // If we're checking for a potential constant expression, evaluate all
       // initializers even if some of them fail.
-      if (!Info.keepEvaluatingAfterFailure())
+      if (!Info.noteFailure())
         return false;
       Success = false;
     }
   }
 
   return Success &&
          EvaluateStmt(Ret, Info, Definition->getBody()) != ESR_Failed;
 }
private:
   }
 
   // Check whether a conditional operator with a non-constant condition is a
   // potential constant expression. If neither arm is a potential constant
   // expression, then the conditional operator is not either.
   template<typename ConditionalOperator>
   void CheckPotentialConstantConditional(const ConditionalOperator *E) {
     assert(Info.checkingPotentialConstantExpression());
+    assert(
+        Info.EvalStatus.HasFailure &&
+        "Need multiple speculative eval RAIIs below if we haven't failed yet.");
 
     // Speculatively evaluate both arms.
     {
       SmallVector<PartialDiagnosticAt, 8> Diag;
       SpeculativeEvaluationRAII Speculate(Info, &Diag);
 
       StmtVisitorTy::Visit(E->getFalseExpr());
       if (Diag.empty())
         return;
 
       Diag.clear();
       StmtVisitorTy::Visit(E->getTrueExpr());
       if (Diag.empty())
         return;
     }
 
     Error(E, diag::note_constexpr_conditional_never_const);
   }
 
 
   template<typename ConditionalOperator>
   bool HandleConditionalOperator(const ConditionalOperator *E) {
     bool BoolResult;
     if (!EvaluateAsBooleanCondition(E->getCond(), BoolResult, Info)) {
+      Info.noteFailure();
       if (Info.checkingPotentialConstantExpression())
         CheckPotentialConstantConditional(E);
       return false;
     }
 
     Expr *EvalExpr = BoolResult ? E->getTrueExpr() : E->getFalseExpr();
     return StmtVisitorTy::Visit(EvalExpr);
   }
 
 protected:
   EvalInfo &Info;
   typedef ConstStmtVisitor<Derived, bool> StmtVisitorTy;
   typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
bool LValueExprEvaluator::VisitCompoundAssignOperator(
     const CompoundAssignOperator *CAO) {
   if (!Info.getLangOpts().CPlusPlus14 && !Info.keepEvaluatingAfterFailure())
     return Error(CAO);
 
   APValue RHS;
 
   // The overall lvalue result is the result of evaluating the LHS.
   if (!this->Visit(CAO->getLHS())) {
-    if (Info.keepEvaluatingAfterFailure())
+    if (Info.noteFailure())
       Evaluate(RHS, this->Info, CAO->getRHS());
     return false;
   }
 
   if (!Evaluate(RHS, this->Info, CAO->getRHS()))
     return false;
 
   return handleCompoundAssignment(
       this->Info, CAO,
       Result, CAO->getLHS()->getType(), CAO->getComputationLHSType(),
       CAO->getOpForCompoundAssignment(CAO->getOpcode()), RHS);
 }
 
 bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
   if (!Info.getLangOpts().CPlusPlus14 && !Info.keepEvaluatingAfterFailure())
     return Error(E);
 
   APValue NewVal;
 
   if (!this->Visit(E->getLHS())) {
-    if (Info.keepEvaluatingAfterFailure())
+    if (Info.noteFailure())
       Evaluate(NewVal, this->Info, E->getRHS());
     return false;
   }
 
   if (!Evaluate(NewVal, this->Info, E->getRHS()))
     return false;
 
   return handleAssignment(this->Info, E, Result, E->getLHS()->getType(),
if (E->getOpcode() != BO_Add &&
     return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
 
   const Expr *PExp = E->getLHS();
   const Expr *IExp = E->getRHS();
   if (IExp->getType()->isPointerType())
     std::swap(PExp, IExp);
 
   bool EvalPtrOK = EvaluatePointer(PExp, Result, Info);
-  if (!EvalPtrOK && !Info.keepEvaluatingAfterFailure())
+  if (!EvalPtrOK && !Info.noteFailure())
     return false;
 
   llvm::APSInt Offset;
   if (!EvaluateInteger(IExp, Offset, Info) || !EvalPtrOK)
     return false;
 
   int64_t AdditionalOffset = getExtValue(Offset);
   if (E->getOpcode() == BO_Sub)
for (const auto &Base : CXXRD->bases()) {
       const Expr *Init = E->getInit(ElementNo);
 
       LValue Subobject = This;
       if (!HandleLValueBase(Info, Init, Subobject, CXXRD, &Base))
         return false;
 
       APValue &FieldVal = Result.getStructBase(ElementNo);
       if (!EvaluateInPlace(FieldVal, Info, Subobject, Init)) {
-        if (!Info.keepEvaluatingAfterFailure())
+        if (!Info.noteFailure())
           return false;
         Success = false;
       }
       ++ElementNo;
     }
   }
 
   // Initialize members.
for (const auto *Field : RD->fields()) {
     // Temporarily override This, in case there's a CXXDefaultInitExpr in here.
     ThisOverrideRAII ThisOverride(*Info.CurrentCall, &This,
                                   isa<CXXDefaultInitExpr>(Init));
 
     APValue &FieldVal = Result.getStructField(Field->getFieldIndex());
     if (!EvaluateInPlace(FieldVal, Info, Subobject, Init) ||
         (Field->isBitField() && !truncateBitfieldValue(Info, Init,
                                                        FieldVal, Field))) {
-      if (!Info.keepEvaluatingAfterFailure())
+      if (!Info.noteFailure())
         return false;
       Success = false;
     }
   }
 
   return Success;
 }
 
bool ArrayExprEvaluator::VisitInitListExpr(const InitListExpr *E) {
   Subobject.addArray(Info, E, CAT);
   for (unsigned Index = 0; Index != NumEltsToInit; ++Index) {
     const Expr *Init =
         Index < E->getNumInits() ? E->getInit(Index) : FillerExpr;
     if (!EvaluateInPlace(Result.getArrayInitializedElt(Index),
                          Info, Subobject, Init) ||
         !HandleLValueArrayAdjustment(Info, Init, Subobject,
                                      CAT->getElementType(), 1)) {
-      if (!Info.keepEvaluatingAfterFailure())
+      if (!Info.noteFailure())
         return false;
       Success = false;
     }
   }
 
   if (!Result.hasArrayFiller())
     return Success;
 
struct Job {
     ~Job() {
       if (StoredInfo) {
         StoredInfo->EvalStatus = OldEvalStatus;
       }
     }
   private:
     EvalInfo *StoredInfo = nullptr; // non-null if status changed.
     Expr::EvalStatus OldEvalStatus;
   };
 
   SmallVector<Job, 16> Queue;
 
   IntExprEvaluator &IntEval;
   EvalInfo &Info;
   APValue &FinalResult;
 
 public:
if (E->isLogicalOp()) {
     }
 
     return true;
   }
 
   assert(E->getLHS()->getType()->isIntegralOrEnumerationType() &&
          E->getRHS()->getType()->isIntegralOrEnumerationType());
 
-  if (LHSResult.Failed && !Info.keepEvaluatingAfterFailure())
+  if (LHSResult.Failed && !Info.noteFailure())
     return false; // Ignore RHS;
 
   return true;
 }
 
 bool DataRecursiveIntBinOpEvaluator::
        VisitBinOp(const EvalResult &LHSResult, const EvalResult &RHSResult,
                   const BinaryOperator *E, APValue &Result) {
case Job::BinOpVisitedLHSKind: {
       return;
     }
   }
   
   llvm_unreachable("Invalid Job::Kind!");
 }
 
 bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
-  if (!Info.keepEvaluatingAfterFailure() && E->isAssignmentOp())
+  if (E->isAssignmentOp() && !Info.noteFailure())
     return Error(E);
 
   if (DataRecursiveIntBinOpEvaluator::shouldEnqueue(E))
     return DataRecursiveIntBinOpEvaluator(*this, Result).Traverse(E);
 
   QualType LHSTy = E->getLHS()->getType();
   QualType RHSTy = E->getRHS()->getType();
 
   if (LHSTy->isAnyComplexType() || RHSTy->isAnyComplexType()) {
     ComplexValue LHS, RHS;
     bool LHSOK;
     if (E->isAssignmentOp()) {
       LValue LV;
       EvaluateLValue(E->getLHS(), LV, Info);
       LHSOK = false;
     } else if (LHSTy->isRealFloatingType()) {
       LHSOK = EvaluateFloat(E->getLHS(), LHS.FloatReal, Info);
       if (LHSOK) {
         LHS.makeComplexFloat();
         LHS.FloatImag = APFloat(LHS.FloatReal.getSemantics());
       }
     } else {
       LHSOK = EvaluateComplex(E->getLHS(), LHS, Info);
     }
-    if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+    if (!LHSOK && !Info.noteFailure())
       return false;
 
     if (E->getRHS()->getType()->isRealFloatingType()) {
       if (!EvaluateFloat(E->getRHS(), RHS.FloatReal, Info) || !LHSOK)
         return false;
       RHS.makeComplexFloat();
       RHS.FloatImag = APFloat(RHS.FloatReal.getSemantics());
     } else if (!EvaluateComplex(E->getRHS(), RHS, Info) || !LHSOK)
if (LHSTy->isAnyComplexType() || RHSTy->isAnyComplexType()) {
     }
   }
 
   if (LHSTy->isRealFloatingType() &&
       RHSTy->isRealFloatingType()) {
     APFloat RHS(0.0), LHS(0.0);
 
     bool LHSOK = EvaluateFloat(E->getRHS(), RHS, Info);
-    if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+    if (!LHSOK && !Info.noteFailure())
       return false;
 
     if (!EvaluateFloat(E->getLHS(), LHS, Info) || !LHSOK)
       return false;
 
     APFloat::cmpResult CR = LHS.compare(RHS);
 
     switch (E->getOpcode()) {
if (LHSTy->isRealFloatingType() &&
     }
   }
 
   if (LHSTy->isPointerType() && RHSTy->isPointerType()) {
     if (E->getOpcode() == BO_Sub || E->isComparisonOp()) {
       LValue LHSValue, RHSValue;
 
       bool LHSOK = EvaluatePointer(E->getLHS(), LHSValue, Info);
-      if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+      if (!LHSOK && !Info.noteFailure())
         return false;
 
       if (!EvaluatePointer(E->getRHS(), RHSValue, Info) || !LHSOK)
         return false;
 
       // Reject differing bases from the normal codepath; we special-case
       // comparisons to null.
       if (!HasSameBase(LHSValue, RHSValue)) {
bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
 
   if (LHSTy->isMemberPointerType()) {
     assert(E->isEqualityOp() && "unexpected member pointer operation");
     assert(RHSTy->isMemberPointerType() && "invalid comparison");
 
     MemberPtr LHSValue, RHSValue;
 
     bool LHSOK = EvaluateMemberPointer(E->getLHS(), LHSValue, Info);
-    if (!LHSOK && Info.keepEvaluatingAfterFailure())
+    if (!LHSOK && !Info.noteFailure())
       return false;
 
     if (!EvaluateMemberPointer(E->getRHS(), RHSValue, Info) || !LHSOK)
       return false;
 
     // C++11 [expr.eq]p2:
     //   If both operands are null, they compare equal. Otherwise if only one is
     //   null, they compare unequal.

 }
 
 bool FloatExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
   if (E->isPtrMemOp() || E->isAssignmentOp() || E->getOpcode() == BO_Comma)
     return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
 
   APFloat RHS(0.0);
   bool LHSOK = EvaluateFloat(E->getLHS(), Result, Info);
-  if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+  if (!LHSOK && !Info.noteFailure())
     return false;
   return EvaluateFloat(E->getRHS(), RHS, Info) && LHSOK &&
          handleFloatFloatBinOp(Info, E, Result, E->getOpcode(), RHS);
 }
 
 bool FloatExprEvaluator::VisitFloatingLiteral(const FloatingLiteral *E) {
   Result = E->getValue();
   return true;
if (E->getLHS()->getType()->isRealFloatingType()) {
     LHSOK = EvaluateFloat(E->getLHS(), Real, Info);
     if (LHSOK) {
       Result.makeComplexFloat();
       Result.FloatImag = APFloat(Real.getSemantics());
     }
   } else {
     LHSOK = Visit(E->getLHS());
   }
-  if (!LHSOK && !Info.keepEvaluatingAfterFailure())
+  if (!LHSOK && !Info.noteFailure())
     return false;
 
   ComplexValue RHS;
   if (E->getRHS()->getType()->isRealFloatingType()) {
     RHSReal = true;
     APFloat &Real = RHS.FloatReal;
     if (!EvaluateFloat(E->getRHS(), Real, Info) || !LHSOK)
       return false;
bool Expr::EvaluateWithSubstitution(APValue &Value, ASTContext &Ctx,
                                     ArrayRef<const Expr*> Args) const {
   Expr::EvalStatus Status;
   EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpressionUnevaluated);
 
   ArgVector ArgValues(Args.size());
   for (ArrayRef<const Expr*>::iterator I = Args.begin(), E = Args.end();
        I != E; ++I) {
     if ((*I)->isValueDependent() ||
-        !Evaluate(ArgValues[I - Args.begin()], Info, *I))
-      // If evaluation fails, throw away the argument entirely.
+        !Evaluate(ArgValues[I - Args.begin()], Info, *I)) {
+      // If evaluation fails, throw away the argument entirely, and note that
+      // something might have failed.
       ArgValues[I - Args.begin()] = APValue();
+      Info.noteFailure();
+    }
     if (Info.EvalStatus.HasSideEffects)
       return false;
   }
 
   // Build fake call to Callee.
   CallStackFrame Frame(Info, Callee->getLocation(), Callee, /*This*/nullptr,
                        ArgValues.data());
   return Evaluate(Value, Info, this) && !Info.EvalStatus.HasSideEffects;

test/SemaCXX/builtin-object-size-cxx14.cpp

+// 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}}
+}
+}

test/SemaCXX/constant-expression-cxx1y.cpp

namespace string_assign {
   }
   static_assert(!test1(1), "");
   static_assert(test1(3), "");
   static_assert(!test1(6), "");
   static_assert(test1(9), "");
   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@+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 {
   constexpr int do_stuff(int k1, int k2) {
     int arr[1234] = { 1, 2, 3, 4 };
     arr[k1] = 5; // expected-note {{past-the-end}} expected-note {{cannot refer to element 1235}} expected-note {{cannot refer to element -1}}
     return arr[k2];
   }
namespace EmptyClass {
   constexpr int test5 = f(e2, 1); // expected-error {{constant expression}} expected-note {{in call}}
   constexpr int test6 = f(e3, 1);
   constexpr int test7 = f(e1, 2);
   constexpr int test8 = f(e2, 2); // expected-error {{constant expression}} expected-note {{in call}}
   constexpr int test9 = f(e3, 2);
   constexpr int testa = f(e1, 3);
   constexpr int testb = f(e2, 3); // expected-error {{constant expression}} expected-note {{in call}}
   constexpr int testc = f(e3, 3);
 }