[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I'm catching up and these changes look great.

In D90157#2433657 <https://reviews.llvm.org/D90157#2433657>, @steakhal wrote:

> I've run the baseline and your patch as well on 15 projects, both with and without Z3 refutation enabled.
> (...)
> All in all, I'm still in favor of this change, but I'm really curious why did we observe such changes. Debugging the cause seems really tricky to me.

If testing on a large codebase uncovered changes in behavior that weren't caught by our LIT test suite it often make sense to update our LIT test suite to include those tests in order to avoid similar regressions in the future. Regressions like this are easy to auto-reduce with tools like `creduce` because there's no functional change intended in the patch so there's a 100% formal reduction criterion "any change in diagnostics is observed" which can be easily fed to `creduce`. Even though NFC commits don't require tests, this doesn't mean they shouldn't add them!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90157