jyknight created this revision.
jyknight added a reviewer: rsmith.

The constant evaluation now returns false whenever indicating failure
would be appropriate for the requested mode, instead of returning
"true" for success, and depending on the caller examining the various
status variables after the fact, in some circumstances.

In EM_ConstantExpression mode, evaluation is aborted as soon as a
diagnostic note is generated, and therefore, a "true" return value now
actually indicates that the expression was a constexpr. (You no longer
have to additionally check for a lack of diagnostic messages.)

In EM_ConstantFold, evaluation is now aborted upon running into a
side-effect -- so a "true" return value now actually indicates that
there were no side-effects

Also -- update and clarify documentation for the evaluation modes.

This is a cleanup, not intending to change the functionality, as
callers had been checking those properties manually before.


https://reviews.llvm.org/D52939

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ExprConstant.cpp

Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -412,52 +412,10 @@
       MostDerivedArraySize = 2;
       MostDerivedPathLength = Entries.size();
     }
-    void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
     void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
                                    const APSInt &N);
     /// Add N to the address of this subobject.
-    void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
-      if (Invalid || !N) return;
-      uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
-      if (isMostDerivedAnUnsizedArray()) {
-        diagnoseUnsizedArrayPointerArithmetic(Info, E);
-        // Can't verify -- trust that the user is doing the right thing (or if
-        // not, trust that the caller will catch the bad behavior).
-        // FIXME: Should we reject if this overflows, at least?
-        Entries.back().ArrayIndex += TruncatedN;
-        return;
-      }
-
-      // [expr.add]p4: For the purposes of these operators, a pointer to a
-      // nonarray object behaves the same as a pointer to the first element of
-      // an array of length one with the type of the object as its element type.
-      bool IsArray = MostDerivedPathLength == Entries.size() &&
-                     MostDerivedIsArrayElement;
-      uint64_t ArrayIndex =
-          IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
-      uint64_t ArraySize =
-          IsArray ? getMostDerivedArraySize() : (uint64_t)1;
-
-      if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
-        // Calculate the actual index in a wide enough type, so we can include
-        // it in the note.
-        N = N.extend(std::max<unsigned>(N.getBitWidth() + 1, 65));
-        (llvm::APInt&)N += ArrayIndex;
-        assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
-        diagnosePointerArithmetic(Info, E, N);
-        setInvalid();
-        return;
-      }
-
-      ArrayIndex += TruncatedN;
-      assert(ArrayIndex <= ArraySize &&
-             "bounds check succeeded for out-of-bounds index");
-
-      if (IsArray)
-        Entries.back().ArrayIndex = ArrayIndex;
-      else
-        IsOnePastTheEnd = (ArrayIndex != 0);
-    }
+    bool adjustIndex(EvalInfo &Info, const Expr *E, APSInt N);
   };
 
   /// A stack frame in the constexpr call stack.
@@ -721,43 +679,68 @@
     bool IsSpeculativelyEvaluating;
 
     enum EvaluationMode {
-      /// Evaluate as a constant expression. Stop if we find that the expression
-      /// is not a constant expression.
+      /* The various EvaluationMode kinds vary in terms of what sorts of
+       * expressions they accept.
+
+         Key for the following table:
+         * D = Diagnostic notes (about non-constexpr, but still evaluable
+               constructs) are OK (keepEvaluatingAfterNote)
+         * S = Failure to evaluate which only occurs in expressions used for
+               side-effect are okay.  (keepEvaluatingAfterSideEffect)
+         * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)
+         * E = Eagerly evaluate certain builtins to a value which would normally
+               defer until after optimizations.
+
+         +-----------------------------------------+---+---+---+---+
+         |                                         | D | S | F | E |
+         +-----------------------------------------+---+---+---+---+
+         |EM_ConstantExpression                    | . | . | . | . |
+         |EM_ConstantFold                          | Y | . | . | . |
+         |EM_ConstantFoldUnevaluated               | Y | . | . | Y |
+         |EM_IgnoreSideEffects                     | Y | Y | . | . |
+         |EM_PotentialConstantExpression           | . | Y | Y | . |
+         |EM_PotentialConstantExpressionUnevaluated| . | Y | Y | Y |
+         |EM_EvaluateForOverflow                   | Y | Y | Y | . |
+         +-----------------------------------------+---+---+---+---+
+
+         TODO: Fix EM_ConstantExpression and EM_PotentialConstantExpression to
+         also eagerly evaluate. (and then delete
+         EM_PotentialConstantExpressionUnevaluated as a duplicate)
+      */
+
+      /// Evaluate as a constant expression, as per C++11-and-later constexpr
+      /// rules. 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
-      /// a constant expression.
-      EM_PotentialConstantExpression,
-
-      /// Fold the expression to a constant. Stop if we hit a side-effect that
-      /// we can't model.
+      /// Fold the expression to a constant, using any techniques we can. Stop
+      /// if we hit a side-effect that we can't model, or undefined behavior.
       EM_ConstantFold,
 
-      /// Evaluate the expression looking for integer overflow and similar
-      /// issues. Don't worry about side-effects, and try to visit all
-      /// subexpressions.
-      EM_EvaluateForOverflow,
+      /// Like EM_ConstantFold, but eagerly attempt to evaluate some builtins
+      /// that'd otherwise fail constant evaluation to defer until after
+      /// optimization.  This affects __builtin_object_size (and in the future
+      /// also __builtin_constant_p)
+      EM_ConstantFoldUnevaluated,
 
-      /// Evaluate in any way we know how. Don't worry about side-effects that
-      /// can't be modeled.
+      /// Like EM_ConstantFold, but in addition, ignores failure to evaluate
+      /// sub-expressions which are being evaluated for side-effect only.
       EM_IgnoreSideEffects,
 
-      /// Evaluate as a constant fold. Some expressions can be retried in the
-      /// optimizer if we don't constant fold them here, but in an unevaluated
-      /// context we try to fold them immediately since the optimizer never gets
-      /// a chance to look at it.
-      EM_ConstantFoldUnevaluated,
+      /// Evaluate as a potential constant expression, where the values of some
+      /// variables may be unknown. Keep going if we hit a construct that we
+      /// can't evaluate yet, but stop if we hit something that could never be a
+      /// constant expression.
+      EM_PotentialConstantExpression,
 
-      /// 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
-      /// a constant expression. Some expressions can be retried in the
-      /// optimizer if we don't constant fold them here, but in an unevaluated
-      /// context we try to fold them immediately since the optimizer never
-      /// gets a chance to look at it.
+      /// Just like EM_PotentialConstantExpression, but with the eager
+      /// evaluation behavior of EM_ConstantFoldUnevaluated.
       EM_PotentialConstantExpressionUnevaluated,
+
+      /// Evaluate the expression looking for integer overflow and similar
+      /// issues. Don't worry about side-effects, and try to visit as much of
+      /// the expression as possible.
+      EM_EvaluateForOverflow,
     } EvalMode;
 
     /// Are we checking whether the expression is a potential constant
