[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Mon Sep 30 13:41:02 PDT 2019

Charusso marked 3 inline comments as done.
Charusso added inline comments.

================
Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa<B>(a))
+    if (isa<C>(a))
+      clang_analyzer_warnIfReached(); // no-warning
----------------
NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Why is `(isa<B>(a) && isa<C>(a))` deemed possible in the first test but not in the second test? o_o
> > In `test_downcast()` we assume that `a` is a record type of `D` where `D` is a `B` and `D` is a `C`.  However in `test_downcast_infeasible()` if `a` is not a record type of `D` is cannot be both `B` and `C` at the same time. That is the purpose of `CastVisitor`.
> I mean, it contradicts to how the program *actually* works, so we should either not do that, or provide a reeeeeaaaaaallly compelling explanation of why we do this (as in "Extraordinary Claims Require Extraordinary Evidence").
Are you sure it does not model the program? I have an `Apple` class and I have a `Pen` class, until it is not defining an `ApplePen` class it is a false assumption to say they are defining an `ApplePen` class. I wanted to prefetch that information before the modeling starts, but it was an impossible challenge for me, so I have picked that `CastVisitor`-based post-elimination idea. In the real world I have removed only two false assumptions with the visitor from 1200 reports of LLVM so an `ApplePen` is very rare (https://www.youtube.com/watch?v=Ct6BUPvE2sM).

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

https://reviews.llvm.org/D67079