[clang] 5dc9e87 - [analyzer] Improve some comments in ArrayBoundCheckerV2 (NFC) (#83545)

[via cfe-commits]

Author: NagyDonat
Date: 2024-03-04T10:37:57+01:00
New Revision: 5dc9e87c8cae7842edcaa4dd01308873109208da

URL: https://github.com/llvm/llvm-project/commit/5dc9e87c8cae7842edcaa4dd01308873109208da
DIFF: https://github.com/llvm/llvm-project/commit/5dc9e87c8cae7842edcaa4dd01308873109208da.diff

LOG:  [analyzer] Improve some comments in ArrayBoundCheckerV2 (NFC) (#83545)

This comment-only change fixes a typo, clarifies some comments and
includes some thoughts about the difficulties in resolving a certain
FIXME.

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index fdcc46e58580b4..29eb932584027d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -301,21 +301,27 @@ compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold,
   // calling `evalBinOpNN`:
   if (isNegative(SVB, State, Value) && isUnsigned(SVB, Threshold)) {
     if (CheckEquality) {
-      // negative_value == unsigned_value is always false
+      // negative_value == unsigned_threshold is always false
       return {nullptr, State};
     }
-    // negative_value < unsigned_value is always false
+    // negative_value < unsigned_threshold is always true
     return {State, nullptr};
   }
   if (isUnsigned(SVB, Value) && isNegative(SVB, State, Threshold)) {
-    // unsigned_value == negative_value and unsigned_value < negative_value are
-    // both always false
+    // unsigned_value == negative_threshold and
+    // unsigned_value < negative_threshold are both always false
     return {nullptr, State};
   }
-  // FIXME: these special cases are sufficient for handling real-world
+  // FIXME: These special cases are sufficient for handling real-world
   // comparisons, but in theory there could be contrived situations where
   // automatic conversion of a symbolic value (which can be negative and can be
   // positive) leads to incorrect results.
+  // NOTE: We NEED to use the `evalBinOpNN` call in the "common" case, because
+  // we want to ensure that assumptions coming from this precondition and
+  // assumptions coming from regular C/C++ operator calls are represented by
+  // constraints on the same symbolic expression. A solution that would
+  // evaluate these "mathematical" compariosns through a separate pathway would
+  // be a step backwards in this sense.
 
   const BinaryOperatorKind OpKind = CheckEquality ? BO_EQ : BO_LT;
   auto BelowThreshold =