[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:
9397bdc67e
...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

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
 -------------------------------------------------

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">;

clang/include/clang/Sema/Sema.h

@@ -12928,7 +12928,8 @@ private:
                           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());

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;
+  };
 
-/// 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) {
+  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;
+      }
+    }
+  }
+
+  // Match a more general floating-point equality comparison (-Wfloat-equal).
   Expr* LeftExprSansParen = LHS->IgnoreParenImpCasts();
   Expr* RightExprSansParen = RHS->IgnoreParenImpCasts();
 

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.

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}}
 }