[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I still need to chew through the code but on a high level, I think it looks correct.
PS: the test coverage is outstanding! F19575968: unite-patch-line-coverage.zip <https://reviews.llvm.org/F19575968>



================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:149
+
+RangeSet RangeSet::Factory::unite(RangeSet Original, llvm::APSInt Point) {
+  return unite(Original, Range(ValueFactory.getValue(Point)));
----------------
Why do you take `APSInt`s by value? Generally, we take them by reference.


================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:81
   const llvm::APSInt &from(BaseType X) {
-    llvm::APSInt Dummy = Base;
-    Dummy = X;
-    return BVF.getValue(Dummy);
+    static llvm::APSInt Base{sizeof(BaseType) * 8,
+                             std::is_unsigned<BaseType>::value};
----------------
Shouldn't you use `sizeof(BaseType) * CHAR_BIT` instead?


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

https://reviews.llvm.org/D99797