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

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:19
+public:
+  enum CastKind { Success, Fail };
+
----------------
NoQ wrote:
> I suggest `enum CastResult { Success, Failure }` ("a fail" is slang, also "result" because it's basically a result).
> 
> Also TODO: The success information should ideally go straight into the dynamic type map.
The successful cast is going to the type map, and the failure will not. Why would we need to store the failing casts in the type map? Therefore no differentiation is necessary I believe so that nor `TODO`.


================
Comment at: clang/test/Analysis/cast-value-state-dump.cpp:30
+  if (dyn_cast_or_null<Triangle>(C)) {
+    // expected-note at -1 {{Assuming dynamic cast from 'Circle' to 'Triangle' fails}}
+    // expected-note at -2 {{Taking false branch}}
----------------
NoQ wrote:
> We're not assuming it, right? We already know it's gonna fail because we already know that it's a circle.
We have no knowledge what is what. At the top we see that: `Shape -> Circle`, and then `Triangle -> Circle` is something new, so we have to assume that. It is an implementation detail we are allowing only one cast to be succeed. From the user point of view it is an assumption as the current state of the checker could not provide more/better information. It is the same logic from the `ConditionBRVisitor`.


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

https://reviews.llvm.org/D66325