@@ -906,8 +889,6 @@
     /// Diagnose that the evaluation does not produce a C++11 core constant
     /// expression.
     ///
-    /// FIXME: Stop evaluating if we're in EM_ConstantExpression or
-    /// EM_PotentialConstantExpression mode and we produce one of these.
     OptionalDiagnostic CCEDiag(SourceLocation Loc, diag::kind DiagId
                                  = diag::note_invalid_subexpr_in_const_expr,
                                unsigned ExtraNotes = 0) {
@@ -964,8 +945,11 @@
       return keepEvaluatingAfterSideEffect();
     }
 
-    /// Should we continue evaluation after encountering undefined behavior?
-    bool keepEvaluatingAfterUndefinedBehavior() {
+    /// Should we continue evaluation after emitting a non-fatal constexpr note?
+    /// If this returns true, then the evaluation will be successful if and only
+    /// no diagnostics are emitted. If it returns false, evaluation can succeed
+    /// with diagnostics.
+    bool keepEvaluatingAfterNote() {
       switch (EvalMode) {
       case EM_EvaluateForOverflow:
       case EM_IgnoreSideEffects:
@@ -986,7 +970,7 @@
     /// division by zero.)
     bool noteUndefinedBehavior() {
       EvalStatus.HasUndefinedBehavior = true;
-      return keepEvaluatingAfterUndefinedBehavior();
+      return keepEvaluatingAfterNote();
     }
 
     /// Should we continue evaluation as much as possible after encountering a
@@ -1182,8 +1166,7 @@
   if (Invalid)
     return false;
   if (isOnePastTheEnd()) {
-    Info.CCEDiag(E, diag::note_constexpr_past_end_subobject)
-      << CSK;
+    Info.FFDiag(E, diag::note_constexpr_past_end_subobject) << CSK;
     setInvalid();
     return false;
   }
@@ -1193,13 +1176,6 @@
   return true;
 }
 
-void SubobjectDesignator::diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info,
-                                                                const Expr *E) {
-  Info.CCEDiag(E, diag::note_constexpr_unsized_array_indexed);
-  // Do not set the designator as invalid: we can represent this situation,
-  // and correct handling of __builtin_object_size requires us to do so.
-}
-
 void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info,
                                                     const Expr *E,
                                                     const APSInt &N) {
@@ -1215,6 +1191,57 @@
   setInvalid();
 }
 
