[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ
Thanks, I could finally draw your attention :)

> When, and why, would a regular user use this functionality?

IMO there is no reason to use evalCast without an **original** type. But I'm not sure that you can get an **original** type in every case.

About `CastRetrievedValBackdoorHackDoNotUse` I need a strong advise or some discussion. Look.
Initially `evalCast` deduces a new `SVal` using **original `QualType`** and **cast `QualType`** in case when we've got a **state** and **expression** and are able to get the **original** type. E.g.:

  ...
  
  SVal OldV = Context.getSVal(Expression);
  QualType OriginalTy = Expr->getType();
  SVal NewV = SVB.evalCast(OldV, CastTy, Expr->getType());
  ...

But `dispatchCast`, `evalCastFromNonLoc`, `evalCastFromLoc` operate using **cast `QualType`** only without knowing the **original** one.

  SVal OldV = Context.getSVal(Expression);
  SVal NewV = SVB.evalCastFromLoc(OldV.castAs<Loc>, CastTy);

Actually many cases don't need to know an exact **original** type. Some cases can extract the //type// from `SVal` (`SymbolVal`, `ConcreteInt`) Other cases need it from outside (`MemRegionVal`, `LocAsInteger`).
`dispatchCast`, `evalCastFromNonLoc`, `evalCastFromLoc` have a lot of similar code which is already in `evalCast`. They also calls inside `evalCast` function. I decided to move their functionality into a new splitted version of `evalCast` to decrease complexity of comprehension and try to substitute them in the future with the single `evalCast`. But substitusion needs to deal something with absent **original** type parameter. Then I decided to add support for`evalCast` to work in both modes. Practically, new `evaCast` has additional checks and casts when the **original** type is not null.

Now `evalCastFromNonLoc` and `evalCastFromLoc` are used in `SimpleSValBuilder` functions only. I think it would be better to add an explaination to the doc like `OriginalTy.isNull() is a CastRetrievedVal backdoor hack. Do not use it.` instead of new flags. I'll make a better description, but at first I'd try to investigate how to pass an **original** type to `evalCastFromNonLoc` and `evalCastFromLoc` in `SimpleSValBuilder` functions. Or maybe some other solution.

In sum I'll try to handle this and provide a better solution.


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

https://reviews.llvm.org/D96090