[PATCH] D154688: [clang] Show verify prefix in error messages

2023-08-17 Thread Timm Bäder via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4e0589b2cd9: [clang][Verify] Show prefix in -verify error 
messages (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154688

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/ARCMT/verify.m
  clang/test/Frontend/verify-any-file.c
  clang/test/Frontend/verify-fatal.c
  clang/test/Frontend/verify-ignore-unexpected.c
  clang/test/Frontend/verify-unknown-arg.c
  clang/test/Frontend/verify.c
  clang/test/Frontend/verify2.c
  clang/test/Frontend/verify3.c
  clang/test/Misc/diag-verify.cpp

Index: clang/test/Misc/diag-verify.cpp
===
--- clang/test/Misc/diag-verify.cpp
+++ clang/test/Misc/diag-verify.cpp
@@ -25,7 +25,7 @@
   x = y; // expected-error{{use of undeclared identifier 'y' identifier 'y'}}
 }
 
-//CHECK: error: 'error' diagnostics expected but not seen: 
+//CHECK: error: 'expected-error' diagnostics expected but not seen: 
 //CHECK:   Line 17: use of undeclared identifier 'y' is fine
 //CHECK:   Line 18: abuse of undeclared identifier 'y'
 //CHECK:   Line 19: good use of undeclared identifier 'y' in code
@@ -35,7 +35,7 @@
 //CHECK:   Line 23: use of undeclared identifier 'y'; please declare y before use
 //CHECK:   Line 24: use of use of undeclared identifier 'y'
 //CHECK:   Line 25: use of undeclared identifier 'y' identifier 'y'
-//CHECK: error: 'error' diagnostics seen but not expected: 
+//CHECK: error: 'expected-error' diagnostics seen but not expected: 
 //CHECK:   Line 17: use of undeclared identifier 'y'
 //CHECK:   Line 18: use of undeclared identifier 'y'
 //CHECK:   Line 19: use of undeclared identifier 'y'
Index: clang/test/Frontend/verify3.c
===
--- clang/test/Frontend/verify3.c
+++ clang/test/Frontend/verify3.c
@@ -7,7 +7,7 @@
 // expected-no-diagnostics
 // expected-note {{}}
 
-//  CHECK1: error: 'error' diagnostics seen but not expected:
+//  CHECK1: error: 'expected-error' diagnostics seen but not expected:
 // CHECK1-NEXT:   Line 8: expected directive cannot follow 'expected-no-diagnostics' directive
 // CHECK1-NEXT: 1 error generated.
 #endif
@@ -18,7 +18,7 @@
 // expected-warning@-1 {{X}}
 // expected-no-diagnostics
 
-//  CHECK2: error: 'error' diagnostics seen but not expected:
+//  CHECK2: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 19: 'expected-no-diagnostics' directive cannot follow other expected directives
 // CHECK2-NEXT: 1 error generated.
 #endif
Index: clang/test/Frontend/verify2.c
===
--- clang/test/Frontend/verify2.c
+++ clang/test/Frontend/verify2.c
@@ -13,7 +13,7 @@
 // expected-error {{should be ignored}}
 
 //  CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics'
-// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK-NEXT:   Line 5: header
 // CHECK-NEXT:   Line 10: source
 // CHECK-NEXT: 3 errors generated.
@@ -31,9 +31,9 @@
 // expected-error@verify2.h:* {{header}}
 // expected-error@verify2.h:* {{unknown}}
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   File {{.*}}verify2.h Line * (directive at {{.*}}verify2.c:32): unknown
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   File {{.*}}verify2.c Line 10: source
 // CHECK2-NEXT: 2 errors generated.
 #endif
Index: clang/test/Frontend/verify.c
===
--- clang/test/Frontend/verify.c
+++ clang/test/Frontend/verify.c
@@ -48,15 +48,15 @@
 // This is encapsulated in "#if 0" so that the expected-* checks below
 // are not inadvertently included in the diagnostic checking!
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 41: define_error
 // CHECK2-NEXT:   Line 43: line_error
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 43: #line directive requires a positive integer argument
 // CHECK2-NEXT:   Line 44: AAA // expected-error {{[{][{]BBB[}][}]}} <- this shall be part of diagnostic