+bool SubobjectDesignator::adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
+  if (Invalid || !N)
+    return true;
+  uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
+  if (isMostDerivedAnUnsizedArray()) {
+    // Do not set the designator as invalid: we can represent this
+    // situation, and correct handling of __builtin_object_size
+    // requires us to do so.
+    Info.CCEDiag(E, diag::note_constexpr_unsized_array_indexed);
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
+    // Can't verify -- trust that the user is doing the right thing (or if
+    // not, trust that the caller will catch the bad behavior).
+    // FIXME: Should we reject if this overflows, at least?
+    Entries.back().ArrayIndex += TruncatedN;
+    return true;
+  }
+
+  // [expr.add]p4: For the purposes of these operators, a pointer to a
+  // nonarray object behaves the same as a pointer to the first element of
+  // an array of length one with the type of the object as its element type.
+  bool IsArray =
+      MostDerivedPathLength == Entries.size() && MostDerivedIsArrayElement;
+  uint64_t ArrayIndex =
+      IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
+  uint64_t ArraySize = IsArray ? getMostDerivedArraySize() : (uint64_t)1;
+
+  if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
+    // Calculate the actual index in a wide enough type, so we can include
+    // it in the note.
+    N = N.extend(std::max<unsigned>(N.getBitWidth() + 1, 65));
+    (llvm::APInt &)N += ArrayIndex;
+    assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
+    diagnosePointerArithmetic(Info, E, N);
+    setInvalid();
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
+    return true;
+  }
+
+  ArrayIndex += TruncatedN;
+  assert(ArrayIndex <= ArraySize &&
+         "bounds check succeeded for out-of-bounds index");
+
+  if (IsArray)
+    Entries.back().ArrayIndex = ArrayIndex;
+  else
+    IsOnePastTheEnd = (ArrayIndex != 0);
+  return true;
+}
+
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
                                const FunctionDecl *Callee, const LValue *This,
                                APValue *Arguments)
@@ -1387,7 +1414,7 @@
     }
 
     // Check that this LValue is not based on a null pointer. If it is, produce
-    // a diagnostic and mark the designator as invalid.
+    // a diagnostic, mark the designator as invalid, and returns false.
     bool checkNullPointer(EvalInfo &Info, const Expr *E,
                           CheckSubobjectKind CSK) {
       if (Designator.Invalid)
@@ -1408,40 +1435,51 @@
              Designator.checkSubobject(Info, E, CSK);
     }
 
-    void addDecl(EvalInfo &Info, const Expr *E,
-                 const Decl *D, bool Virtual = false) {
-      if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base))
+    bool addDecl(EvalInfo &Info, const Expr *E, const Decl *D,
+                 bool Virtual = false) {
+      if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base)) {
         Designator.addDeclUnchecked(D, Virtual);
+        return true;
+      }
+      return Info.keepEvaluatingAfterNote();
     }
-    void addUnsizedArray(EvalInfo &Info, const Expr *E, QualType ElemTy) {
+    bool addUnsizedArray(EvalInfo &Info, const Expr *E, QualType ElemTy) {
       if (!Designator.Entries.empty()) {
         Info.CCEDiag(E, diag::note_constexpr_unsupported_unsized_array);
         Designator.setInvalid();
-        return;
+        return Info.keepEvaluatingAfterNote();
       }
       if (checkSubobject(Info, E, CSK_ArrayToPointer)) {
         assert(getType(Base)->isPointerType() || getType(Base)->isArrayType());
         Designator.FirstEntryIsAnUnsizedArray = true;
         Designator.addUnsizedArrayUnchecked(ElemTy);
+        return true;
       }
+      return Info.keepEvaluatingAfterNote();
     }
-    void addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType *CAT) {
-      if (checkSubobject(Info, E, CSK_ArrayToPointer))
+    bool addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType *CAT) {
+      if (checkSubobject(Info, E, CSK_ArrayToPointer)) {
         Designator.addArrayUnchecked(CAT);
+        return true;
+      }
+      return Info.keepEvaluatingAfterNote();
     }
-    void addComplex(EvalInfo &Info, const Expr *E, QualType EltTy, bool Imag) {
-      if (checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real))
+    bool addComplex(EvalInfo &Info, const Expr *E, QualType EltTy, bool Imag) {
+      if (checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real)) {
         Designator.addComplexUnchecked(EltTy, Imag);
+        return true;
+      }
+      return Info.keepEvaluatingAfterNote();
     }
     void clearIsNullPointer() {
       IsNullPtr = false;
     }
