[clang] [clang][dataflow] Make optional checker work for types derived from optional. (PR #84138)

[via cfe-commits]

martinboehme wrote:

> One high level question: is there a simpler path by which we avoid crashing on derived types but also don't support them? It seems like there is a lot of complexity required in getting this just right. Is it worth it (both in the code itself and in terms of runtime cost) to support these unusual cases? Of course, if it's necessary then the point is moot...

This is a good question. tl;dr: I think actually handling these cases instead of just ignoring them is the right tradeoff, but the situation certainly isn't clear cut.

It's _possible_ to avoid crashing on derived types but otherwise not support them. What we would need to do is call `RecordStorageLocation::getType()` on the storage location of the object that we're accessing; we would only proceed if we see an actual optional type (not a derived type). The thing is though that we would need to do this in _every_ `transfer_...` handler; this isn't ideal because it's easy to forget one, and then we get crashes there. I considered this option, but it seemed to me that the more principled and less error-prone approach is to actually treat derived types correctly.

As I say, the tradeoff isn't clear-cut. To summarize:

**Bail out but don't crash**
Pros:
*  Less conceptual complexity...
Cons:
*  ...but need a check in every handler (i.e. still lots of lines of code).
* If we forget to add the check everywhere, we get crashes.

**Correctly handle derived types**
Pros:
*  The additional code is centralized in one place.
*  Actually makes the check more expressive
Cons:
*  We need to walk the inheritance hierarchy and chains of casts, which adds complexity and potentially makes the check slower.

Will respond to the individual comments too, but probably not before Monday.

https://github.com/llvm/llvm-project/pull/84138