[PATCH] D110357: [Analyzer] Extend ConstraintAssignor to handle remainder op

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

steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1609-1612
+      const SymExpr *LHS = Sym->getLHS();
+      const llvm::APSInt &Zero =
+          Builder.getBasicValueFactory().getValue(0, LHS->getType());
+      State = RCM->assumeSymNE(State, LHS, Zero, Zero);
----------------
ASDenysPetrov wrote:
> Maybe make some more complex assumptions to cover complex **LHS's**?
Oh nice.


================
Comment at: clang/test/Analysis/constraint-assignor.c:18
+  clang_analyzer_warnIfReached(); // no-warning
+  (void)x; // keep the constraints alive.
+}
----------------
ASDenysPetrov wrote:
> steakhal wrote:
> > It's still mindboggling that we need to do this. 
> This is needed because of `RemoveDeadBindings`.
I know, I just wanted to highlight that in contrast to this test code, on real code we would not draw the same conclusion, since we reclaimed the constraints too early. And this is what worries me.
Not in this patch in particular, but I still think that we need something better than this 'keeping symbols alive manually'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110357