-    void adjustOffsetAndIndex(EvalInfo &Info, const Expr *E,
+    bool adjustOffsetAndIndex(EvalInfo &Info, const Expr *E,
                               const APSInt &Index, CharUnits ElementSize) {
       // An index of 0 has no effect. (In C, adding 0 to a null pointer is UB,
       // but we're not required to diagnose it and it's valid in C++.)
       if (!Index)
-        return;
+        return true;
 
       // Compute the new offset in the appropriate width, wrapping at 64 bits.
       // FIXME: When compiling for a 32-bit target, we should use 32-bit
@@ -1451,9 +1489,16 @@
       uint64_t Index64 = Index.extOrTrunc(64).getZExtValue();
       Offset = CharUnits::fromQuantity(Offset64 + ElemSize64 * Index64);
 
-      if (checkNullPointer(Info, E, CSK_ArrayIndex))
-        Designator.adjustIndex(Info, E, Index);
+      if (!checkNullPointer(Info, E, CSK_ArrayIndex)) {
+        if (!Info.keepEvaluatingAfterNote())
+          return false;
+      }
+
+      if (!Designator.adjustIndex(Info, E, Index))
+        return false;
+
       clearIsNullPointer();
+      return true;
     }
     void adjustOffset(CharUnits N) {
       Offset += N;
@@ -1839,15 +1884,16 @@
   if (!Base) {
     // FIXME: diagnostic
     Info.CCEDiag(Loc);
-    return true;
+    return Info.keepEvaluatingAfterNote();
   }
 
   // Does this refer one past the end of some object?
   if (!Designator.Invalid && Designator.isOnePastTheEnd()) {
     const ValueDecl *VD = Base.dyn_cast<const ValueDecl*>();
-    Info.FFDiag(Loc, diag::note_constexpr_past_end, 1)
-      << !Designator.Entries.empty() << !!VD << VD;
+    Info.CCEDiag(Loc, diag::note_constexpr_past_end, 1)
+        << !Designator.Entries.empty() << !!VD << VD;
     NoteLValueLocation(Info, Base);
+    return Info.keepEvaluatingAfterNote();
   }
 
   return true;
@@ -2227,6 +2273,9 @@
       // During constant-folding, a negative shift is an opposite shift. Such
       // a shift is not a constant expression.
       Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
+
       RHS = -RHS;
       goto shift_right;
     }
@@ -2237,13 +2286,20 @@
     if (SA != RHS) {
       Info.CCEDiag(E, diag::note_constexpr_large_shift)
         << RHS << E->getType() << LHS.getBitWidth();
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
     } else if (LHS.isSigned()) {
       // C++11 [expr.shift]p2: A signed left shift must have a non-negative
       // operand, and must not overflow the corresponding unsigned type.
-      if (LHS.isNegative())
+      if (LHS.isNegative()) {
         Info.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS;
-      else if (LHS.countLeadingZeros() < SA)
+        if (!Info.keepEvaluatingAfterNote())
+          return false;
+      } else if (LHS.countLeadingZeros() < SA) {
         Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
+        if (!Info.keepEvaluatingAfterNote())
+          return false;
+      }
     }
     Result = LHS << SA;
     return true;
@@ -2258,16 +2314,22 @@
       // During constant-folding, a negative shift is an opposite shift. Such a
       // shift is not a constant expression.
       Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
+
       RHS = -RHS;
       goto shift_left;
     }
   shift_right:
     // C++11 [expr.shift]p1: Shift width must be less than the bit width of the
     // shifted type.
     unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-    if (SA != RHS)
+    if (SA != RHS) {
       Info.CCEDiag(E, diag::note_constexpr_large_shift)
         << RHS << E->getType() << LHS.getBitWidth();
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
+    }
     Result = LHS >> SA;
     return true;
   }
@@ -2353,8 +2415,7 @@
   }
 
   Obj.getLValueOffset() += RL->getBaseClassOffset(Base);
-  Obj.addDecl(Info, E, Base, /*Virtual*/ false);
-  return true;
+  return Obj.addDecl(Info, E, Base, /*Virtual*/ false);
 }
 
 static bool HandleLValueBase(EvalInfo &Info, const Expr *E, LValue &Obj,
@@ -2378,8 +2439,7 @@
   if (DerivedDecl->isInvalidDecl()) return false;
   const ASTRecordLayout &Layout = Info.Ctx.getASTRecordLayout(DerivedDecl);
   Obj.getLValueOffset() += Layout.getVBaseClassOffset(BaseDecl);
-  Obj.addDecl(Info, E, BaseDecl, /*Virtual*/ true);
-  return true;
+  return Obj.addDecl(Info, E, BaseDecl, /*Virtual*/ true);
 }
 
 static bool HandleLValueBasePath(EvalInfo &Info, const CastExpr *E,
@@ -2407,8 +2467,7 @@
 
   unsigned I = FD->getFieldIndex();
   LVal.adjustOffset(Info.Ctx.toCharUnitsFromBits(RL->getFieldOffset(I)));
-  LVal.addDecl(Info, E, FD);
-  return true;
+  return LVal.addDecl(Info, E, FD);
 }
 
 /// Update LVal to refer to the given indirect field.
@@ -2460,7 +2519,8 @@
   if (!HandleSizeof(Info, E->getExprLoc(), EltTy, SizeOfPointee))
     return false;
 
-  LVal.adjustOffsetAndIndex(Info, E, Adjustment, SizeOfPointee);
+  if (!LVal.adjustOffsetAndIndex(Info, E, Adjustment, SizeOfPointee))
+    return false;
   return true;
 }
 
@@ -2485,8 +2545,7 @@
       return false;
     LVal.Offset += SizeOfComponent;
   }
-  LVal.addComplex(Info, E, EltTy, Imag);
-  return true;
+  return LVal.addComplex(Info, E, EltTy, Imag);
 }
 
 static bool handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv,
@@ -2581,6 +2640,8 @@
                  Notes.size() + 1) << VD;
     Info.Note(VD->getLocation(), diag::note_declared_at);
     Info.addNotes(Notes);
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
   }
 
   Result = VD->getEvaluatedValue();
@@ -3204,8 +3265,12 @@
         } else {
           Info.CCEDiag(E);
         }
