[PATCH] D31886: [analyzer] Simplify values in binary operations more aggressively

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

xazax.hun added a comment.

This is a very welcome addition. I hope the performance will be still good :)



================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1023
+
+    SVal VisitSymIntExpr(const SymIntExpr *S) {
+      SValBuilder &SVB = State->getStateManager().getSValBuilder();
----------------
We do not expect to see IntSymExpr? Maybe it is worth to document why. 


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1024
+    SVal VisitSymIntExpr(const SymIntExpr *S) {
+      SValBuilder &SVB = State->getStateManager().getSValBuilder();
+      SVal LHS, RHS;
----------------
Maybe worth to have this as a member instead of query it every time?


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1027
+      if (Loc::isLocType(S->getLHS()->getType()) &&
+          BinaryOperator::isComparisonOp(S->getOpcode())) {
+        LHS = Visit(S->getLHS());
----------------
Why are these ops special?


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1033
+      } else {
+        LHS = Visit(S->getLHS());
+        RHS = SVB.makeIntVal(S->getRHS());
----------------
Maybe this can be hoisted?


https://reviews.llvm.org/D31886