[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks!!

Could you also unforget the diff context? >.<



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:270-272
+    default:
+      llvm_unreachable("Invalid template argument for isa<> or "
+                       "isa_and_nonnull<>");
----------------
We shouldn't crash when code under analysis doesn't match our expectation.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:315
+    if (CastSucceeds) {
+      Success = true;
+      C.addTransition(
----------------
Let's return immediately after the transition instead. Like, generally, it's a good practice to return immediately after a transition if you don't plan any more state splits, because otherwise it's too easy to accidentally introduce unwanted state splits.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:482
+        !(ParamT->isReferenceType() && ResultT->isReferenceType())) {
+      Call.dump();
       return false;
----------------
Looks like a leftover debug print.


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

https://reviews.llvm.org/D85728