[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 9 inline comments as done.
Charusso added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:216
+  const NoteTag *Tag = C.getNoteTag(
+      [=](BugReport &) -> std::string {
+        SmallString<128> Msg;
----------------
NoQ wrote:
> Can you use the overload without the bug report parameter? Cause you don't seem to be using it anyway.
I think you have forgotten to implement that scenario, but now we have that as well since D66325.


================
Comment at: clang/test/Analysis/cast-value-notes.cpp:5-27
 namespace llvm {
 template <class X, class Y>
 const X *cast(Y Value);
 
 template <class X, class Y>
 const X *dyn_cast(Y *Value);
 template <class X, class Y>
----------------
NoQ wrote:
> Szelethus wrote:
> > Hmm, we may want to move these eventually to `clang/test/Inputs/`, since this isn't the only LLVM checker. Thought that said, I want to see the other LLVM checker gone, like, purged from the codebase for being the abomination that it is.
> > 
> > Feel free to leave these here for know, just thinking aloud :)
> I agree, extracting common definitions into a system header simulator usually ends up being worth it pretty quickly.
Thanks for the idea! Moved in D66325.


================
Comment at: clang/test/Analysis/cast-value-notes.cpp:111
+  if (isa<Triangle>(C)) {
+    // expected-note at -1 {{Assuming 'C' with type 'Circle' is not the instance of 'Triangle'}}
+    // expected-note at -2 {{Taking false branch}}
----------------
NoQ wrote:
> This test looks incorrect. We shouldn't be assuming this, we already know that it's not a triangle because we know that it's a circle.
Let us discuss in D66325.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66423