[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:488-489
+static ProgramStateRef applyBitwiseRanges(ProgramStateRef State,
+                                          BasicValueFactory &BV,
+                                          RangeSet::Factory &F, RangeSet RS,
+                                          SymbolRef Sym) {
----------------
I recommend making this method non-static, so that not to have to pass BV and F around.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:510
+  const SymExpr *CurrentSym = SIE->getLHS();
+  if (const RangeSet *CurrentRS = Constraints.lookup(CurrentSym)) {
+    const RangeSet NewRS = assumeNonZero(BV, F, CurrentSym, *CurrentRS);
----------------
If there's no current range in the map, it doesn't mean that there's no current range at all. Instead it means that the symbol is completely unconstrained and its range is equal to the whole domain of the type's possible values. You should use `RangeConstraintManager::getRange()` instead to retrieve the "default" range for the symbol. It would also always exist, so no need to check.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:517
+    if (!NewRS.isEmpty()) {
+      State = State->set<ConstraintRange>(CurrentSym, NewRS);
+    } else {
----------------
Aaand at this point you might as well call `applyBitwiseRanges()` recursively. Or even call `assume()` recursively. This will be the better way to implement your loop idea, because now it's gonna be correct on all recursion depth levels.


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

https://reviews.llvm.org/D65239