[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358719: [analyzer] NFC: MoveChecker: Refactor tests to use 
-verify=prefix. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60732?vs=195250=195838#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60732

Files:
  cfe/trunk/test/Analysis/use-after-move.cpp

Index: cfe/trunk/test/Analysis/use-after-move.cpp
===
--- cfe/trunk/test/Analysis/use-after-move.cpp
+++ cfe/trunk/test/Analysis/use-after-move.cpp
@@ -1,31 +1,37 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,non-aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,non-aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
-// RUN:  -analyzer-checker debug.ExprInspection
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,non-aggressive
 // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,non-aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=All\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
-// RUN:  -analyzer-checker debug.ExprInspection
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=All\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,aggressive
 
 // RUN: not %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=core \
@@ -75,10 +81,7 @@
 moveconstruct(std::move(*a));
   }
   A(const A ) : i(other.i), d(other.d), b(other.b) {}
-  A(A &) : i(other.i), d(other.d), b(std::move(other.b)) {
-#ifdef AGGRESSIVE
-// expected-note@-2{{Object 'b' is moved}}
-#endif
+  A(A &) : i(other.i), d(other.d), b(std::move(other.b)) { // aggressive-note{{Object 'b' is moved}}
   }
   A(A &, char *k) {
 moveconstruct(std::move(other));
@@ -141,33 +144,21 @@
 void simpleMoveCtorTest() {
   {
 A a;
-A b = std::move(a);
-a.foo();
-#ifndef PEACEFUL
-// expected-note@-3 {{Object 'a' is moved}}
-// expected-warning@-3 {{Method called on moved-from object 'a'}}
-// expected-note@-4{{Method called on moved-from object 'a'}}
-#endif
+A b = std::move(a); // peaceful-note {{Object 'a' is moved}}
+a.foo(); // peaceful-warning {{Method called on moved-from object 'a'}}
+ // peaceful-note@-1 {{Method called on 

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D60732#1469026 , @jdenny wrote:

> The original patch added documentation to `-cc1 -help`.  Doxygen for 
> VerifyDiagnosticConsumer is another possibility except it currently doesn't 
> mention command-line usage at all.  Maybe it should?


I would have definitely discovered this feature earlier if it was mentioned on 
the doxygen page for VerifyDiagnosticConsumer!

In D60732#1467504 , @Charusso wrote:

> I think this functionality is unused because you would split the file into 
> six to reduce the overhead/scroll and that is it.


The current test tests all 6 modes on all functions, that's a lot of coverage. 
You can't obtain the same coverage by splitting the file into mode-specific 
tests; you'd have to duplicate all code in all files, is much harder to 
maintain than `#ifdef`s. It would probably still be easier to read, but the 
constant fear that the files aren't actually identical pretty much defeats the 
purpose.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60732



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D60732#1469026 , @jdenny wrote:

> Sometimes you can avoid an explosion of FileCheck prefixes using -D.  I'm not 
> aware of anything like -D for -verify.


I mean a -D that expands a variable within an expected diagnostic.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60732



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

> Just wanted to give a bit more visibility to the underrated technology of 
> writing -verify=a,b,c instead of the #ifdefclutter.

It's great to see this getting more use.

> It's also a bit wonky because it seems that you have to write an exponential 
> number of flags to verify in order to have full flexibility (which is why i 
> didn't update the #ifdef DFS test), but it still seems to be much better in 
> most cases.

Sometimes you can avoid an explosion of FileCheck prefixes using -D.  I'm not 
aware of anything like -D for -verify.

In D60732#1467504 , @Charusso wrote:

> It is a cool reveal, could you provide a documentation?


The original patch added documentation to `-cc1 -help`.  Doxygen for 
VerifyDiagnosticConsumer is another possibility except it currently doesn't 
mention command-line usage at all.  Maybe it should?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60732



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

This is also very useful to test that a given warning is only emitted in c++xx. 
Funnily when grepping for `verify=` in `test/` most matches are from tests 
exercising this functionality.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60732



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso accepted this revision.
Charusso added a comment.
This revision is now accepted and ready to land.

I think this functionality is unused because you would split the file into six 
to reduce the overhead/scroll and that is it.

It is a cool reveal, could you provide a documentation?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60732



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

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso, jdenny.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.

Just wanted to give a bit more visibility to the underrated technology of 
writing `-verify=a,b,c` instead of the `#ifdef`clutter.

It's also a bit wonky because it seems that you have to write an exponential 
number of flags to verify in order to have full flexibility (which is why i 
didn't update the `#ifdef DFS` test), but it still seems to be much better in 
most cases.


Repository:
  rC Clang

https://reviews.llvm.org/D60732

Files:
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -1,31 +1,37 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,non-aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,non-aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
-// RUN:  -analyzer-checker debug.ExprInspection
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,non-aggressive
 // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,non-aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
-// RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=All\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,aggressive
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
-// RUN:  -analyzer-checker debug.ExprInspection
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=All\
+// RUN:  -analyzer-checker debug.ExprInspection\
+// RUN:  -verify=expected,peaceful,aggressive
 
 // RUN: not %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=core \
@@ -75,10 +81,7 @@
 moveconstruct(std::move(*a));
   }
   A(const A ) : i(other.i), d(other.d), b(other.b) {}
-  A(A &) : i(other.i), d(other.d), b(std::move(other.b)) {
-#ifdef AGGRESSIVE
-// expected-note@-2{{Object 'b' is moved}}
-#endif
+  A(A &) : i(other.i), d(other.d), b(std::move(other.b)) { // aggressive-note{{Object 'b' is moved}}
   }
   A(A &, char *k) {
 moveconstruct(std::move(other));
@@ -141,33 +144,21 @@
 void simpleMoveCtorTest() {
   {
 A a;
-A b = std::move(a);
-a.foo();
-#ifndef PEACEFUL
-// expected-note@-3 {{Object 'a' is moved}}
-// expected-warning@-3 {{Method called on moved-from object 'a'}}
-// expected-note@-4{{Method called on moved-from object 'a'}}