-// CHECK2-NEXT: error: 'warning' diagnostics expected but not seen:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 42: undef_error
-// CHECK2-NEXT: error: 'warning' diagnostics seen

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-08-10 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-08-10 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-31 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-23 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added inline comments.



Comment at: clang/test/Frontend/verify.c:159
+
+// what-error {{huh?}}
+// CHECK9: error: 'what-error' diagnostics expected but not seen:

MaskRay wrote:
> This may need a comment explaining that this is not recognized as a directive.
Not sure I follow, what isn't being recognized as a directive?


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-23 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder updated this revision to Diff 543298.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D154688

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/ARCMT/verify.m
  clang/test/Frontend/verify-any-file.c
  clang/test/Frontend/verify-fatal.c
  clang/test/Frontend/verify-ignore-unexpected.c
  clang/test/Frontend/verify-unknown-arg.c
  clang/test/Frontend/verify.c
  clang/test/Frontend/verify2.c
  clang/test/Frontend/verify3.c
  clang/test/Misc/diag-verify.cpp

Index: clang/test/Misc/diag-verify.cpp
===
--- clang/test/Misc/diag-verify.cpp
+++ clang/test/Misc/diag-verify.cpp
@@ -25,7 +25,7 @@
   x = y; // expected-error{{use of undeclared identifier 'y' identifier 'y'}}
 }
 
-//CHECK: error: 'error' diagnostics expected but not seen: 
+//CHECK: error: 'expected-error' diagnostics expected but not seen: 
 //CHECK:   Line 17: use of undeclared identifier 'y' is fine
 //CHECK:   Line 18: abuse of undeclared identifier 'y'
 //CHECK:   Line 19: good use of undeclared identifier 'y' in code
@@ -35,7 +35,7 @@
 //CHECK:   Line 23: use of undeclared identifier 'y'; please declare y before use
 //CHECK:   Line 24: use of use of undeclared identifier 'y'
 //CHECK:   Line 25: use of undeclared identifier 'y' identifier 'y'
-//CHECK: error: 'error' diagnostics seen but not expected: 
+//CHECK: error: 'expected-error' diagnostics seen but not expected: 
 //CHECK:   Line 17: use of undeclared identifier 'y'
 //CHECK:   Line 18: use of undeclared identifier 'y'
 //CHECK:   Line 19: use of undeclared identifier 'y'
Index: clang/test/Frontend/verify3.c
===
--- clang/test/Frontend/verify3.c
+++ clang/test/Frontend/verify3.c
@@ -7,7 +7,7 @@
 // expected-no-diagnostics
 // expected-note {{}}
 
-//  CHECK1: error: 'error' diagnostics seen but not expected:
+//  CHECK1: error: 'expected-error' diagnostics seen but not expected:
 // CHECK1-NEXT:   Line 8: expected directive cannot follow 'expected-no-diagnostics' directive
 // CHECK1-NEXT: 1 error generated.
 #endif
@@ -18,7 +18,7 @@
 // expected-warning@-1 {{X}}
 // expected-no-diagnostics
 
-//  CHECK2: error: 'error' diagnostics seen but not expected:
+//  CHECK2: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 19: 'expected-no-diagnostics' directive cannot follow other expected directives
 // CHECK2-NEXT: 1 error generated.
 #endif
Index: clang/test/Frontend/verify2.c
===
--- clang/test/Frontend/verify2.c
+++ clang/test/Frontend/verify2.c
@@ -13,7 +13,7 @@
 // expected-error {{should be ignored}}
 
 //  CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics'
-// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK-NEXT:   Line 5: header
 // CHECK-NEXT:   Line 10: source
 // CHECK-NEXT: 3 errors generated.
@@ -31,9 +31,9 @@
 // expected-error@verify2.h:* {{header}}
 // expected-error@verify2.h:* {{unknown}}
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   File {{.*}}verify2.h Line * (directive at {{.*}}verify2.c:32): unknown
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   File {{.*}}verify2.c Line 10: source
 // CHECK2-NEXT: 2 errors generated.
 #endif