+        if (!Info.keepEvaluatingAfterNote())
+          return CompleteObject();
       } else if (BaseType.isConstQualified() && VD->hasDefinition(Info.Ctx)) {
         Info.CCEDiag(E, diag::note_constexpr_ltor_non_constexpr) << VD;
+        if (!Info.keepEvaluatingAfterNote())
+          return CompleteObject();
         // Keep evaluating to see what we can do.
       } else {
         // FIXME: Allow folding of values of any literal type in all languages.
@@ -3775,8 +3840,8 @@
 
   // Check this cast lands within the final derived-to-base subobject path.
   if (D.MostDerivedPathLength + E->path_size() > D.Entries.size()) {
-    Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
-      << D.MostDerivedType << TargetQT;
+    Info.FFDiag(E, diag::note_constexpr_invalid_downcast)
+        << D.MostDerivedType << TargetQT;
     return false;
   }
 
@@ -3790,8 +3855,8 @@
   else
     FinalType = getAsBaseClass(D.Entries[NewEntriesSize - 1]);
   if (FinalType->getCanonicalDecl() != TargetType->getCanonicalDecl()) {
-    Info.CCEDiag(E, diag::note_constexpr_invalid_downcast)
-      << D.MostDerivedType << TargetQT;
+    Info.FFDiag(E, diag::note_constexpr_invalid_downcast)
+        << D.MostDerivedType << TargetQT;
     return false;
   }
 
@@ -4295,6 +4360,8 @@
     } else {
       Info.CCEDiag(Loc, diag::note_invalid_subexpr_in_const_expr);
     }
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
   }
   return true;
 }
@@ -4687,10 +4754,6 @@
   typedef ConstStmtVisitor<Derived, bool> StmtVisitorTy;
   typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
 
-  OptionalDiagnostic CCEDiag(const Expr *E, diag::kind D) {
-    return Info.CCEDiag(E, D);
-  }
-
   bool ZeroInitialization(const Expr *E) { return Error(E); }
 
 public:
@@ -4744,11 +4807,15 @@
     { return StmtVisitorTy::Visit(E->getSubExpr()); }
 
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
-    CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+    Info.CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
     return static_cast<Derived*>(this)->VisitCastExpr(E);
   }
   bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
-    CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+    Info.CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
     return static_cast<Derived*>(this)->VisitCastExpr(E);
   }
 
@@ -4758,7 +4825,8 @@
       return Error(E);
 
     case BO_Comma:
-      VisitIgnoredValue(E->getLHS());
+      if (!VisitIgnoredValue(E->getLHS()))
+        return false;
       return StmtVisitorTy::Visit(E->getRHS());
 
     case BO_PtrMemD:
@@ -5094,17 +5162,17 @@
   }
 
   /// Visit a value which is evaluated, but whose value is ignored.
-  void VisitIgnoredValue(const Expr *E) {
-    EvaluateIgnoredValue(Info, E);
+  bool VisitIgnoredValue(const Expr *E) {
+    return EvaluateIgnoredValue(Info, E);
   }
 
   /// Potentially visit a MemberExpr's base expression.
-  void VisitIgnoredBaseExpression(const Expr *E) {
+  bool VisitIgnoredBaseExpression(const Expr *E) {
     // While MSVC doesn't evaluate the base expression, it does diagnose the
     // presence of side-effecting behavior.
     if (Info.getLangOpts().MSVCCompat && !E->HasSideEffects(Info.Ctx))
-      return;
-    VisitIgnoredValue(E);
+      return true;
+    return VisitIgnoredValue(E);
   }
 };
 
@@ -5286,11 +5354,13 @@
       return LValueExprEvaluatorBaseTy::VisitCastExpr(E);
 
     case CK_LValueBitCast:
-      this->CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+      Info.CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
       if (!Visit(E->getSubExpr()))
         return false;
       Result.Designator.setInvalid();
-      return true;
+      return Info.keepEvaluatingAfterNote();
 
     case CK_BaseToDerived:
       if (!Visit(E->getSubExpr()))
@@ -5484,14 +5554,16 @@
 bool LValueExprEvaluator::VisitMemberExpr(const MemberExpr *E) {
   // Handle static data members.
   if (const VarDecl *VD = dyn_cast<VarDecl>(E->getMemberDecl())) {
-    VisitIgnoredBaseExpression(E->getBase());
+    if (!VisitIgnoredBaseExpression(E->getBase()))
+      return false;
     return VisitVarDecl(E, VD);
   }
 
   // Handle static member functions.
   if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(E->getMemberDecl())) {
     if (MD->isStatic()) {
-      VisitIgnoredBaseExpression(E->getBase());
+      if (!VisitIgnoredBaseExpression(E->getBase()))
+        return false;
       return Success(MD);
     }
   }
@@ -5693,8 +5765,7 @@
   Result.setInvalid(E);
 
   QualType Pointee = E->getType()->castAs<PointerType>()->getPointeeType();
-  Result.addUnsizedArray(Info, E, Pointee);
-  return true;
+  return Result.addUnsizedArray(Info, E, Pointee);
 }
 
 namespace {
@@ -5847,10 +5918,12 @@
     if (!E->getType()->isVoidPointerType()) {
       Result.Designator.setInvalid();
       if (SubExpr->getType()->isVoidPointerType())
-        CCEDiag(E, diag::note_constexpr_invalid_cast)
-          << 3 << SubExpr->getType();
+        Info.CCEDiag(E, diag::note_constexpr_invalid_cast)
+            << 3 << SubExpr->getType();
       else
-        CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+        Info.CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
     }
     if (E->getCastKind() == CK_AddressSpaceConversion && Result.IsNullPtr)
       ZeroInitialization(E);
@@ -5877,11 +5950,14 @@
     return HandleBaseToDerivedCast(Info, E, Result);
 
   case CK_NullToPointer:
-    VisitIgnoredValue(E->getSubExpr());
+    if (!VisitIgnoredValue(E->getSubExpr()))
+      return false;
     return ZeroInitialization(E);
 
   case CK_IntegralToPointer: {
-    CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+    Info.CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
 
     APValue Value;
     if (!EvaluateIntegerOrLValue(SubExpr, Value, Info))
@@ -5916,10 +5992,9 @@
     // The result is a pointer to the first element of the array.
     auto *AT = Info.Ctx.getAsArrayType(SubExpr->getType());
     if (auto *CAT = dyn_cast<ConstantArrayType>(AT))
-      Result.addArray(Info, E, CAT);
+      return Result.addArray(Info, E, CAT);
     else
-      Result.addUnsizedArray(Info, E, AT->getElementType());
-    return true;
+      return Result.addUnsizedArray(Info, E, AT->getElementType());
   }
 
   case CK_FunctionToPointerDecay:
@@ -5987,8 +6062,7 @@
 
   Result.setInvalid(E);
   QualType PointeeTy = E->getType()->castAs<PointerType>()->getPointeeType();
-  Result.addUnsizedArray(Info, E, PointeeTy);
-  return true;
+  return Result.addUnsizedArray(Info, E, PointeeTy);
 }
 
 bool PointerExprEvaluator::VisitCallExpr(const CallExpr *E) {
@@ -6042,10 +6116,10 @@
       if (BaseAlignment < Align) {
         Result.Designator.setInvalid();
         // FIXME: Add support to Diagnostic for long / long long.
-        CCEDiag(E->getArg(0),
-                diag::note_constexpr_baa_insufficient_alignment) << 0
-          << (unsigned)BaseAlignment.getQuantity()
-          << (unsigned)Align.getQuantity();
+        Info.FFDiag(E->getArg(0),
+                    diag::note_constexpr_baa_insufficient_alignment)
+            << 0 << (unsigned)BaseAlignment.getQuantity()
+            << (unsigned)Align.getQuantity();
         return false;
       }
     }
