[PATCH] D104550: [analyzer] Implement getType for SVal

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D104550#2838845 <https://reviews.llvm.org/D104550#2838845>, @vsavchenko wrote:

>> Another thing is that we can garantee returning `QualType`. I mean, we can replace `Optional` with `QualType` itself. `QualType` has a default ctor and `isNull` predicate, which is //true// when defaultly constructed.
>
> I wanted to be more explicit here and convey to the user that the lack of type is normal.  I can change that, it won't be a problem at all.  @NoQ, @Szelethus what do you think?



  // May return nullptr.

And its all good!

In D104550#2835398 <https://reviews.llvm.org/D104550#2835398>, @NoQ wrote:

> Another thing to mention about `SVal::getType()` in its doxygen comment is that //most of the time you don't need it//. Similarly to how checking whether an `SVal` is a `Loc` or a `NonLoc` results in incorrect code 95% of the time (because such code is unable to discriminate between arbitrary lvalues and pointer rvalues which is typically crucial in order to get everything right but often forgotten / misunderstood), relying on the type of the `SVal` rather than its storage raises similar problems.

Then I especially can't imagine a more important doxygen comment then this! We could even consider adding your point about `Loc`s and `NonLoc`s to `SVal::getAs`. The target audience are new to clang, static analysis, and C++ itself. I recall you had to beat some of this stuff into me for weeks when I implemented pointer chasing in `UninitializedObjectChecker` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104550