Index: clang/test/Frontend/verify.c
===
--- clang/test/Frontend/verify.c
+++ clang/test/Frontend/verify.c
@@ -48,15 +48,15 @@
 // This is encapsulated in "#if 0" so that the expected-* checks below
 // are not inadvertently included in the diagnostic checking!
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 41: define_error
 // CHECK2-NEXT:   Line 43: line_error
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 43: #line directive requires a positive integer argument
 // CHECK2-NEXT:   Line 44: AAA // expected-error {{[{][{]BBB[}][}]}} <- this shall be part of diagnostic
-// CHECK2-NEXT: error: 'warning' diagnostics expected but not seen:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 42: undef_error
-// CHECK2-NEXT: error: 'warning' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 42: extra tokens

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-23 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:881
+  std::string KindStr = Prefix + "-" + Kind;
+
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()

The variables are immediately used. I think in this case our convention is omit 
the blank line (and in general I think we omit blank lines for many more cases, 
compared to some other projects (GNU, Linux kernel, etc)).



Comment at: clang/test/Frontend/verify.c:159
+
+// what-error {{huh?}}
+// CHECK9: error: 'what-error' diagnostics expected but not seen:

This may need a comment explaining that this is not recognized as a directive.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-23 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder updated this revision to Diff 543260.

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

https://reviews.llvm.org/D154688

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/ARCMT/verify.m
  clang/test/Frontend/verify-any-file.c
  clang/test/Frontend/verify-fatal.c
  clang/test/Frontend/verify-ignore-unexpected.c
  clang/test/Frontend/verify-unknown-arg.c
  clang/test/Frontend/verify.c
  clang/test/Frontend/verify2.c
  clang/test/Frontend/verify3.c
  clang/test/Misc/diag-verify.cpp

Index: clang/test/Misc/diag-verify.cpp
===
--- clang/test/Misc/diag-verify.cpp
+++ clang/test/Misc/diag-verify.cpp
@@ -25,7 +25,7 @@
   x = y; // expected-error{{use of undeclared identifier 'y' identifier 'y'}}
 }
 
-//CHECK: error: 'error' diagnostics expected but not seen: 
+//CHECK: error: 'expected-error' diagnostics expected but not seen: 
 //CHECK:   Line 17: use of undeclared identifier 'y' is fine
 //CHECK:   Line 18: abuse of undeclared identifier 'y'
 //CHECK:   Line 19: good use of undeclared identifier 'y' in code
@@ -35,7 +35,7 @@
 //CHECK:   Line 23: use of undeclared identifier 'y'; please declare y before use
 //CHECK:   Line 24: use of use of undeclared identifier 'y'
 //CHECK:   Line 25: use of undeclared identifier 'y' identifier 'y'
-//CHECK: error: 'error' diagnostics seen but not expected: 
+//CHECK: error: 'expected-error' diagnostics seen but not expected: 
 //CHECK:   Line 17: use of undeclared identifier 'y'
 //CHECK:   Line 18: use of undeclared identifier 'y'
 //CHECK:   Line 19: use of undeclared identifier 'y'
Index: clang/test/Frontend/verify3.c
===
--- clang/test/Frontend/verify3.c
+++ clang/test/Frontend/verify3.c
@@ -7,7 +7,7 @@
 // expected-no-diagnostics
 // expected-note {{}}
 
-//  CHECK1: error: 'error' diagnostics seen but not expected:
+//  CHECK1: error: 'expected-error' diagnostics seen but not expected:
 // CHECK1-NEXT:   Line 8: expected directive cannot follow 'expected-no-diagnostics' directive
 // CHECK1-NEXT: 1 error generated.
 #endif
@@ -18,7 +18,7 @@
 // expected-warning@-1 {{X}}
 // expected-no-diagnostics
 
-//  CHECK2: error: 'error' diagnostics seen but not expected:
+//  CHECK2: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 19: 'expected-no-diagnostics' directive cannot follow other expected directives
 // CHECK2-NEXT: 1 error generated.
 #endif
