[clang] 474177c - [AST] Improve overflow diagnostics for fixed-point constant evaluation.
Bevin Hansson via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 26 04:42:02 PDT 2020
Author: Bevin Hansson
Date: 2020-06-26T13:38:11+02:00
New Revision: 474177c05381a6eb2971e67753481b5efc78d848
URL: https://github.com/llvm/llvm-project/commit/474177c05381a6eb2971e67753481b5efc78d848
DIFF: https://github.com/llvm/llvm-project/commit/474177c05381a6eb2971e67753481b5efc78d848.diff
LOG: [AST] Improve overflow diagnostics for fixed-point constant evaluation.
Summary:
Diagnostics for overflow were not being produced for fixed-point
evaluation. This patch refactors a bit of the evaluator and adds
a proper diagnostic for these cases.
Reviewers: rjmccall, leonardchan, bjope
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73188
Added:
Modified:
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/AST/ExprConstant.cpp
clang/test/Frontend/fixed_point_errors.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 75c4d0e0a139..905e3158cf40 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -346,6 +346,9 @@ def err_experimental_clang_interp_failed : Error<
def warn_integer_constant_overflow : Warning<
"overflow in expression; result is %0 with type %1">,
InGroup<DiagGroup<"integer-overflow">>;
+def warn_fixedpoint_constant_overflow : Warning<
+ "overflow in expression; result is %0 with type %1">,
+ InGroup<DiagGroup<"fixed-point-overflow">>;
// This is a temporary diagnostic, and shall be removed once our
// implementation is complete, and like the preceding constexpr notes belongs
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b55499fd5075..35b5bad677a0 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12881,8 +12881,14 @@ bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) {
return false;
bool Overflowed;
APFixedPoint Result = Src.convert(DestFXSema, &Overflowed);
- if (Overflowed && !HandleOverflow(Info, E, Result, DestType))
- return false;
+ if (Overflowed) {
+ if (Info.checkingForUndefinedBehavior())
+ Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
+ diag::warn_fixedpoint_constant_overflow)
+ << Result.toString() << E->getType();
+ else if (!HandleOverflow(Info, E, Result, E->getType()))
+ return false;
+ }
return Success(Result, E);
}
case CK_IntegralToFixedPoint: {
@@ -12894,8 +12900,14 @@ bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) {
APFixedPoint IntResult = APFixedPoint::getFromIntValue(
Src, Info.Ctx.getFixedPointSemantics(DestType), &Overflowed);
- if (Overflowed && !HandleOverflow(Info, E, IntResult, DestType))
- return false;
+ if (Overflowed) {
+ if (Info.checkingForUndefinedBehavior())
+ Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
+ diag::warn_fixedpoint_constant_overflow)
+ << IntResult.toString() << E->getType();
+ else if (!HandleOverflow(Info, E, IntResult, E->getType()))
+ return false;
+ }
return Success(IntResult, E);
}
@@ -12920,47 +12932,41 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
if (!EvaluateFixedPointOrInteger(RHS, RHSFX, Info))
return false;
+ bool OpOverflow = false, ConversionOverflow = false;
+ APFixedPoint Result(LHSFX.getSemantics());
switch (E->getOpcode()) {
case BO_Add: {
- bool AddOverflow, ConversionOverflow;
- APFixedPoint Result = LHSFX.add(RHSFX, &AddOverflow)
- .convert(ResultFXSema, &ConversionOverflow);
- if ((AddOverflow || ConversionOverflow) &&
- !HandleOverflow(Info, E, Result, E->getType()))
- return false;
- return Success(Result, E);
+ Result = LHSFX.add(RHSFX, &OpOverflow)
+ .convert(ResultFXSema, &ConversionOverflow);
+ break;
}
case BO_Sub: {
- bool AddOverflow, ConversionOverflow;
- APFixedPoint Result = LHSFX.sub(RHSFX, &AddOverflow)
- .convert(ResultFXSema, &ConversionOverflow);
- if ((AddOverflow || ConversionOverflow) &&
- !HandleOverflow(Info, E, Result, E->getType()))
- return false;
- return Success(Result, E);
+ Result = LHSFX.sub(RHSFX, &OpOverflow)
+ .convert(ResultFXSema, &ConversionOverflow);
+ break;
}
case BO_Mul: {
- bool AddOverflow, ConversionOverflow;
- APFixedPoint Result = LHSFX.mul(RHSFX, &AddOverflow)
- .convert(ResultFXSema, &ConversionOverflow);
- if ((AddOverflow || ConversionOverflow) &&
- !HandleOverflow(Info, E, Result, E->getType()))
- return false;
- return Success(Result, E);
+ Result = LHSFX.mul(RHSFX, &OpOverflow)
+ .convert(ResultFXSema, &ConversionOverflow);
+ break;
}
case BO_Div: {
- bool AddOverflow, ConversionOverflow;
- APFixedPoint Result = LHSFX.div(RHSFX, &AddOverflow)
- .convert(ResultFXSema, &ConversionOverflow);
- if ((AddOverflow || ConversionOverflow) &&
- !HandleOverflow(Info, E, Result, E->getType()))
- return false;
- return Success(Result, E);
+ Result = LHSFX.div(RHSFX, &OpOverflow)
+ .convert(ResultFXSema, &ConversionOverflow);
+ break;
}
default:
return false;
}
- llvm_unreachable("Should've exited before this");
+ if (OpOverflow || ConversionOverflow) {
+ if (Info.checkingForUndefinedBehavior())
+ Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
+ diag::warn_fixedpoint_constant_overflow)
+ << Result.toString() << E->getType();
+ else if (!HandleOverflow(Info, E, Result, E->getType()))
+ return false;
+ }
+ return Success(Result, E);
}
//===----------------------------------------------------------------------===//
diff --git a/clang/test/Frontend/fixed_point_errors.c b/clang/test/Frontend/fixed_point_errors.c
index 9b600fbc2642..c1d80d5daf46 100644
--- a/clang/test/Frontend/fixed_point_errors.c
+++ b/clang/test/Frontend/fixed_point_errors.c
@@ -250,3 +250,17 @@ unsigned u_const = -2.5hk; // expected-warning{{implicit conversi
char c_const = 256.0uk; // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'char'}}
short _Accum sa_const5 = 256; // expected-warning{{implicit conversion from 256 cannot fit within the range of values for 'short _Accum'}}
unsigned short _Accum usa_const2 = -2; // expected-warning{{implicit conversion from -2 cannot fit within the range of values for 'unsigned short _Accum'}}
+
+short _Accum add_ovf1 = 255.0hk + 20.0hk; // expected-warning {{overflow in expression; result is -237.0 with type 'short _Accum'}}
+short _Accum add_ovf2 = 10 + 0.5hr; // expected-warning {{overflow in expression; result is 0.5 with type 'short _Fract'}}
+short _Accum sub_ovf1 = 16.0uhk - 32.0uhk; // expected-warning {{overflow in expression; result is 240.0 with type 'unsigned short _Accum'}}
+short _Accum sub_ovf2 = -255.0hk - 20; // expected-warning {{overflow in expression; result is 237.0 with type 'short _Accum'}}
+short _Accum mul_ovf1 = 200.0uhk * 10.0uhk; // expected-warning {{overflow in expression; result is 208.0 with type 'unsigned short _Accum'}}
+short _Accum mul_ovf2 = (-0.5hr - 0.5hr) * (-0.5hr - 0.5hr); // expected-warning {{overflow in expression; result is -1.0 with type 'short _Fract'}}
+short _Accum div_ovf1 = 255.0hk / 0.5hk; // expected-warning {{overflow in expression; result is -2.0 with type 'short _Accum'}}
+
+// No warnings for saturation
+short _Fract add_sat = (_Sat short _Fract)0.5hr + 0.5hr;
+short _Accum sub_sat = (_Sat short _Accum)-200.0hk - 80.0hk;
+short _Accum mul_sat = (_Sat short _Accum)80.0hk * 10.0hk;
+short _Fract div_sat = (_Sat short _Fract)0.9hr / 0.1hr;
More information about the cfe-commits
mailing list