constexpr tidyups:

* Fix bug when determining whether && / || are potential constant expressions
  * Try harder when determining whether ?: is a potential constant expression
  * Produce a diagnostic on sizeof(VLA) to provide a better source location

llvm-svn: 150657

clang/include/clang/Basic/DiagnosticASTKinds.td

@@ -72,6 +72,9 @@ def note_constexpr_typeid_polymorphic : Note<
 def note_constexpr_void_comparison : Note<
   "comparison between unequal pointers to void has unspecified result">;
 def note_constexpr_temporary_here : Note<"temporary created here">;
+def note_constexpr_conditional_never_const : Note<
+  "both arms of conditional operator are unable to produce a "
+  "constant expression">;
 def note_constexpr_depth_limit_exceeded : Note<
   "constexpr evaluation exceeded maximum depth of %0 calls">;
 def note_constexpr_call_limit_exceeded : Note<

clang/lib/AST/ExprConstant.cpp

@@ -544,7 +544,8 @@ namespace {
     /// Should we continue evaluation as much as possible after encountering a
     /// construct which can't be folded?
     bool keepEvaluatingAfterFailure() {
-      return CheckingPotentialConstantExpression && EvalStatus.Diag->empty();
+      return CheckingPotentialConstantExpression &&
+             EvalStatus.Diag && EvalStatus.Diag->empty();
     }
   };
 
@@ -563,6 +564,24 @@ namespace {
         Info.EvalStatus.Diag->clear();
     }
   };