Index: clang/test/Frontend/verify2.c
===
--- clang/test/Frontend/verify2.c
+++ clang/test/Frontend/verify2.c
@@ -13,7 +13,7 @@
 // expected-error {{should be ignored}}
 
 //  CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics'
-// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK-NEXT:   Line 5: header
 // CHECK-NEXT:   Line 10: source
 // CHECK-NEXT: 3 errors generated.
@@ -31,9 +31,9 @@
 // expected-error@verify2.h:* {{header}}
 // expected-error@verify2.h:* {{unknown}}
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   File {{.*}}verify2.h Line * (directive at {{.*}}verify2.c:32): unknown
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   File {{.*}}verify2.c Line 10: source
 // CHECK2-NEXT: 2 errors generated.
 #endif
Index: clang/test/Frontend/verify.c
===
--- clang/test/Frontend/verify.c
+++ clang/test/Frontend/verify.c
@@ -48,15 +48,15 @@
 // This is encapsulated in "#if 0" so that the expected-* checks below
 // are not inadvertently included in the diagnostic checking!
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 41: define_error
 // CHECK2-NEXT:   Line 43: line_error
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 43: #line directive requires a positive integer argument
 // CHECK2-NEXT:   Line 44: AAA // expected-error {{[{][{]BBB[}][}]}} <- this shall be part of diagnostic
-// CHECK2-NEXT: error: 'warning' diagnostics expected but not seen:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 42: undef_error
-// CHECK2-NEXT: error: 'warning' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 42: extra tokens at end of #undef directive
 //

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-21 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

Precommit CI found relevant failures that should be addressed.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-21 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder updated this revision to Diff 542768.

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

https://reviews.llvm.org/D154688

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/Frontend/verify-any-file.c
  clang/test/Frontend/verify-fatal.c
  clang/test/Frontend/verify-ignore-unexpected.c
  clang/test/Frontend/verify-unknown-arg.c
  clang/test/Frontend/verify.c
  clang/test/Frontend/verify2.c
  clang/test/Frontend/verify3.c

Index: clang/test/Frontend/verify3.c
===
--- clang/test/Frontend/verify3.c
+++ clang/test/Frontend/verify3.c
@@ -7,7 +7,7 @@
 // expected-no-diagnostics
 // expected-note {{}}
 
-//  CHECK1: error: 'error' diagnostics seen but not expected:
+//  CHECK1: error: 'expected-error' diagnostics seen but not expected:
 // CHECK1-NEXT:   Line 8: expected directive cannot follow 'expected-no-diagnostics' directive
 // CHECK1-NEXT: 1 error generated.
 #endif
@@ -18,7 +18,7 @@
 // expected-warning@-1 {{X}}
 // expected-no-diagnostics
 
-//  CHECK2: error: 'error' diagnostics seen but not expected:
+//  CHECK2: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 19: 'expected-no-diagnostics' directive cannot follow other expected directives
 // CHECK2-NEXT: 1 error generated.
 #endif
Index: clang/test/Frontend/verify2.c
===
--- clang/test/Frontend/verify2.c
+++ clang/test/Frontend/verify2.c
@@ -13,7 +13,7 @@
 // expected-error {{should be ignored}}
 
 //  CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics'
-// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK-NEXT:   Line 5: header
 // CHECK-NEXT:   Line 10: source
 // CHECK-NEXT: 3 errors generated.
@@ -31,9 +31,9 @@
 // expected-error@verify2.h:* {{header}}
 // expected-error@verify2.h:* {{unknown}}
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   File {{.*}}verify2.h Line * (directive at {{.*}}verify2.c:32): unknown
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   File {{.*}}verify2.c Line 10: source
 // CHECK2-NEXT: 2 errors generated.
 #endif
