ASDenysPetrov added a comment.

Here are my remarks.



================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1415-1416
+
+  Range CoarseLHS = fillGaps(LHS);
+  Range CoarseRHS = fillGaps(RHS);
+
----------------
Avoid filling the gaps, because it's completely possible to compare all the 
ranges.
E.g. //LHS //`[1,2]U[8,9]`  and  //RHS //`[5,6]` are not equal.
And you don't need to fiil the gap in LHS, because it will lead you to a wrong 
answer, like `[1,9] != [5,6]` => **UNKNOWN** instead of **TRUE**.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1420-1421
+
+  auto ConvertedCoarseLHS = convert(CoarseLHS, ResultType);
+  auto ConvertedCoarseRHS = convert(CoarseRHS, ResultType);
+
----------------
The `ResultType` of `BO_NE` is `bool`. You don't need to convert your **integer 
**ranges to **boolean**. Otherwise, you'll lose the information to compare. 


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1430-1433
+  if ((ConvertedCoarseLHS->To() < ConvertedCoarseRHS->From()) ||
+      (ConvertedCoarseLHS->From() > ConvertedCoarseRHS->To())) {
+    return getTrueRange(T);
+  }
----------------
You can simply check an intersection (`RangeSet::Factory::intersect`) between 
the ranges.
If the result range is //empty// then return TRUE.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1437-1441
+  if (ConvertedCoarseLHS->getConcreteValue() &&
+      ConvertedCoarseLHS->getConcreteValue() ==
+          ConvertedCoarseRHS->getConcreteValue()) {
+    return getFalseRange(T);
+  }
----------------
This is OK but check `ConvertedCoarseRHS->getConcreteValue()` for `nullptr` 
before getting the value.



================
Comment at: clang/test/Analysis/constant-folding.c:339
+  }
+}
----------------
I'd like to see the cases with concrete integers as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112621/new/

https://reviews.llvm.org/D112621

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to