+
+  /// RAII object used to suppress diagnostics and side-effects from a
+  /// speculative evaluation.
+  class SpeculativeEvaluationRAII {
+    EvalInfo &Info;
+    Expr::EvalStatus Old;
+
+  public:
+    SpeculativeEvaluationRAII(EvalInfo &Info,
+                              llvm::SmallVectorImpl<PartialDiagnosticAt>
+                                *NewDiag = 0)
+      : Info(Info), Old(Info.EvalStatus) {
+      Info.EvalStatus.Diag = NewDiag;
+    }
+    ~SpeculativeEvaluationRAII() {
+      Info.EvalStatus = Old;
+    }
+  };
 }
 
 bool SubobjectDesignator::checkSubobject(EvalInfo &Info, const Expr *E,
@@ -1357,7 +1376,8 @@ static void HandleLValueIndirectMember(EvalInfo &Info, const Expr *E,
 }
 
 /// Get the size of the given type in char units.
-static bool HandleSizeof(EvalInfo &Info, QualType Type, CharUnits &Size) {
+static bool HandleSizeof(EvalInfo &Info, SourceLocation Loc,
+                         QualType Type, CharUnits &Size) {
   // sizeof(void), __alignof__(void), sizeof(function) = 1 as a gcc
   // extension.
   if (Type->isVoidType() || Type->isFunctionType()) {
@@ -1367,7 +1387,8 @@ static bool HandleSizeof(EvalInfo &Info, QualType Type, CharUnits &Size) {
 
   if (!Type->isConstantSizeType()) {
     // sizeof(vla) is not a constantexpr: C99 6.5.3.4p2.
-    // FIXME: Diagnostic.
+    // FIXME: Better diagnostic.
+    Info.Diag(Loc);
     return false;
   }
 
@@ -1385,7 +1406,7 @@ static bool HandleLValueArrayAdjustment(EvalInfo &Info, const Expr *E,
                                         LValue &LVal, QualType EltTy,
                                         int64_t Adjustment) {
   CharUnits SizeOfPointee;
-  if (!HandleSizeof(Info, EltTy, SizeOfPointee))
+  if (!HandleSizeof(Info, E->getExprLoc(), EltTy, SizeOfPointee))
     return false;
 
   // Compute the new offset in the appropriate width.
@@ -2330,8 +2351,9 @@ public:
   bool hasError() const { return opaqueValue == 0; }
 
   ~OpaqueValueEvaluation() {
-    // FIXME: This will not work for recursive constexpr functions using opaque
-    // values. Restore the former value.
+    // FIXME: For a recursive constexpr call, an outer stack frame might have
+    // been using this opaque value too, and will now have to re-evaluate the
+    // source expression.
     if (opaqueValue) info.OpaqueValues.erase(opaqueValue);
   }
 };
@@ -2355,6 +2377,45 @@ private:
     return static_cast<Derived*>(this)->ZeroInitialization(E);
   }
 
+  // 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);
+
+    // Speculatively evaluate both arms.
+    {
+      llvm::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)) {
+      if (Info.CheckingPotentialConstantExpression)
+        CheckPotentialConstantConditional(E);
+      return false;
+    }
+
+    Expr *EvalExpr = BoolResult ? E->getTrueExpr() : E->getFalseExpr();
+    return StmtVisitorTy::Visit(EvalExpr);
+  }
+
 protected:
   EvalInfo &Info;
   typedef ConstStmtVisitor<Derived, RetTy> StmtVisitorTy;
@@ -2437,15 +2498,12 @@ public:
   }
 
   RetTy VisitBinaryConditionalOperator(const BinaryConditionalOperator *E) {
+    // Cache the value of the common expression.
     OpaqueValueEvaluation opaque(Info, E->getOpaqueValue(), E->getCommon());
     if (opaque.hasError())
       return false;
 
-    bool cond;
-    if (!EvaluateAsBooleanCondition(E->getCond(), cond, Info))
-      return false;
-
-    return StmtVisitorTy::Visit(cond ? E->getTrueExpr() : E->getFalseExpr());
+    return HandleConditionalOperator(E);
   }
 
   RetTy VisitConditionalOperator(const ConditionalOperator *E) {
@@ -2466,12 +2524,7 @@ public:
 
     FoldConstant Fold(Info);
 
-    bool BoolResult;
-    if (!EvaluateAsBooleanCondition(E->getCond(), BoolResult, Info))
-      return false;
-
-    Expr *EvalExpr = BoolResult ? E->getTrueExpr() : E->getFalseExpr();
-    if (!StmtVisitorTy::Visit(EvalExpr))
+    if (!HandleConditionalOperator(E))
       return false;
 
     if (IsBcpCall)
@@ -4365,7 +4418,7 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
 
   if (E->isLogicalOp()) {
     // These need to be handled specially because the operands aren't
-    // necessarily integral
+    // necessarily integral nor evaluated.
     bool lhsResult, rhsResult;
 
     if (EvaluateAsBooleanCondition(E->getLHS(), lhsResult, Info)) {
@@ -4381,19 +4434,17 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
           return Success(lhsResult && rhsResult, E);
       }
     } else {
-      // FIXME: If both evaluations fail, we should produce the diagnostic from
-      // the LHS. If the LHS is non-constant and the RHS is unevaluatable, it's
-      // less clear how to diagnose this.
+      // Since we weren't able to evaluate the left hand side, it
+      // must have had side effects.
+      Info.EvalStatus.HasSideEffects = true;
+
+      // Suppress diagnostics from this arm.
+      SpeculativeEvaluationRAII Speculative(Info);
       if (EvaluateAsBooleanCondition(E->getRHS(), rhsResult, Info)) {
         // We can't evaluate the LHS; however, sometimes the result
         // is determined by the RHS: X && 0 -> 0, X || 1 -> 1.
-        if (rhsResult == (E->getOpcode() == BO_LOr)) {
-          // Since we weren't able to evaluate the left hand side, it
-          // must have had side effects.
-          Info.EvalStatus.HasSideEffects = true;
-
+        if (rhsResult == (E->getOpcode() == BO_LOr))
           return Success(rhsResult, E);
-        }
       }
     }
 
@@ -4561,7 +4612,7 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         QualType ElementType = Type->getAs<PointerType>()->getPointeeType();
 
         CharUnits ElementSize;
-        if (!HandleSizeof(Info, ElementType, ElementSize))
+        if (!HandleSizeof(Info, E->getExprLoc(), ElementType, ElementSize))
           return false;
 
         // FIXME: LLVM and GCC both compute LHSOffset - RHSOffset at runtime,
@@ -4851,8 +4902,6 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
 }
 
 CharUnits IntExprEvaluator::GetAlignOfType(QualType T) {
-  // C++ [expr.sizeof]p2: "When applied to a reference or a reference type,
-  //   the result is the size of the referenced type."
   // C++ [expr.alignof]p3: "When alignof is applied to a reference type, the
   //   result shall be the alignment of the referenced type."
   if (const ReferenceType *Ref = T->getAs<ReferenceType>())
@@ -4912,13 +4961,11 @@ bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
     QualType SrcTy = E->getTypeOfArgument();
     // C++ [expr.sizeof]p2: "When applied to a reference or a reference type,
     //   the result is the size of the referenced type."
-    // C++ [expr.alignof]p3: "When alignof is applied to a reference type, the
-    //   result shall be the alignment of the referenced type."
     if (const ReferenceType *Ref = SrcTy->getAs<ReferenceType>())
       SrcTy = Ref->getPointeeType();
 
     CharUnits Sizeof;
-    if (!HandleSizeof(Info, SrcTy, Sizeof))
+    if (!HandleSizeof(Info, E->getExprLoc(), SrcTy, Sizeof))
       return false;
     return Success(Sizeof, E);
   }

clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp

@@ -51,7 +51,7 @@ constexpr int Comma(int n) { return // expected-error {{constexpr function never
   0;
 }
 
-int ng; // expected-note 5{{here}}
+int ng; // expected-note 6{{here}}
 constexpr int BinaryOp1(int n) { return n + ng; } // expected-error {{never produces}} expected-note {{read}}
 constexpr int BinaryOp2(int n) { return ng + n; } // expected-error {{never produces}} expected-note {{read}}
 
@@ -64,13 +64,18 @@ constexpr int FunctionArgs(int a) { return Add(a, ng, a); } // expected-error {{
 
 struct S { int a; int b; int c[2]; };
 constexpr S InitList(int a) { return { a, ng }; }; // expected-error {{never produces}} expected-note {{read}}
+constexpr S InitList1a(int a) { return S{ a, ng }; }; // expected-error {{never produces}} expected-note {{read}}
 constexpr S InitList2(int a) { return { a, a, { ng } }; }; // expected-error {{never produces}} expected-note {{read}}
+constexpr S InitList3(int a) { return a ? S{ a, a } : S{ a, ng }; }; // ok
 
-constexpr S InitList3(int a) { return a ? (S){ a, a } : (S){ a, ng }; }; // ok
+constexpr int LogicalAnd1(int n) { return n && (throw, 0); } // ok
+constexpr int LogicalAnd2(int n) { return 1 && (throw, 0); } // expected-error {{never produces}} expected-note {{subexpression}}
 
-// FIXME: Check both arms of a ?: if the conditional is a potential constant
-// expression with an unknown value, and diagnose if neither is constant.
-constexpr S InitList4(int a) { return a ? (S){ a, ng } : (S){ a, ng }; };
+constexpr int LogicalOr1(int n) { return n || (throw, 0); } // ok
+constexpr int LogicalOr2(int n) { return 0 || (throw, 0); } // expected-error {{never produces}} expected-note {{subexpression}}
+
+constexpr int Conditional1(bool b, int n) { return b ? n : ng; } // ok
+constexpr int Conditional2(bool b, int n) { return b ? n * ng : n + ng; } // expected-error {{never produces}} expected-note {{both arms of conditional operator are unable to produce a constant expression}}
 
 // __builtin_constant_p ? : is magical, and is always a potential constant.
 constexpr bool BcpCall(int n) {

clang/test/SemaCXX/constant-expression-cxx11.cpp

@@ -1169,3 +1169,26 @@ constexpr const int &i = 0; // expected-error {{constant expression}} expected-n
 constexpr const int j = i; // expected-error {{constant expression}} expected-note {{initializer of 'i' is not a constant expression}}
 
 }
+
+namespace RecursiveOpaqueExpr {
+  template<typename Iter>
+  constexpr auto LastNonzero(Iter p, Iter q) -> decltype(+*p) {
+    return p != q ? (LastNonzero(p+1, q) ?: *p) : 0; // expected-warning {{GNU}}
+  }
+
+  constexpr int arr1[] = { 1, 0, 0, 3, 0, 2, 0, 4, 0, 0 };
+  static_assert(LastNonzero(begin(arr1), end(arr1)) == 4, "");
+
+  constexpr int arr2[] = { 1, 0, 0, 3, 0, 2, 0, 4, 0, 5 };
+  static_assert(LastNonzero(begin(arr2), end(arr2)) == 5, "");
+}
+
+namespace VLASizeof {
+
+  void f(int k) {
+    int arr[k]; // expected-warning {{C99}}
+    constexpr int n = 1 +
+        sizeof(arr) // expected-error {{constant expression}}
+        * 3;
+  }
+}