Index: clang/test/Frontend/verify.c
===
--- clang/test/Frontend/verify.c
+++ clang/test/Frontend/verify.c
@@ -48,15 +48,15 @@
 // This is encapsulated in "#if 0" so that the expected-* checks below
 // are not inadvertently included in the diagnostic checking!
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 41: define_error
 // CHECK2-NEXT:   Line 43: line_error
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 43: #line directive requires a positive integer argument
 // CHECK2-NEXT:   Line 44: AAA // expected-error {{[{][{]BBB[}][}]}} <- this shall be part of diagnostic
-// CHECK2-NEXT: error: 'warning' diagnostics expected but not seen:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 42: undef_error
-// CHECK2-NEXT: error: 'warning' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 42: extra tokens at end of #undef directive
 // CHECK2-NEXT:   Line 45: CCC // expected-warning {{[{][{]DDD[}][}]}} <- this shall be part of diagnostic
 // CHECK2-NEXT: 7 errors generated.
@@ -78,7 +78,7 @@
 # endif   // expected-note {{line_78}}
 #endif
 
-//  CHECK3: error: 'note' diagnostics expected but not seen:
+//  CHECK3: error: 'expected-note' diagnostics expected but not seen:
 // CHECK3-NEXT:   Line 67: line_67
 // CHECK3-NEXT:   Line 71: line_71
 // CHECK3-NEXT:   Line 72: line_72
@@ -91,9 +91,9 @@
 #ifdef TEST4
 #include "missing_header_file.include" // expected-error {{include_error}}
 
-//  CHECK4: error: 'error' diagnostics expected but not seen:
+//  CHECK4: error: 'expected-error' diagnostics expected but not seen:
 // CHECK4-NEXT:   Line 92: include_error
-// CHECK4-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK4-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK4-NEXT:   Line 92: 'missing_header_file.include' file not found
 // CHECK4-NEXT: 2 errors generated.
 #endif
@@ -102,7 +102,7 @@
 #include "verify-directive.h"
 // expected-error@50 {{source file test}}
 
-//  CHECK5: error: 'error' diagnostics expected but not seen:
+//

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-20 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

The change looks great: (from `error: 'error' diagnostics seen but not 
expected:` to `error: 'expected-error' diagnostics seen but not expected`).
When I read Clang tests, I was confused by what `error` meant. I eventually 
figured out `-verify` is special. The new diagnostic is clearer and more 
helpful.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-20 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D154688#4499797 , @tbaeder wrote:

> In D154688#4498398 , @aaron.ballman 
> wrote:
>
>> In D154688#4497967 , @tbaeder 
>> wrote:
>>
>>> When passing a different prefix via `-verify=foo`, the error messages now 
>>> say "error: 'foo-error' diagnostics seen but not expected", etc.
>>>
>>> I'm often working in test files where two different prefixes are used and 
>>> I'm always confused about which one of the two the error messages are 
>>> talking about.
>>
>> What I'm confused by is that we list the line numbers of the failures, so 
>> the prefix only seems like it's helpful in a case where two prefixes use the 
>> same message and the same severity on the same line. e.g., `// foo-error 
>> {{whatever}} bar-error {{whatever}}`. In the other cases, either the line 
>> number is different, or the severity is different, or the message is 
>> different which I thought was giving sufficient context.
>
> This is also reported as being on line 4:
>
>   // RUN: %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info 
> -verify=bar %s 2>&1
>   
>   
>   static_assert(true); // foo \
>// bar-error {{failed}}
>
> which is also the case if line 4 contained another `foo-error {{failed}}` 
> which didn't trigger, leaving developers wondering which one it is.

Okay, we do that often enough in our tests (along with two diagnostics on the 
same line directly) that this seems pretty reasonable to me.

Can you add some test coverage where the prefix is not the default `expected`?


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-20 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-13 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

In D154688#4498398 , @aaron.ballman 
wrote:

> In D154688#4497967 , @tbaeder wrote:
>
>> When passing a different prefix via `-verify=foo`, the error messages now 
>> say "error: 'foo-error' diagnostics seen but not expected", etc.
>>
>> I'm often working in test files where two different prefixes are used and 
>> I'm always confused about which one of the two the error messages are 
>> talking about.
>
> What I'm confused by is that we list the line numbers of the failures, so the 
> prefix only seems like it's helpful in a case where two prefixes use the same 
> message and the same severity on the same line. e.g., `// foo-error 
> {{whatever}} bar-error {{whatever}}`. In the other cases, either the line 
> number is different, or the severity is different, or the message is 
> different which I thought was giving sufficient context.

