[clang] [Clang] Improve ``-Wtautological-overlap-compare`` diagnostics flag (PR #133653)

[via cfe-commits]

================
@@ -1170,82 +1171,117 @@ class CFGBuilder {
     if (!areExprTypesCompatible(NumExpr1, NumExpr2))
       return {};
 
+    // Check that the two expressions are of the same type.
     Expr::EvalResult L1Result, L2Result;
-    if (!NumExpr1->EvaluateAsInt(L1Result, *Context) ||
-        !NumExpr2->EvaluateAsInt(L2Result, *Context))
-      return {};
-
-    llvm::APSInt L1 = L1Result.Val.getInt();
-    llvm::APSInt L2 = L2Result.Val.getInt();
-
-    // Can't compare signed with unsigned or with different bit width.
-    if (L1.isSigned() != L2.isSigned() || L1.getBitWidth() != L2.getBitWidth())
+    if (!NumExpr1->EvaluateAsRValue(L1Result, *Context) ||
+        !NumExpr2->EvaluateAsRValue(L2Result, *Context))
       return {};
 
-    // Values that will be used to determine if result of logical
-    // operator is always true/false
-    const llvm::APSInt Values[] = {
-      // Value less than both Value1 and Value2
-      llvm::APSInt::getMinValue(L1.getBitWidth(), L1.isUnsigned()),
-      // L1
-      L1,
-      // Value between Value1 and Value2
-      ((L1 < L2) ? L1 : L2) + llvm::APSInt(llvm::APInt(L1.getBitWidth(), 1),
-                              L1.isUnsigned()),
-      // L2
-      L2,
-      // Value greater than both Value1 and Value2
-      llvm::APSInt::getMaxValue(L1.getBitWidth(), L1.isUnsigned()),
-    };
-
-    // Check whether expression is always true/false by evaluating the following
+    // Check whether expression is always true/false by evaluating the
+    // following
     // * variable x is less than the smallest literal.
     // * variable x is equal to the smallest literal.
     // * Variable x is between smallest and largest literal.
     // * Variable x is equal to the largest literal.
     // * Variable x is greater than largest literal.
-    bool AlwaysTrue = true, AlwaysFalse = true;
-    // Track value of both subexpressions.  If either side is always
-    // true/false, another warning should have already been emitted.
-    bool LHSAlwaysTrue = true, LHSAlwaysFalse = true;
-    bool RHSAlwaysTrue = true, RHSAlwaysFalse = true;
-    for (const llvm::APSInt &Value : Values) {
-      TryResult Res1, Res2;
-      Res1 = analyzeLogicOperatorCondition(BO1, Value, L1);
-      Res2 = analyzeLogicOperatorCondition(BO2, Value, L2);
-
-      if (!Res1.isKnown() || !Res2.isKnown())
-        return {};
+    auto analyzeConditions = [&](const auto &Values,
+                                 const BinaryOperatorKind *BO1,
+                                 const BinaryOperatorKind *BO2) -> TryResult {
+      bool AlwaysTrue = true, AlwaysFalse = true;
+      // Track value of both subexpressions.  If either side is always
+      // true/false, another warning should have already been emitted.
+      bool LHSAlwaysTrue = true, LHSAlwaysFalse = true;
+      bool RHSAlwaysTrue = true, RHSAlwaysFalse = true;
+
+      for (const auto &Value : Values) {
+        TryResult Res1 =
+            analyzeLogicOperatorCondition(*BO1, Value, Values[1] /* L1 */);
+        TryResult Res2 =
+            analyzeLogicOperatorCondition(*BO2, Value, Values[3] /* L2 */);
+
+        if (!Res1.isKnown() || !Res2.isKnown())
+          return {};
+
+        const bool isAnd = B->getOpcode() == BO_LAnd;
+        const bool combine = isAnd ? (Res1.isTrue() && Res2.isTrue())
+                                   : (Res1.isTrue() || Res2.isTrue());
+
+        AlwaysTrue &= combine;
+        AlwaysFalse &= !combine;
+
+        LHSAlwaysTrue &= Res1.isTrue();
+        LHSAlwaysFalse &= Res1.isFalse();
+        RHSAlwaysTrue &= Res2.isTrue();
+        RHSAlwaysFalse &= Res2.isFalse();
+      }
 
-      if (B->getOpcode() == BO_LAnd) {
-        AlwaysTrue &= (Res1.isTrue() && Res2.isTrue());
-        AlwaysFalse &= !(Res1.isTrue() && Res2.isTrue());
-      } else {
-        AlwaysTrue &= (Res1.isTrue() || Res2.isTrue());
-        AlwaysFalse &= !(Res1.isTrue() || Res2.isTrue());
+      if (AlwaysTrue || AlwaysFalse) {
+        if (!LHSAlwaysTrue && !LHSAlwaysFalse && !RHSAlwaysTrue &&
+            !RHSAlwaysFalse && BuildOpts.Observer) {
+          BuildOpts.Observer->compareAlwaysTrue(B, AlwaysTrue);
+        }
+        return TryResult(AlwaysTrue);
       }
+      return {};
+    };
 
-      LHSAlwaysTrue &= Res1.isTrue();
-      LHSAlwaysFalse &= Res1.isFalse();
-      RHSAlwaysTrue &= Res2.isTrue();
-      RHSAlwaysFalse &= Res2.isFalse();
+    // Handle integer comparison
+    if (L1Result.Val.getKind() == APValue::Int &&
+        L2Result.Val.getKind() == APValue::Int) {
+      llvm::APSInt L1 = L1Result.Val.getInt();
+      llvm::APSInt L2 = L2Result.Val.getInt();
+
+      // Can't compare signed with unsigned or with different bit width.
+      if (L1.isSigned() != L2.isSigned() ||
+          L1.getBitWidth() != L2.getBitWidth())
+        return {};
+
+      // Values that will be used to determine if result of logical
+      // operator is always true/false
+      const llvm::APSInt Values[] = {
+          // Value less than both Value1 and Value2
+          llvm::APSInt::getMinValue(L1.getBitWidth(), L1.isUnsigned()),
+          // L1
+          L1,
+          // Value between Value1 and Value2
+          ((L1 < L2) ? L1 : L2) +
+              llvm::APSInt(llvm::APInt(L1.getBitWidth(), 1), L1.isUnsigned()),
+          // L2
+          L2,
+          // Value greater than both Value1 and Value2
+          llvm::APSInt::getMaxValue(L1.getBitWidth(), L1.isUnsigned()),
+      };
+
+      return analyzeConditions(Values, &BO1, &BO2);
     }
 
-    if (AlwaysTrue || AlwaysFalse) {
-      if (!LHSAlwaysTrue && !LHSAlwaysFalse && !RHSAlwaysTrue &&
-          !RHSAlwaysFalse && BuildOpts.Observer)
-        BuildOpts.Observer->compareAlwaysTrue(B, AlwaysTrue);
-      return TryResult(AlwaysTrue);
+    // Handle float comparison
+    if (L1Result.Val.getKind() == APValue::Float &&
+        L2Result.Val.getKind() == APValue::Float) {
+      llvm::APFloat L1 = L1Result.Val.getFloat();
+      llvm::APFloat L2 = L2Result.Val.getFloat();
+      llvm::APFloat MidValue = L1;
+      MidValue.add(L2, llvm::APFloat::rmNearestTiesToEven);
+      MidValue.divide(llvm::APFloat(MidValue.getSemantics(), "2.0"),
+                      llvm::APFloat::rmNearestTiesToEven);
+
+      const llvm::APFloat Values[] = {
+          llvm::APFloat::getSmallest(L1.getSemantics(), true), L1, MidValue, L2,
+          llvm::APFloat::getLargest(L2.getSemantics(), false),
+      };
----------------
Sirraide wrote:

```suggestion
      const llvm::APFloat Values[] = {
          llvm::APFloat::getSmallest(L1.getSemantics(), true), 
          L1, 
          MidValue, 
          L2,
          llvm::APFloat::getLargest(L2.getSemantics(), false),
      };
```
This is a bit easier to read imo (unless clang-format doesn’t like this for some reason).

https://github.com/llvm/llvm-project/pull/133653