[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I'm really sorry about being sooo picky about this patch.
It's not my expertise and the change seems to address a corner-case, so we have to especially careful not introducing bugs.

My concern is that I still don't understand why do we want to do anything with reinterpret casts, besides remembering what the original type was.
AFAIK the resulting pointer can not be safely dereferenced unless it's reinterpreted back to the original type.
But for example, you are expecting this:

  void testMultiple() {
    int F::*f = &F::field;
    int A::*a = reinterpret_cast<int A::*>(f); // Dereferencing 'a' is UB, AFAIK
    int C::*c = reinterpret_cast<int C::*>(f); // Same here!
    A aobj;
    C cobj;
    aobj.*a = 13; // Now an airplane suddenly crashes.
    cobj.field = 29;
    clang_analyzer_eval(aobj.*a == 13); // expected-warning{{UNKNOWN}} // Same here!
    clang_analyzer_eval(cobj.*c == 29); // expected-warning{{TRUE}} // Same here!
  }

IMO if an expression results in UB, the symbolic value associated with that expression should be `Undef`.
`Unknown` would be also fine, but nothing else.

I could spam a couple of nits, but I think it's better to sort this question out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96976