NoQ 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 ---------------- Charusso wrote: > 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). I'm sure that the possibility of taking a true branch in `if (x)` only depends on the value of `x`, not on the contents of the branch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
