This revision was automatically updated to reflect the committed changes.
Closed by commit rGab982eace6e4: [Sema] add warning for tautological FP compare
with literal (authored by spatel).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121306/new/
https://reviews.llvm.org/D121306
Files:
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
Index: clang/test/Sema/floating-point-compare.c
===================================================================
--- clang/test/Sema/floating-point-compare.c
+++ clang/test/Sema/floating-point-compare.c
@@ -12,6 +12,7 @@
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 @@
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}}
}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -12022,7 +12022,7 @@
// 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 @@
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.
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11436,12 +11436,40 @@
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();
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12928,7 +12928,8 @@
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());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -118,6 +118,10 @@
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">;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -64,6 +64,9 @@
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
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
