[clang][ExprConst] Add RHS source range to div by zero diags

[cor3ntin]

clang/lib/AST/ExprConstant.cpp

2831–2832

Why not just use E->getRHS()->getSourceRange() here? It avoids an extra param

[tbaeder]

clang/lib/AST/ExprConstant.cpp

2831–2832

I thought there are cases where E is not a BinaryOperator, but that was wrong, thanks.

[aaron.ballman]

clang/lib/AST/ExprConstant.cpp

2808–2811

I think we should remove that parameter because it'd be pretty weird to pass in a binary operator and a different binary operator kind than the one used by the binary operator, right?

[cor3ntin]

Thanks!

clang/lib/AST/ExprConstant.cpp

if (Info.checkingForUndefinedBehavior())
                                        diag::warn_integer_constant_overflow)
           << toString(Result, 10) << E->getType();
     return HandleOverflow(Info, E, Value, E->getType());
   }
   return true;
 }
 
 /// Perform the given binary integer operation.
-static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
-                              BinaryOperatorKind Opcode, APSInt RHS,
-                              APSInt &Result) {
+static bool handleIntIntBinOp(EvalInfo &Info, const BinaryOperator *E,
+                              const APSInt &LHS, APSInt RHS, APSInt &Result) {
+  BinaryOperatorKind Opcode = E->getOpcode();
   bool HandleOverflowResult = true;
   switch (Opcode) {
   default:
     Info.FFDiag(E);
     return false;
   case BO_Mul:
     return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() * 2,
                                 std::multiplies<APSInt>(), Result);
   case BO_Add:
     return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
                                 std::plus<APSInt>(), Result);
   case BO_Sub:
     return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
                                 std::minus<APSInt>(), Result);
   case BO_And: Result = LHS & RHS; return true;
   case BO_Xor: Result = LHS ^ RHS; return true;
   case BO_Or:  Result = LHS | RHS; return true;
   case BO_Div:
   case BO_Rem:
     if (RHS == 0) {
-      Info.FFDiag(E, diag::note_expr_divide_by_zero);
+      Info.FFDiag(E, diag::note_expr_divide_by_zero)
+          << E->getRHS()->getSourceRange();
       return false;
     }
     // Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
     // this operation and gives the two's complement result.
     if (RHS.isNegative() && RHS.isAllOnes() && LHS.isSigned() &&
         LHS.isMinSignedValue())
       HandleOverflowResult = HandleOverflow(
           Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
if (EltTy->isIntegerType()) {
                        EltTy->isUnsignedIntegerType()};
       bool Success = true;
 
       if (BinaryOperator::isLogicalOp(Opcode))
         Success = handleLogicalOpForVector(LHSElt, Opcode, RHSElt, EltResult);
       else if (BinaryOperator::isComparisonOp(Opcode))
         Success = handleCompareOpForVector(LHSElt, Opcode, RHSElt, EltResult);
       else
-        Success = handleIntIntBinOp(Info, E, LHSElt.getInt(), Opcode,
-                                    RHSElt.getInt(), EltResult);
+        Success = handleIntIntBinOp(Info, E, LHSElt.getInt(), RHSElt.getInt(),
+                                    EltResult);
 
       if (!Success) {
         Info.FFDiag(E);
         return false;
       }
       ResultElements.emplace_back(EltResult);
 
     } else if (EltTy->isFloatingType()) {
if (!SubobjType->isIntegerType()) {
       // values.
       Info.FFDiag(E);
       return false;
     }
 
     if (RHS.isInt()) {
       APSInt LHS =
           HandleIntToIntCast(Info, E, PromotedLHSType, SubobjType, Value);
-      if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
+      if (!handleIntIntBinOp(Info, E, LHS, RHS.getInt(), LHS))
         return false;
       Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
       return true;
     } else if (RHS.isFloat()) {
       const FPOptions FPO = E->getFPFeaturesInEffect(
                                     Info.Ctx.getLangOpts());
       APFloat FValue(0.0);
       return HandleIntToFloatCast(Info, E, FPO, SubobjType, Value,
bool DataRecursiveIntBinOpEvaluator::
   if (!LHSVal.isInt() || !RHSVal.isInt())
     return Error(E);
 
   // Set up the width and signedness manually, in case it can't be deduced
   // from the operation we're performing.
   // FIXME: Don't do this in the cases where we can deduce it.
   APSInt Value(Info.Ctx.getIntWidth(E->getType()),
                E->getType()->isUnsignedIntegerOrEnumerationType());
-  if (!handleIntIntBinOp(Info, E, LHSVal.getInt(), E->getOpcode(),
-                         RHSVal.getInt(), Value))
+  if (!handleIntIntBinOp(Info, E, LHSVal.getInt(), RHSVal.getInt(), Value))
     return false;
   return Success(Value, E, Result);
 }
 
 void DataRecursiveIntBinOpEvaluator::process(EvalResult &Result) {
   Job &job = Queue.back();
 
   switch (job.Kind) {

clang/lib/AST/Interp/Interp.h

bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
   //    E1 x 2^E2 module 2^N.
   return true;
 }
 
 /// Checks if Div/Rem operation on LHS and RHS is valid.
 template <typename T>
 bool CheckDivRem(InterpState &S, CodePtr OpPC, const T &LHS, const T &RHS) {
   if (RHS.isZero()) {
-    const SourceInfo &Loc = S.Current->getSource(OpPC);
-    S.FFDiag(Loc, diag::note_expr_divide_by_zero);
+    const auto *Op = cast<BinaryOperator>(S.Current->getExpr(OpPC));
+    S.FFDiag(Op, diag::note_expr_divide_by_zero)
+        << Op->getRHS()->getSourceRange();
     return false;
   }
 
   if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
     APSInt LHSInt = LHS.toAPSInt();
     SmallString<32> Trunc;
     (-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
     const SourceInfo &Loc = S.Current->getSource(OpPC);

clang/test/Misc/constexpr-source-ranges.cpp

 // RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info -fcxx-exceptions %s 2>&1 | FileCheck %s
 // RUN: not %clang_cc1 -fsyntax-only -fexperimental-new-constant-interpreter -fdiagnostics-print-source-range-info -fcxx-exceptions %s 2>&1 | FileCheck %s
 
 constexpr int f() {
   throw 1;
   return 0;
 }
 
 // CHECK: constexpr-source-ranges.cpp:5:3:{5:3-5:10}
 
 
 constexpr int I = 12;
 constexpr const int *P = &I;
 constexpr long L = (long)P;
 // CHECK: constexpr-source-ranges.cpp:14:20:{14:20-14:27}
+
+constexpr int zero() {
+  return 0;
+}
+constexpr int divByZero() {
+  return 1 / zero();
+}
+static_assert(divByZero() == 0, "");
+/// We see this twice. Once from sema and once when
+/// evaluating the static_assert above.
+// CHECK: constexpr-source-ranges.cpp:23:15:{23:15-23:31}
+// CHECK: constexpr-source-ranges.cpp:21:12:{21:14-21:20}