@@ -6055,12 +6129,13 @@
       Result.Designator.setInvalid();
 
       (OffsetResult.Base
-           ? CCEDiag(E->getArg(0),
-                     diag::note_constexpr_baa_insufficient_alignment) << 1
-           : CCEDiag(E->getArg(0),
-                     diag::note_constexpr_baa_value_insufficient_alignment))
-        << (int)OffsetResult.Offset.getQuantity()
-        << (unsigned)Align.getQuantity();
+           ? Info.FFDiag(E->getArg(0),
+                         diag::note_constexpr_baa_insufficient_alignment)
+                 << 1
+           : Info.FFDiag(E->getArg(0),
+                         diag::note_constexpr_baa_value_insufficient_alignment))
+          << (int)OffsetResult.Offset.getQuantity()
+          << (unsigned)Align.getQuantity();
       return false;
     }
 
@@ -6077,6 +6152,8 @@
         << (std::string("'") + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'");
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
     LLVM_FALLTHROUGH;
   case Builtin::BI__builtin_strchr:
   case Builtin::BI__builtin_wcschr:
@@ -6163,6 +6240,8 @@
         << (std::string("'") + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'");
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
     LLVM_FALLTHROUGH;
   case Builtin::BI__builtin_memcpy:
   case Builtin::BI__builtin_memmove:
@@ -6343,7 +6422,8 @@
     return ExprEvaluatorBaseTy::VisitCastExpr(E);
 
   case CK_NullToMemberPointer:
-    VisitIgnoredValue(E->getSubExpr());
+    if (!VisitIgnoredValue(E->getSubExpr()))
+      return false;
     return ZeroInitialization(E);
 
   case CK_BaseToDerivedMemberPointer: {
@@ -6714,7 +6794,8 @@
     return false;
 
   // Get a pointer to the first element of the array.
-  Array.addArray(Info, E, ArrayType);
+  if (!Array.addArray(Info, E, ArrayType))
+    return false;
 
   // FIXME: Perform the checks on the field types in SemaInit.
   RecordDecl *Record = E->getType()->castAs<RecordType>()->getDecl();
@@ -7034,7 +7115,8 @@
 }
 
 bool VectorExprEvaluator::VisitUnaryImag(const UnaryOperator *E) {
-  VisitIgnoredValue(E->getSubExpr());
+  if (!VisitIgnoredValue(E->getSubExpr()))
+    return false;
   return ZeroInitialization(E);
 }
 
@@ -7071,7 +7153,8 @@
 
       // Zero-initialize all elements.
       LValue Subobject = This;
-      Subobject.addArray(Info, E, CAT);
+      if (!Subobject.addArray(Info, E, CAT))
+        return false;
       ImplicitValueInitExpr VIE(CAT->getElementType());
       return EvaluateInPlace(Result.getArrayFiller(), Info, Subobject, &VIE);
     }
@@ -7158,7 +7241,8 @@
   }
 
   LValue Subobject = This;
