[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

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

steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:372
                                   QualType resultTy)  {
   NonLoc InputLHS = lhs;
   NonLoc InputRHS = rhs;
----------------
martong wrote:
> steakhal wrote:
> > @martong, you simplified the operands, but you overwrite only the lhs and rhs. Shouldnt you overwite these as well?
> > What is the purpose of these variables, do you know?
> > @martong, you simplified the operands, but you overwrite only the lhs and rhs. Shouldnt you overwite these as well?
> > What is the purpose of these variables, do you know?
> 
> They are used exclusively to create `SymExpr`s with `makeSymExprValNN` when both lhs and rhs are symbols. If we were to simplify the `Input` variables then it might happen that we end up with a non symbol (i.e. a concrete int) and then `makeSymExprValNN` would fail miserably (would assert I guess). 
I see. Although, it still seems suboptimal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113753