This is also reported as being on line 4:

  // RUN: %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info 
-verify=bar %s 2>&1
  
  
  static_assert(true); // foo \
   // bar-error {{failed}}

which is also the case if line 4 contained another `foo-error {{failed}}` which 
didn't trigger, leaving developers wondering which one it is.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-13 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D154688#4497967 , @tbaeder wrote:

> When passing a different prefix via `-verify=foo`, the error messages now say 
> "error: 'foo-error' diagnostics seen but not expected", etc.
>
> I'm often working in test files where two different prefixes are used and I'm 
> always confused about which one of the two the error messages are talking 
> about.

What I'm confused by is that we list the line numbers of the failures, so the 
prefix only seems like it's helpful in a case where two prefixes use the same 
message and the same severity on the same line. e.g., `// foo-error 
{{whatever}} bar-error {{whatever}}`. In the other cases, either the line 
number is different, or the severity is different, or the message is different 
which I thought was giving sufficient context.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-13 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

When passing a different prefix via `-verify=foo`, the error messages now say 
"error: 'foo-error' diagnostics seen but not expected", etc.

I'm often working in test files where two different prefixes are used and I'm 
always confused about which one of the two the error messages are talking about.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-13 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

I'm not opposed but I am wondering what the need is for this? (It'd be good to 
capture that information in the patch summary as well.) Some examples showing 
where this helps to correct the issue would be useful.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-07 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder updated this revision to Diff 538058.

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

https://reviews.llvm.org/D154688

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/Frontend/verify-any-file.c
  clang/test/Frontend/verify-fatal.c
  clang/test/Frontend/verify-ignore-unexpected.c
  clang/test/Frontend/verify-unknown-arg.c
  clang/test/Frontend/verify.c
  clang/test/Frontend/verify2.c
  clang/test/Frontend/verify3.c

Index: clang/test/Frontend/verify3.c
===
--- clang/test/Frontend/verify3.c
+++ clang/test/Frontend/verify3.c
@@ -7,7 +7,7 @@
 // expected-no-diagnostics
 // expected-note {{}}
 
-//  CHECK1: error: 'error' diagnostics seen but not expected:
+//  CHECK1: error: 'expected-error' diagnostics seen but not expected:
 // CHECK1-NEXT:   Line 8: expected directive cannot follow 'expected-no-diagnostics' directive
 // CHECK1-NEXT: 1 error generated.
 #endif
@@ -18,7 +18,7 @@
 // expected-warning@-1 {{X}}
 // expected-no-diagnostics
 
-//  CHECK2: error: 'error' diagnostics seen but not expected:
+//  CHECK2: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 19: 'expected-no-diagnostics' directive cannot follow other expected directives
 // CHECK2-NEXT: 1 error generated.
 #endif
Index: clang/test/Frontend/verify2.c
===
--- clang/test/Frontend/verify2.c
+++ clang/test/Frontend/verify2.c
@@ -13,7 +13,7 @@
 // expected-error {{should be ignored}}
 
 //  CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics'
-// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK-NEXT:   Line 5: header
 // CHECK-NEXT:   Line 10: source
 // CHECK-NEXT: 3 errors generated.
@@ -31,9 +31,9 @@
 // expected-error@verify2.h:* {{header}}
 // expected-error@verify2.h:* {{unknown}}
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   File {{.*}}verify2.h Line * (directive at {{.*}}verify2.c:32): unknown
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   File {{.*}}verify2.c Line 10: source
 // CHECK2-NEXT: 2 errors generated.
 #endif
Index: clang/test/Frontend/verify.c
===
--- clang/test/Frontend/verify.c
+++ clang/test/Frontend/verify.c
@@ -48,15 +48,15 @@
 // This is encapsulated in "#if 0" so that the expected-* checks below
 // are not inadvertently included in the diagnostic checking!
 