-  Subobject.addArray(Info, E, CAT);
+  if (!Subobject.addArray(Info, E, CAT))
+    return false;
   for (unsigned Index = 0; Index != NumEltsToInit; ++Index) {
     const Expr *Init =
         Index < E->getNumInits() ? E->getInit(Index) : FillerExpr;
@@ -7194,7 +7278,8 @@
   Result = APValue(APValue::UninitArray(), Elements, Elements);
 
   LValue Subobject = This;
-  Subobject.addArray(Info, E, CAT);
+  if (!Subobject.addArray(Info, E, CAT))
+    return false;
 
   bool Success = true;
   for (EvalInfo::ArrayInitLoopIndex Index(Info); Index != Elements; ++Index) {
@@ -7237,7 +7322,8 @@
 
     // Initialize the elements.
     LValue ArrayElt = Subobject;
-    ArrayElt.addArray(Info, E, CAT);
+    if (!ArrayElt.addArray(Info, E, CAT))
+      return false;
     for (unsigned I = 0; I != N; ++I)
       if (!VisitCXXConstructExpr(E, ArrayElt, &Value->getArrayInitializedElt(I),
                                  CAT->getElementType()) ||
@@ -7343,8 +7429,7 @@
   }
   bool VisitMemberExpr(const MemberExpr *E) {
     if (CheckReferencedDecl(E, E->getMemberDecl())) {
-      VisitIgnoredBaseExpression(E->getBase());
-      return true;
+      return VisitIgnoredBaseExpression(E->getBase());
     }
 
     return ExprEvaluatorBaseTy::VisitMemberExpr(E);
@@ -8254,6 +8339,8 @@
         << (std::string("'") + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'");
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
     LLVM_FALLTHROUGH;
   case Builtin::BI__builtin_strlen:
   case Builtin::BI__builtin_wcslen: {
@@ -8314,6 +8401,8 @@
         << (std::string("'") + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'");
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
     LLVM_FALLTHROUGH;
   case Builtin::BI__builtin_strcmp:
   case Builtin::BI__builtin_wcscmp:
@@ -8651,10 +8740,6 @@
     return IntEval.Error(E, D);
   }
 
-  OptionalDiagnostic CCEDiag(const Expr *E, diag::kind D) {
-    return Info.CCEDiag(E, D);
-  }
-
   // Returns true if visiting the RHS is necessary, false otherwise.
   bool VisitBinOpLHSOnly(EvalResult &LHSResult, const BinaryOperator *E,
                          bool &SuppressRHSDiags);
@@ -9072,8 +9157,11 @@
     //   operator is <= or >= and false otherwise; otherwise the result is
     //   unspecified.
     // We interpret this as applying to pointers to *cv* void.
-    if (LHSTy->isVoidPointerType() && LHSOffset != RHSOffset && IsRelational)
+    if (LHSTy->isVoidPointerType() && LHSOffset != RHSOffset && IsRelational) {
       Info.CCEDiag(E, diag::note_constexpr_void_comparison);
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
+    }
 
     // C++11 [expr.rel]p2:
     // - If two pointers point to non-static data members of the same object,
@@ -9097,22 +9185,31 @@
           Mismatch < RHSDesignator.Entries.size()) {
         const FieldDecl *LF = getAsField(LHSDesignator.Entries[Mismatch]);
         const FieldDecl *RF = getAsField(RHSDesignator.Entries[Mismatch]);
-        if (!LF && !RF)
+        if (!LF && !RF) {
           Info.CCEDiag(E, diag::note_constexpr_pointer_comparison_base_classes);
-        else if (!LF)
+          if (!Info.keepEvaluatingAfterNote())
+            return false;
+        } else if (!LF) {
           Info.CCEDiag(E, diag::note_constexpr_pointer_comparison_base_field)
               << getAsBaseClass(LHSDesignator.Entries[Mismatch])
               << RF->getParent() << RF;
-        else if (!RF)
+          if (!Info.keepEvaluatingAfterNote())
+            return false;
+        } else if (!RF) {
           Info.CCEDiag(E, diag::note_constexpr_pointer_comparison_base_field)
               << getAsBaseClass(RHSDesignator.Entries[Mismatch])
               << LF->getParent() << LF;
-        else if (!LF->getParent()->isUnion() &&
-                 LF->getAccess() != RF->getAccess())
+          if (!Info.keepEvaluatingAfterNote())
+            return false;
+        } else if (!LF->getParent()->isUnion() &&
+                   LF->getAccess() != RF->getAccess()) {
           Info.CCEDiag(E,
                        diag::note_constexpr_pointer_comparison_differing_access)
               << LF << LF->getAccess() << RF << RF->getAccess()
               << LF->getParent();
+          if (!Info.keepEvaluatingAfterNote())
+            return false;
+        }
       }
     }
 
@@ -9170,11 +9267,17 @@
     //   Otherwise if either is a pointer to a virtual member function, the
     //   result is unspecified.
     if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(LHSValue.getDecl()))
-      if (MD->isVirtual())
+      if (MD->isVirtual()) {
         Info.CCEDiag(E, diag::note_constexpr_compare_virtual_mem_ptr) << MD;
+        if (!Info.keepEvaluatingAfterNote())
+          return false;
+      }
     if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(RHSValue.getDecl()))
-      if (MD->isVirtual())
+      if (MD->isVirtual()) {
         Info.CCEDiag(E, diag::note_constexpr_compare_virtual_mem_ptr) << MD;
+        if (!Info.keepEvaluatingAfterNote())
+          return false;
+      }
 
     //   Otherwise they compare equal if and only if they would refer to the
     //   same member of the same most derived object or the same subobject if
@@ -9307,8 +9410,11 @@
     //   undefined.
     if (!LHSDesignator.Invalid && !RHSDesignator.Invalid &&
         !AreElementsOfSameArray(getType(LHSValue.Base), LHSDesignator,
-                                RHSDesignator))
+                                RHSDesignator)) {
       Info.CCEDiag(E, diag::note_constexpr_pointer_subtraction_not_same_array);
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
+    }
 
     QualType Type = E->getLHS()->getType();
     QualType ElementType = Type->getAs<PointerType>()->getPointeeType();
