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

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 3 inline comments as done.
baloghadamsoftware added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:270-272
+    default:
+      llvm_unreachable("Invalid template argument for isa<> or "
+                       "isa_and_nonnull<>");
----------------
NoQ wrote:
> We shouldn't crash when code under analysis doesn't match our expectation.
The question is whether we can get any other template argument kind for `llvm::isa<>()` and `llvm::isa_and_nonnull<>()` than `Type` or `Pack`?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:315
+    if (CastSucceeds) {
+      Success = true;
+      C.addTransition(
----------------
NoQ wrote:
> 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.
Now I inserted a return but only if this is a known conversion. If this is an assumption, the we should assume every type in the template argument list.


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

https://reviews.llvm.org/D85728