[PATCH] D79336: [analyzer] Generalize bitwise OR rules for ranges

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:77
+  assert(!isEmpty());
+  // NOTE: It's a shame that we can't implement 'getMaxValue' without scanning
+  //       the whole tree to get to the last element.
----------------
Yeah, this is quite unfortunate. But you might end up calling this a lot for bitwise operations. I wonder if it is worth to solve this problem before commiting this patch. I do not insist though. 


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:388
+  /// it will return the range [x_0, y_N].
+  static Range roughen(RangeSet Origin) {
+    assert(!Origin.isEmpty());
----------------
Is `roughen` part of a nomenclature? If not, what about something like `cover`?


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:418
+
+    auto ConvertedCoarseLHS = convert(CoarseLHS, ResultType);
+    auto ConvertedCoarseRHS = convert(CoarseRHS, ResultType);
----------------
Why do we need this conversion?
Do we want to model a cast in the code somewhere?
If so, I think this is more like a workaround and in the future we would need an explicit way to represent those cast operations. It might be worth to have a TODO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79336