@@ -9608,7 +9714,9 @@
   }
 
   case CK_PointerToIntegral: {
-    CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+    Info.CCEDiag(E, diag::note_constexpr_invalid_cast) << 2;
+    if (!Info.keepEvaluatingAfterNote())
+      return false;
 
     LValue LV;
     if (!EvaluatePointer(SubExpr, LV, Info))
@@ -9682,7 +9790,8 @@
     return Success(LV.getComplexIntImag(), E);
   }
 
-  VisitIgnoredValue(E->getSubExpr());
+  if (!VisitIgnoredValue(E->getSubExpr()))
+    return false;
   return Success(0, E);
 }
 
@@ -9892,7 +10001,8 @@
     return true;
   }
 
-  VisitIgnoredValue(E->getSubExpr());
+  if (!VisitIgnoredValue(E->getSubExpr()))
+    return false;
   const llvm::fltSemantics &Sem = Info.Ctx.getFloatTypeSemantics(E->getType());
   Result = llvm::APFloat::getZero(Sem);
   return true;
@@ -10518,8 +10628,7 @@
     default:
       return ExprEvaluatorBaseTy::VisitCastExpr(E);
     case CK_ToVoid:
-      VisitIgnoredValue(E->getSubExpr());
-      return true;
+      return VisitIgnoredValue(E->getSubExpr());
     }
   }
 
@@ -10596,9 +10705,12 @@
       return false;
     Result = Value;
   } else if (T->isVoidType()) {
-    if (!Info.getLangOpts().CPlusPlus11)
+    if (!Info.getLangOpts().CPlusPlus11) {
       Info.CCEDiag(E, diag::note_constexpr_nonliteral)
         << E->getType();
+      if (!Info.keepEvaluatingAfterNote())
+        return false;
+    }
     if (!EvaluateVoid(E, Info))
       return false;
   } else if (T->isAtomicType()) {
@@ -10834,10 +10946,9 @@
          !hasUnacceptableSideEffect(Result, SEK);
 }
 
-APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx,
-                    SmallVectorImpl<PartialDiagnosticAt> *Diag) const {
+APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx) const {
   EvalResult EvalResult;
-  EvalResult.Diag = Diag;
+
   bool Result = EvaluateAsRValue(EvalResult, Ctx);
   (void)Result;
   assert(Result && "Could not evaluate expression");
@@ -11369,12 +11480,14 @@
   APValue Scratch;
   bool IsConstExpr = ::EvaluateAsRValue(Info, this, Result ? *Result : Scratch);
 
+  // Diags should always contain a message when the expression is determined to
+  // not be a constexpr, and should always be empty when the result _is_ a
+  // constant expression.  (Note: the above applies in EM_ConstantExpression
+  // mode, which we use here, but not in, e.g. EM_ConstantFold or others.)
+  assert(Diags.empty() == IsConstExpr);
+
   if (!Diags.empty()) {
-    IsConstExpr = false;
     if (Loc) *Loc = Diags[0].first;
-  } else if (!IsConstExpr) {
-    // FIXME: This shouldn't happen.
-    if (Loc) *Loc = getExprLoc();
   }
 
   return IsConstExpr;
@@ -11455,7 +11568,10 @@
     HandleFunctionCall(Loc, FD, (MD && MD->isInstance()) ? &This : nullptr,
                        Args, FD->getBody(), Info, Scratch, nullptr);
   }
-
+  // Note: the 'success' return value from the evaluation is not
+  // useful here: it's expected that it'll fail, due to argument
+  // values being missing. We just care if some diagnosable errors
+  // have been reported.
   return Diags.empty();
 }
 
@@ -11480,6 +11596,8 @@
 
   APValue ResultScratch;
   Evaluate(ResultScratch, Info, E);
+  // Same as above -- the success return value from Evaluate is not
+  // useful, we just care about diagnoses.
   return Diags.empty();
 }
 
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -631,8 +631,7 @@
   /// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
   /// integer. This must be called on an expression that constant folds to an
   /// integer.
-  llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
-                    SmallVectorImpl<PartialDiagnosticAt> *Diag = nullptr) const;
+  llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx) const;
 
   void EvaluateForOverflow(const ASTContext &Ctx) const;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D52939: E... James Y Knight via Phabricator via cfe-commits

Reply via email to