[clang] ab982ea - [Sema] add warning for tautological FP compare with literal
Sanjay Patel via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 17 05:25:42 PDT 2022
Author: Sanjay Patel
Date: 2022-03-17T08:22:30-04:00
New Revision: ab982eace6e4951a2986567d29f4d6be002c1ba7
URL: https://github.com/llvm/llvm-project/commit/ab982eace6e4951a2986567d29f4d6be002c1ba7
DIFF: https://github.com/llvm/llvm-project/commit/ab982eace6e4951a2986567d29f4d6be002c1ba7.diff
LOG: [Sema] add warning for tautological FP compare with literal
If we are equality comparing an FP literal with a value cast from a type
where the literal can't be represented, that's known true or false and
probably a programmer error.
Fixes issue #54222.
https://github.com/llvm/llvm-project/issues/54222
Note - I added the optimizer change with:
9397bdc67eb2
...and as discussed in the post-commit comments, that transform might be
too dangerous without this warning in place, so it was reverted to allow
this change first.
Differential Revision: https://reviews.llvm.org/D121306
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/floating-point-compare.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a0e3cabe89a9a..d457be1305cf7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -64,6 +64,9 @@ Bug Fixes
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- ``-Wliteral-range`` will warn on floating-point equality comparisons with
+ constants that are not representable in a casted value. For example,
+ ``(float) f == 0.1`` is always false.
Non-comprehensive list of changes in this release
-------------------------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index eda47588680f4..e6b11c943a705 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -118,6 +118,10 @@ def warn_float_overflow : Warning<
def warn_float_underflow : Warning<
"magnitude of floating-point constant too small for type %0; minimum is %1">,
InGroup<LiteralRange>;
+def warn_float_compare_literal : Warning<
+ "floating-point comparison is always %select{true|false}0; "
+ "constant cannot be represented exactly in type %1">,
+ InGroup<LiteralRange>;
def warn_double_const_requires_fp64 : Warning<
"double precision constant requires %select{cl_khr_fp64|cl_khr_fp64 and __opencl_c_fp64}0, "
"casting to single precision">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e8b9bc2d7990c..fe8a1f371fe74 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12928,7 +12928,8 @@ class Sema final {
const FunctionDecl *FD = nullptr);
public:
- void CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS);
+ void CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS,
+ BinaryOperatorKind Opcode);
private:
void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d14019cdbf18..2d2250771eb6e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11436,12 +11436,40 @@ Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
CheckPPCMMAType(RetValExp->getType(), ReturnLoc);
}
-//===--- CHECK: Floating-Point comparisons (-Wfloat-equal) ---------------===//
+/// Check for comparisons of floating-point values using == and !=. Issue a
+/// warning if the comparison is not likely to do what the programmer intended.
+void Sema::CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS,
+ BinaryOperatorKind Opcode) {
+ // Match and capture subexpressions such as "(float) X == 0.1".
+ FloatingLiteral *FPLiteral;
+ CastExpr *FPCast;
+ auto getCastAndLiteral = [&FPLiteral, &FPCast](Expr *L, Expr *R) {
+ FPLiteral = dyn_cast<FloatingLiteral>(L->IgnoreParens());
+ FPCast = dyn_cast<CastExpr>(R->IgnoreParens());
+ return FPLiteral && FPCast;
+ };
+
+ if (getCastAndLiteral(LHS, RHS) || getCastAndLiteral(RHS, LHS)) {
+ auto *SourceTy = FPCast->getSubExpr()->getType()->getAs<BuiltinType>();
+ auto *TargetTy = FPLiteral->getType()->getAs<BuiltinType>();
+ if (SourceTy && TargetTy && SourceTy->isFloatingPoint() &&
+ TargetTy->isFloatingPoint()) {
+ bool Lossy;
+ llvm::APFloat TargetC = FPLiteral->getValue();
+ TargetC.convert(Context.getFloatTypeSemantics(QualType(SourceTy, 0)),
+ llvm::APFloat::rmNearestTiesToEven, &Lossy);
+ if (Lossy) {
+ // If the literal cannot be represented in the source type, then a
+ // check for == is always false and check for != is always true.
+ Diag(Loc, diag::warn_float_compare_literal)
+ << (Opcode == BO_EQ) << QualType(SourceTy, 0)
+ << LHS->getSourceRange() << RHS->getSourceRange();
+ return;
+ }
+ }
+ }
-/// Check for comparisons of floating point operands using != and ==.
-/// Issue a warning if these are no self-comparisons, as they are not likely
-/// to do what the programmer intended.
-void Sema::CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr *RHS) {
+ // Match a more general floating-point equality comparison (-Wfloat-equal).
Expr* LeftExprSansParen = LHS->IgnoreParenImpCasts();
Expr* RightExprSansParen = RHS->IgnoreParenImpCasts();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ab8bbb69d9dbb..635721b4cc3b3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -12022,7 +12022,7 @@ static QualType checkArithmeticOrEnumeralCompare(Sema &S, ExprResult &LHS,
// Check for comparisons of floating point operands using != and ==.
if (Type->hasFloatingRepresentation() && BinaryOperator::isEqualityOp(Opc))
- S.CheckFloatComparison(Loc, LHS.get(), RHS.get());
+ S.CheckFloatComparison(Loc, LHS.get(), RHS.get(), Opc);
// The result of comparisons is 'bool' in C++, 'int' in C.
return S.Context.getLogicalOperationType();
@@ -12618,7 +12618,7 @@ QualType Sema::CheckVectorCompareOperands(ExprResult &LHS, ExprResult &RHS,
if (BinaryOperator::isEqualityOp(Opc) &&
LHSType->hasFloatingRepresentation()) {
assert(RHS.get()->getType()->hasFloatingRepresentation());
- CheckFloatComparison(Loc, LHS.get(), RHS.get());
+ CheckFloatComparison(Loc, LHS.get(), RHS.get(), Opc);
}
// Return a signed type for the vector.
diff --git a/clang/test/Sema/floating-point-compare.c b/clang/test/Sema/floating-point-compare.c
index 60f971c6df97f..4ccfae203f9d1 100644
--- a/clang/test/Sema/floating-point-compare.c
+++ b/clang/test/Sema/floating-point-compare.c
@@ -12,6 +12,7 @@ int f3(float x) {
return x == x; // no-warning
}
+// 0.0 can be represented exactly, so don't warn.
int f4(float x) {
return x == 0.0; // no-warning {{comparing}}
}
@@ -20,6 +21,43 @@ int f5(float x) {
return x == __builtin_inf(); // no-warning
}
-int f7(float x) {
- return x == 3.14159; // expected-warning {{comparing}}
+// The literal is a double that can't be represented losslessly as a float.
+int tautological_FP_compare(float x) {
+ return x == 3.14159; // expected-warning {{floating-point comparison is always false}}
+}
+
+int tautological_FP_compare_inverse(float x) {
+ return x != 3.14159; // expected-warning {{floating-point comparison is always true}}
+}
+
+// The literal is a double that can be represented losslessly as a long double,
+// but this might not be the comparison what was intended.
+int not_tautological_FP_compare(long double f) {
+ return f == 0.1; // expected-warning {{comparing floating point with ==}}
+}
+
+int tautological_FP_compare_commute(float f) {
+ return 0.3 == f; // expected-warning {{floating-point comparison is always false}}
+}
+
+int tautological_FP_compare_commute_inverse(float f) {
+ return 0.3 != f; // expected-warning {{floating-point comparison is always true}}
+}
+
+int tautological_FP_compare_explicit_upcast(float f) {
+ return 0.3 == (double) f; // expected-warning {{floating-point comparison is always false}}
+}
+
+int tautological_FP_compare_explicit_downcast(double d) {
+ return 0.3 == (float) d; // expected-warning {{floating-point comparison is always false}}
+}
+
+int tautological_FP_compare_ignore_parens(float f) {
+ return f == (0.3); // expected-warning {{floating-point comparison is always false}}
+}
+
+#define CST 0.3
+
+int tautological_FP_compare_macro(float f) {
+ return f == CST; // expected-warning {{floating-point comparison is always false}}
}
More information about the cfe-commits
mailing list