[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Gabor Marton via Phabricator via cfe-commits]

martong marked an inline comment as done.
martong added a comment.

In D126481#3542919 <https://reviews.llvm.org/D126481#3542919>, @ASDenysPetrov wrote:

> @martong As you said, my solution D103096 <https://reviews.llvm.org/D103096> suppose to pass these and more other tests cases. So how it will help in combination with my solution D103096 <https://reviews.llvm.org/D103096>? 
> Although, your patch is really simple but it's more like a plug then a real `SymbolCast ` support, isn't it? I don't quite understand the motivation.

This part of the SValBuilder is responsible for **constant folding**. We need this constant folding, so the engine can work with less symbols, this way it can be more efficient. Whenever a symbol is constrained with a constant then we substitute the symbol with the corresponding integer. If a symbol is constrained with a range, then the symbol is kept and we fall-back to use the range based constraint manager, which is not that efficient. This patch is the natural extension of the existing constant folding machinery with the support of `SymbolCast` symbols.



================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1354
+        return I->second;
+      const SymExpr *OpSym = S->getOperand();
+      SVal OpVal = getConstOrVisit(OpSym);
----------------
ASDenysPetrov wrote:
> Should this imply to use the root symbol and not the second nested one?
> E.g. from `(int)(short)(x)` do you want `(short)(x)` or `(x)`?
> `getOperand` gives you `(short)(x)` in this case.
We need the operand, with your words it is the second nested one, not the root. In case of `(int)(short)(x)` we need the `(short)(x)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481