[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D113754#3127245 <https://reviews.llvm.org/D113754#3127245>, @steakhal wrote:

> Great stuff. Have you checked the coverage?

Thanks for the review! So, here are the coverage results for the new test file:

  1186           2 :     SVal VisitIntSymExpr(const IntSymExpr *S) {
  1187           2 :       auto I = Cached.find(S);
  1188           2 :       if (I != Cached.end())
  1189           0 :         return I->second;
  1190             :
  1191           2 :       SVal RHS = Visit(S->getRHS());
  1192           2 :       if (isUnchanged(S->getRHS(), RHS))
  1193           0 :         return skip(S);
  1194           2 :       SVal LHS = SVB.makeIntVal(S->getLHS());
  1195             :       return cache(
  1196           2 :           S, SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()));
  1197             :     }

L1189 is not covered, but that is related to the caching mechanism, which is already existing and this patch is independent from that. We have the very same caching implementation in all the other `Visit` functions.
L1193 There is an existing test case in `find-binop-constraints.cpp` which covers this line. :

  int test_lhs_and_rhs_further_constrained(int x, int y) {
    if (x % y != 1)
      return 0;
    if (x != 1)
      return 0;
    if (y != 2)
      return 0;
    clang_analyzer_eval(x % y == 1); // expected-warning{{TRUE}}
    clang_analyzer_eval(y == 2);     // expected-warning{{TRUE}}
    return 0;
  }

I could have replicated this test case here, but hadn't been able to formulate any new expectations with a `clang_analyzer_` function. So there is no visible functional change in this case with this patch concerning this line. Besides, all other existing `Visit` functions have the same optimization and they do an early return with `skip` if the symbol is unchanged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113754