[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 added a comment.

I was indeed on vacation, so thanks for committing it, @NoQ! I was waiting for 
agreement for the prerequisite patch then I forgot to notify you that I was 
going on vacation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4448affede51: [analyzer] pr47037: CastValueChecker: Support 
for the new variadic isa<>. (authored by baloghadamsoftware, committed by 
dergachev.a).

Changed prior to commit:
  https://reviews.llvm.org/D85728?vs=285047&id=288420#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85728

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -13,6 +13,8 @@
   const T *getAs() const;
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
@@ -27,7 +29,6 @@
 }
 
 void evalNonNullParamNonNullReturnReference(const Shape &S) {
-  // Unmodeled cast from reference to pointer.
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{'C' initialized here}}
 
@@ -43,13 +44,37 @@
 return;
   }
 
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   if (isa(C)) {
 // expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (isa(C)) {
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
 // expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking true branch}}
 
@@ -65,22 +90,57 @@
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!isa(C)) {
-// expected-note@-1 {{Assuming 'C' is a 'Triangle'}}
+  if (!dyn_cast_or_null(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (!isa(C)) {
-// expected-note@-1 {{'C' is a 'Triangle'}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  (void)(1 / !C);
-  // expected-note@-1 {{'C' is non-null}}
-  // expected-note@-2 {{Division by zero}}
-  // expected-warning@-3 {{Division by zero}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
 }
 
 void evalNonNullParamNullReturn(const Shape *S) {
Index: clang/test/Analysis/cast-value-logic.cpp
===
--- clang/test/Analysis/cast-value-logic.cpp
+++ clang/test/Analysis/cast-value-logic.cpp
@@ -19,6 +19,8 @@
   virtual double area();
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {
 public:
   ~Circle();
@@ -39,6 +41,23 @@
 clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
 }
 
+void test_regions_isa_variadic(const Shape *A, const Shape *B) {
+  if (isa(A) &&
+  !isa(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) && !isa_and_nonnull(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+vo

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

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D85728#2237006 , @NoQ wrote:

> Hans is pinging us on Bugzilla because this patch is marked as a release 
> blocker (and i believe that it indeed is). I think we should land these 
> patches.

I believe @baloghadamsoftware is on vacation.
I'm sure he won't be mad if you commit on his behalf.


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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.

Hans is pinging us on Bugzilla because this patch is marked as a release 
blocker (and i believe that it indeed is). I think we should land these patches.


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:289
+  for (QualType CastToTy: CastToTyVec) {
+if (CastFromTy->isPointerType())
+  CastToTy = C.getASTContext().getPointerType(CastToTy);

whisperity wrote:
> NoQ wrote:
> > Hmm, is this phabricator's way of displaying tabs?
> I believe it is not displaying tabs, but rather just indicating that the 
> current line changed in a way that //only// the indentation has changed... 
> instead of marking the old side of the diff red. The HTML element is 
> `span.depth-in`. A `span.depth-out` is red, and the arrow points the other 
> way.
> 
> But I agree this is a new thing since the version update.
Aha, ok, nice!


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 2 inline comments as done.
baloghadamsoftware added a comment.

@NoQ Thank you for the review. Please also review D85752 
 after my update and tell me what I have to 
change there if it is not acceptable yet. This patch depends on that one.


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:289
+  for (QualType CastToTy: CastToTyVec) {
+if (CastFromTy->isPointerType())
+  CastToTy = C.getASTContext().getPointerType(CastToTy);

NoQ wrote:
> Hmm, is this phabricator's way of displaying tabs?
I believe it is not displaying tabs, but rather just indicating that the 
current line changed in a way that //only// the indentation has changed... 
instead of marking the old side of the diff red. The HTML element is 
`span.depth-in`. A `span.depth-out` is red, and the arrow points the other way.

But I agree this is a new thing since the version update.


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:289
+  for (QualType CastToTy: CastToTyVec) {
+if (CastFromTy->isPointerType())
+  CastToTy = C.getASTContext().getPointerType(CastToTy);

Hmm, is this phabricator's way of displaying tabs?



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:270-272
+default:
+  llvm_unreachable("Invalid template argument for isa<> or "
+   "isa_and_nonnull<>");

martong wrote:
> baloghadamsoftware wrote:
> > 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`?
> I think it is worth to ponder about the below case. What if `isa` is used in 
> a dependent context (i.e. it is used inside another template)?
> ```
> /// The template argument is an expression, and we've not resolved it to 
> one
> /// of the other forms yet, either because it's dependent or because we're
> /// representing a non-canonical template argument (for instance, in a
> /// TemplateSpecializationType).
> Expression,
> ```
Aha, yup, that's better, thanks.

> The question is whether we can get any other template argument kind for 
> `llvm::isa<>()` and `llvm::isa_and_nonnull<>()` than Type or Pack?

It's a more general problem: what about a completely unrelated template 
function that is accidentally called `llvm::isa<>()` or 
`llvm::isa_and_nonnull<>()`? Or what if `llvm::isa<>()`'s/ 
`llvm::isa_and_nonnull<>()`'s implementation changes again? If it's at all 
possible in any template function, we should take it into account and at least 
provide an early return, we shouldn't rule out that possibility by looking at 
the name only.

> I think it is worth to ponder about the below case. What if `isa` is used in 
> a dependent context (i.e. it is used inside another template)?

Ok, this one probably doesn't happen, because we don't ever try to symbolically 
execute templated ASTs (that aren't fully instantiated).


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gabor Marton via Phabricator via cfe-commits]

martong 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<>");

baloghadamsoftware wrote:
> 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`?
I think it is worth to ponder about the below case. What if `isa` is used in a 
dependent context (i.e. it is used inside another template)?
```
/// The template argument is an expression, and we've not resolved it to one
/// of the other forms yet, either because it's dependent or because we're
/// representing a non-canonical template argument (for instance, in a
/// TemplateSpecializationType).
Expression,
```


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 285047.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D85728

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -13,6 +13,8 @@
   const T *getAs() const;
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
@@ -27,7 +29,6 @@
 }
 
 void evalNonNullParamNonNullReturnReference(const Shape &S) {
-  // Unmodeled cast from reference to pointer.
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{'C' initialized here}}
 
@@ -43,13 +44,37 @@
 return;
   }
 
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   if (isa(C)) {
 // expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (isa(C)) {
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
 // expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking true branch}}
 
@@ -65,22 +90,57 @@
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!isa(C)) {
-// expected-note@-1 {{Assuming 'C' is a 'Triangle'}}
+  if (!dyn_cast_or_null(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (!isa(C)) {
-// expected-note@-1 {{'C' is a 'Triangle'}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  (void)(1 / !C);
-  // expected-note@-1 {{'C' is non-null}}
-  // expected-note@-2 {{Division by zero}}
-  // expected-warning@-3 {{Division by zero}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
 }
 
 void evalNonNullParamNullReturn(const Shape *S) {
Index: clang/test/Analysis/cast-value-logic.cpp
===
--- clang/test/Analysis/cast-value-logic.cpp
+++ clang/test/Analysis/cast-value-logic.cpp
@@ -19,6 +19,8 @@
   virtual double area();
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {
 public:
   ~Circle();
@@ -39,6 +41,23 @@
 clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
 }
 
+void test_regions_isa_variadic(const Shape *A, const Shape *B) {
+  if (isa(A) &&
+  !isa(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) && !isa_and_nonnull(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull_variadic(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) &&
+  !isa_and_nonnull(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
 namespace test_cast {
 void

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

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:265
+  SmallVector CastToTyVec;
+  for (unsigned idx = 0; idx < FD->getTemplateSpecializationArgs()->size() - 1;
+   ++idx) {

Do we intentionally skip the last arg with this loop boundary, or is the loop 
variable just off-by-one?


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

https://reviews.llvm.org/D85728

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 updated this revision to Diff 284801.
baloghadamsoftware retitled this revision from "[Analyzer][WIP] Support for the 
new variadic isa<> and isa_and_not_null<> in CastValueChecker" to "[Analyzer] 
Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker".
baloghadamsoftware edited the summary of this revision.
baloghadamsoftware added a comment.

Working version. Debug printouts removed.


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

https://reviews.llvm.org/D85728

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -13,6 +13,8 @@
   const T *getAs() const;
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
@@ -27,7 +29,6 @@
 }
 
 void evalNonNullParamNonNullReturnReference(const Shape &S) {
-  // Unmodeled cast from reference to pointer.
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{'C' initialized here}}
 
@@ -43,13 +44,37 @@
 return;
   }
 
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   if (isa(C)) {
 // expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (isa(C)) {
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
 // expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking true branch}}
 
@@ -65,22 +90,57 @@
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!isa(C)) {
-// expected-note@-1 {{Assuming 'C' is a 'Triangle'}}
+  if (!dyn_cast_or_null(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  if (!isa(C)) {
-// expected-note@-1 {{'C' is a 'Triangle'}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
 
-  (void)(1 / !C);
-  // expected-note@-1 {{'C' is non-null}}
-  // expected-note@-2 {{Division by zero}}
-  // expected-warning@-3 {{Division by zero}}
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is neither a 'Triangle' nor a 'Rectangle' nor a 'Hexagon'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
 }
 
 void evalNonNullParamNullReturn(const Shape *S) {
Index: clang/test/Analysis/cast-value-logic.cpp
===
--- clang/test/Analysis/cast-value-logic.cpp
+++ clang/test/Analysis/cast-value-logic.cpp
@@ -19,6 +19,8 @@
   virtual double area();
 };
 class Triangle : public Shape {};
+class Rectangle : public Shape {};
+class Hexagon : public Shape {};
 class Circle : public Shape {
 public:
   ~Circle();
@@ -39,6 +41,23 @@
 clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
 }
 
+void test_regions_isa_variadic(const Shape *A, const Shape *B) {
+  if (isa(A) &&
+  !isa(B))
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_regions_isa_and_nonnull(const Shape *A, const Shape *B) {
+  if (isa_and_nonnull(A) && !isa_and_nonnull(B))
+clang_a