[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Mon Aug 19 15:37:57 PDT 2019

Charusso added inline comments.

================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)

----------------
NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Emm, so you're saving successes and failures of all casts //regardless of which object is getting casted//? That's definitely not sufficient. If `X` is a `Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is a `Stmt` that isn't an `Expr` for any object `Y` other than `X` . This information needs to be per-object. Then you'll need to clean it up in `checkDeadSymbols`.
> > I have two implementations, the other is set factory based on memory regions. Is it would be a good idea to go through all the regions every time and all their casts to know every possible cast-assumption? I have made it, and I felt like it is overcomplicated as the DynamicTypeMap could be used for asking what is the type now and whether we have better precision and update that information therefore we could update the cast-set as well.
> It's about correctness, we don't have much choice here. The current data structure simply cannot work correctly because there's a "one vs many" problem in it: for every pair of types (T₁, T₂) there are *many* possible outcomes of a cast from T₁ to T₂ (depending on the object that's being casted) but the current data structure only has room for *one* such outcome. Your data structure is basically saying "Oh, this shape turned out to be a circle, let's from now on forever believe that triangles don't exist" (?)
Well, let me swap then with the set factory. I think I could handle that in the current shape, but I cannot say no to your clear idea. Thanks for the factory idea! It does the same at the moment.

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

https://reviews.llvm.org/D66325