-//  CHECK2: error: 'error' diagnostics expected but not seen:
+//  CHECK2: error: 'expected-error' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 41: define_error
 // CHECK2-NEXT:   Line 43: line_error
-// CHECK2-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 43: #line directive requires a positive integer argument
 // CHECK2-NEXT:   Line 44: AAA // expected-error {{[{][{]BBB[}][}]}} <- this shall be part of diagnostic
-// CHECK2-NEXT: error: 'warning' diagnostics expected but not seen:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics expected but not seen:
 // CHECK2-NEXT:   Line 42: undef_error
-// CHECK2-NEXT: error: 'warning' diagnostics seen but not expected:
+// CHECK2-NEXT: error: 'expected-warning' diagnostics seen but not expected:
 // CHECK2-NEXT:   Line 42: extra tokens at end of #undef directive
 // CHECK2-NEXT:   Line 45: CCC // expected-warning {{[{][{]DDD[}][}]}} <- this shall be part of diagnostic
 // CHECK2-NEXT: 7 errors generated.
@@ -78,7 +78,7 @@
 # endif   // expected-note {{line_78}}
 #endif
 
-//  CHECK3: error: 'note' diagnostics expected but not seen:
+//  CHECK3: error: 'expected-note' diagnostics expected but not seen:
 // CHECK3-NEXT:   Line 67: line_67
 // CHECK3-NEXT:   Line 71: line_71
 // CHECK3-NEXT:   Line 72: line_72
@@ -91,9 +91,9 @@
 #ifdef TEST4
 #include "missing_header_file.include" // expected-error {{include_error}}
 
-//  CHECK4: error: 'error' diagnostics expected but not seen:
+//  CHECK4: error: 'expected-error' diagnostics expected but not seen:
 // CHECK4-NEXT:   Line 92: include_error
-// CHECK4-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK4-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK4-NEXT:   Line 92: 'missing_header_file.include' file not found
 // CHECK4-NEXT: 2 errors generated.
 #endif
@@ -102,7 +102,7 @@
 #include "verify-directive.h"
 // expected-error@50 {{source file test}}
 
-//  CHECK5: error: 'error' diagnostics expected but not seen:
+//

[PATCH] D154688: [clang] Show verify prefix in error messages

2023-07-07 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder created this revision.
tbaeder added a reviewer: clang.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154688

Files:
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp


Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -876,8 +876,11 @@
 OS << ": " << I->second;
   }
 
+  std::string Prefix = *Diags.getDiagnosticOptions().VerifyPrefixes.begin();
+  std::string KindStr = Prefix + "-" + Kind;
+
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()
-<< Kind << /*Unexpected=*/true << OS.str();
+  << KindStr << /*Unexpected=*/true << OS.str();
   return std::distance(diag_begin, diag_end);
 }
 
@@ -907,8 +910,11 @@
 OS << ": " << D->Text;
   }
 
+  std::string Prefix = *Diags.getDiagnosticOptions().VerifyPrefixes.begin();
+  std::string KindStr = Prefix + "-" + Kind;
+
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()
-<< Kind << /*Unexpected=*/false << OS.str();
+  << KindStr << /*Unexpected=*/false << OS.str();
   return DL.size();
 }
 


Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -876,8 +876,11 @@
 OS << ": " << I->second;
   }
 
+  std::string Prefix = *Diags.getDiagnosticOptions().VerifyPrefixes.begin();
+  std::string KindStr = Prefix + "-" + Kind;
+
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()
-<< Kind << /*Unexpected=*/true << OS.str();
+  << KindStr << /*Unexpected=*/true << OS.str();
   return std::distance(diag_begin, diag_end);
 }
 
@@ -907,8 +910,11 @@
 OS << ": " << D->Text;
   }
 
+  std::string Prefix = *Diags.getDiagnosticOptions().VerifyPrefixes.begin();
+  std::string KindStr = Prefix + "-" + Kind;
+
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()
-<< Kind << /*Unexpected=*/false << OS.str();
+  << KindStr << /*Unexpected=*/false << OS.str();
   return DL.size();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits