[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51
+
+  const CallDescriptionMap<CastCheck> SugarCastCDM = {
+      {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs},
----------------
NoQ wrote:
> I'd rather have only one map from call descriptions to (callback, kind) pairs. This should make the `evalCall` code much more readable.
The problem with that I really want to use something like that:
```
auto &&[Check, Kind, MoreData] = *Lookup;
```
but I cannot. Until that time it equally non-readable and difficult to scale sadly. For now it is fine, but that C++17 feature would be more awesome.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:231
+    // If we cannot obtain 'MCE' we cannot be sure how to model it.
+    const auto *MCE = dyn_cast<CXXMemberCallExpr>(CE->IgnoreParenImpCasts());
+    if (!MCE)
----------------
NoQ wrote:
> ```lang=c++
> const auto *InstanceCall = dyn_cast<CXXInstanceCall>(&Call);
> ...
> DV = InstanceCall->getCXXThisVal();
> ```
Hm, not bad. Thanks!


================
Comment at: clang/test/Analysis/cast-value.cpp:186
+  // expected-note at -1 {{Assuming pointer value is null}}
+  // expected-note at -2 {{Assuming dynamic cast to 'Circle' succeeds}}
+  // expected-note at -3 {{'C' initialized here}}
----------------
NoQ wrote:
> To think: this note isn't very useful because hard casts aren't really supposed to fail anyway.
I believe it is better than the generic `Assuming...` piece.


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

https://reviews.llvm.org/D65889