[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
This revision was automatically updated to reflect the committed changes.
Closed by commit rC320908: [VerifyDiagnosticConsumer] support
-verify=prefixes (authored by hfinkel, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/diagnostics-order.c
test/Frontend/verify-prefixes.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: include/clang/Basic/DiagnosticOptions.h
===
--- include/clang/Basic/DiagnosticOptions.h
+++ include/clang/Basic/DiagnosticOptions.h
@@ -100,6 +100,10 @@
/// prefixes removed.
std::vector Remarks;
+ /// The prefixes for comment directives sought by -verify ("expected" by
+ /// default).
+ std::vector VerifyPrefixes;
+
public:
// Define accessors/mutators for diagnostic options of enumeration type.
#define DIAGOPT(Name, Bits, Default)
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -338,4 +338,8 @@
def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
"option '-ffine-grained-bitfield-accesses' cannot be enabled together with a sanitizer; flag ignored">,
InGroup;
+
+def note_drv_verify_prefix_spelling : Note<
+ "-verify prefixes must start with a letter and contain only alphanumeric"
+ " characters, hyphens, and underscores">;
}
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -400,8 +400,12 @@
HelpText<"Set the maximum number of source lines to show in a caret diagnostic">;
def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"">,
HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">;
+def verify_EQ : CommaJoined<["-"], "verify=">,
+ MetaVarName<"">,
+ HelpText<"Verify diagnostic output using comment directives that start with"
+ " prefixes in the comma-separated sequence ">;
def verify : Flag<["-"], "verify">,
- HelpText<"Verify diagnostic output using comment directives">;
+ HelpText<"Equivalent to -verify=expected">;
def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">,
HelpText<"Ignore unexpected diagnostic messages">;
def verify_ignore_unexpected_EQ : CommaJoined<["-"], "verify-ignore-unexpected=">,
Index: test/Sema/tautological-unsigned-enum-zero-compare.c
===
--- test/Sema/tautological-unsigned-enum-zero-compare.c
+++ test/Sema/tautological-unsigned-enum-zero-compare.c
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-verify=unsigned,unsigned-signed %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-verify=unsigned-signed %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-Wno-tautological-unsigned-enum-zero-compare \
+// RUN:-verify=silence %s
// Okay, this is where it gets complicated.
// Then default enum sigdness is target-specific.
@@ -12,175 +16,38 @@
enum B { B_a = -1 };
enum B b;
-#ifdef UNSIGNED
- if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
- if (0 >= a)
-return 0;
- if (a > 0)
-return 0;
- if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-return 0;
- if (a <= 0)
-return 0;
- if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-return 0;
- if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-return 0;
- if (0 < a)
-return 0;
-
- if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-return 0;
- if (0U >= a)
-return 0;
- if (a > 0U)
-return 0;
- if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-return 0;
- if (a <= 0U)
-return 0;
- if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-return 0;
- if (a >= 0U) // expected-warning
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny added a comment. In https://reviews.llvm.org/D39694#956661, @hfinkel wrote: > LGTM Thanks for accepting. I don't have commit privileges. Would you please commit for me? This depends on https://reviews.llvm.org/D40995, which also needs to be committed. https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny marked an inline comment as done. jdenny added inline comments. Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:398 +// DToken is foo-bar-warning, but foo is the only -verify prefix). +if (Prefixes.end() == std::find(Prefixes.begin(), Prefixes.end(), DToken)) + continue; hfinkel wrote: > > Converts from std::vector to std::set for more efficient prefix lookup. > > Don't do that. First, std::find is O(N) here anyway. You'd want to use > Prefixes.find(...) instead. However, this set is essentially constant and > you're trying to optimize the lookup. In such a case, std::set is not a good > choice. Just use a sorted vector instead (e.g., call std::sort on the > vector), and then use std::binary_search here. That's likely much faster. > Thanks. https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny updated this revision to Diff 127088.
jdenny added a comment.
Herald added a subscriber: mgrang.
1. Use std::binary_search, as suggested by Hal.
2. Fix another case of line wrapping.
3. Rebase onto a recent master, and remove rewrites of tests that have recently
changed.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/diagnostics-order.c
test/Frontend/verify-prefixes.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
unsigned uvalue(void);
signed int svalue(void);
@@ -13,13 +13,8 @@
void TFunc() {
// Make sure that we do warn for normal variables in template functions !
unsigned char c = svalue();
-#ifdef TEST
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
return;
-#else
- if (c < 0)
- return;
-#endif
if (c < macro(0))
return;
@@ -39,7 +34,8 @@
unsigned un = uvalue();
-#ifdef TEST
+ // silence-no-diagnostics
+
if (un == 0)
return 0;
if (un != 0)
@@ -91,65 +87,10 @@
return 0;
if (0UL >= un)
return 0;
-#else
-// expected-no-diagnostics
- if (un == 0)
- return 0;
- if (un != 0)
- return 0;
- if (un < 0)
- return 0;
- if (un <= 0)
- return 0;
- if (un > 0)
- return 0;
- if (un >= 0)
- return 0;
-
- if (0 == un)
- return 0;
- if (0 != un)
- return 0;
- if (0 < un)
- return 0;
- if (0 <= un)
- return 0;
- if (0 > un)
- return 0;
- if (0 >= un)
- return 0;
-
- if (un == 0UL)
- return 0;
- if (un != 0UL)
- return 0;
- if (un < 0UL)
- return 0;
- if (un <= 0UL)
- return 0;
- if (un > 0UL)
- return 0;
- if (un >= 0UL)
- return 0;
-
- if (0UL == un)
- return 0;
- if (0UL != un)
- return 0;
- if (0UL < un)
- return 0;
- if (0UL <= un)
- return 0;
- if (0UL > un)
- return 0;
- if (0UL >= un)
- return 0;
-#endif
signed int a = svalue();
-#ifdef TEST
if (a == 0)
return 0;
if (a != 0)
@@ -201,60 +142,6 @@
return 0;
if (0UL >= a)
return 0;
-#else
-// expected-no-diagnostics
- if (a == 0)
- return 0;
- if (a != 0)
- return 0;
- if (a < 0)
- return 0;
- if (a <= 0)
- return 0;
- if (a > 0)
- return 0;
- if (a >= 0)
- return 0;
-
- if (0 == a)
- return 0;
- if (0 != a)
- return 0;
- if (0 < a)
- return 0;
- if (0 <= a)
- return 0;
- if (0 > a)
- return 0;
- if (0 >= a)
- return 0;
-
- if (a == 0UL)
- return 0;
- if (a != 0UL)
- return 0;
- if (a < 0UL)
- return 0;
- if (a <= 0UL)
- return 0;
- if (a > 0UL)
- return 0;
- if (a >= 0UL)
- return 0;
-
- if (0UL == a)
- return 0;
- if (0UL != a)
- return 0;
- if (0UL < a)
- return 0;
- if (0UL <= a)
- return 0;
- if (0UL > a)
- return 0;
- if (0UL >= a)
- return 0;
-#endif
float fl = 0;
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-verify=unsigned,unsigned-signed %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-verify=unsigned-signed %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
hfinkel added inline comments. Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:398 +// DToken is foo-bar-warning, but foo is the only -verify prefix). +if (Prefixes.end() == std::find(Prefixes.begin(), Prefixes.end(), DToken)) + continue; > Converts from std::vector to std::set for more efficient prefix lookup. Don't do that. First, std::find is O(N) here anyway. You'd want to use Prefixes.find(...) instead. However, this set is essentially constant and you're trying to optimize the lookup. In such a case, std::set is not a good choice. Just use a sorted vector instead (e.g., call std::sort on the vector), and then use std::binary_search here. That's likely much faster. https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny updated this revision to Diff 126086.
jdenny added a comment.
This update does the following:
1. Fixes the line wrapping that Richard pointed out.
2. Converts from std::vector to std::set for more efficient prefix lookup.
3. Grows a dependence on https://reviews.llvm.org/D40995 (which is a small
patch) because the test case here now exercises multiple invalid prefixes in
one command line.
4. Rebases onto a more recent master. One of the test cases rewritten here
recently changed significantly.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/diagnostics-order.c
test/Frontend/verify-prefixes.c
test/Sema/tautological-constant-compare.c
test/Sema/tautological-constant-enum-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
unsigned uvalue(void);
signed int svalue(void);
@@ -13,13 +13,8 @@
void TFunc() {
// Make sure that we do warn for normal variables in template functions !
unsigned char c = svalue();
-#ifdef TEST
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
return;
-#else
- if (c < 0)
- return;
-#endif
if (c < macro(0))
return;
@@ -39,7 +34,8 @@
unsigned un = uvalue();
-#ifdef TEST
+ // silence-no-diagnostics
+
if (un == 0)
return 0;
if (un != 0)
@@ -91,65 +87,10 @@
return 0;
if (0UL >= un)
return 0;
-#else
-// expected-no-diagnostics
- if (un == 0)
- return 0;
- if (un != 0)
- return 0;
- if (un < 0)
- return 0;
- if (un <= 0)
- return 0;
- if (un > 0)
- return 0;
- if (un >= 0)
- return 0;
-
- if (0 == un)
- return 0;
- if (0 != un)
- return 0;
- if (0 < un)
- return 0;
- if (0 <= un)
- return 0;
- if (0 > un)
- return 0;
- if (0 >= un)
- return 0;
-
- if (un == 0UL)
- return 0;
- if (un != 0UL)
- return 0;
- if (un < 0UL)
- return 0;
- if (un <= 0UL)
- return 0;
- if (un > 0UL)
- return 0;
- if (un >= 0UL)
- return 0;
-
- if (0UL == un)
- return 0;
- if (0UL != un)
- return 0;
- if (0UL < un)
- return 0;
- if (0UL <= un)
- return 0;
- if (0UL > un)
- return 0;
- if (0UL >= un)
- return 0;
-#endif
signed int a = svalue();
-#ifdef TEST
if (a == 0)
return 0;
if (a != 0)
@@ -201,60 +142,6 @@
return 0;
if (0UL >= a)
return 0;
-#else
-// expected-no-diagnostics
- if (a == 0)
- return 0;
- if (a != 0)
- return 0;
- if (a < 0)
- return 0;
- if (a <= 0)
- return 0;
- if (a > 0)
- return 0;
- if (a >= 0)
- return 0;
-
- if (0 == a)
- return 0;
- if (0 != a)
- return 0;
- if (0 < a)
- return 0;
- if (0 <= a)
- return 0;
- if (0 > a)
- return 0;
- if (0 >= a)
- return 0;
-
- if (a == 0UL)
- return 0;
- if (a != 0UL)
- return 0;
- if (a < 0UL)
- return 0;
- if (a <= 0UL)
- return 0;
- if (a > 0UL)
- return 0;
- if (a >= 0UL)
- return 0;
-
- if (0UL == a)
- return 0;
- if (0UL != a)
- return 0;
- if (0UL < a)
- return 0;
- if (0UL <= a)
- return 0;
- if (0UL > a)
- return 0;
- if (0UL >= a)
- return 0;
-#endif
float fl = 0;
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny added a comment. In https://reviews.llvm.org/D39694#949066, @rsmith wrote: > I've not done a detailed review of the string manipulation here, but this > looks like a great feature, thanks! Hi Richard. Thanks for your feedback. I'll fix the line wrapping you pointed out. https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny marked 13 inline comments as done. jdenny added a comment. I marked the comments related to Hal's suggestions as done to avoid confusion for future reviews. I'm not used to using this sort of tool for reviews. Hopefully it's appropriate for the author to do that rather than the reviewer. https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
rsmith added a comment. I've not done a detailed review of the string manipulation here, but this looks like a great feature, thanks! Comment at: include/clang/Basic/DiagnosticDriverKinds.td:342 + +def note_drv_verify_prefix_spelling : Note<"-verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores">; } Please wrap this to 80 columns. Comment at: include/clang/Driver/CC1Options.td:407 def verify : Flag<["-"], "verify">, - HelpText<"Verify diagnostic output using comment directives">; + HelpText<"Similar to -verify=expected">; def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">, jdenny wrote: > hfinkel wrote: > > "Similar to" seems unfortunately vague. Can it say, "Equivalent to ..."? > I agree I should have made it clearer. > > "Equivalent to -verify=expected" works if we decide that duplicate explicit > prefixes are permitted, as you've suggested in a later comment. > > With the current implementation, it should be "All occurrences together are > equivalent to one occurrence of -verify=expected". That is, I chose to > permit multiple occurrences of -verify without explicit prefixes for backward > compatibility, but I chose not to permit duplicate explicit prefixes for > reasons I'll discuss in the other comment. > > I'll clean up the documentation once we agree on the right behavior. > > I don't think we need to worry about backwards compatibility with people passing `-verify` more than once; it seems OK to disallow that if we need to. https://reviews.llvm.org/D39694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny updated this revision to Diff 125640.
jdenny edited the summary of this revision.
jdenny added a comment.
This update includes all of Hal's suggestions.
I'm also thinking of converting prefix storage from a std::vector to a std::map
so that lookup should be faster during parsing.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/verify-prefixes.c
test/Sema/tautological-constant-compare.c
test/Sema/tautological-constant-enum-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
unsigned uvalue(void);
signed int svalue(void);
@@ -13,13 +13,8 @@
void TFunc() {
// Make sure that we do warn for normal variables in template functions !
unsigned char c = svalue();
-#ifdef TEST
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
return;
-#else
- if (c < 0)
- return;
-#endif
if (c < macro(0))
return;
@@ -39,7 +34,8 @@
unsigned un = uvalue();
-#ifdef TEST
+ // silence-no-diagnostics
+
if (un == 0)
return 0;
if (un != 0)
@@ -91,65 +87,10 @@
return 0;
if (0UL >= un)
return 0;
-#else
-// expected-no-diagnostics
- if (un == 0)
- return 0;
- if (un != 0)
- return 0;
- if (un < 0)
- return 0;
- if (un <= 0)
- return 0;
- if (un > 0)
- return 0;
- if (un >= 0)
- return 0;
-
- if (0 == un)
- return 0;
- if (0 != un)
- return 0;
- if (0 < un)
- return 0;
- if (0 <= un)
- return 0;
- if (0 > un)
- return 0;
- if (0 >= un)
- return 0;
-
- if (un == 0UL)
- return 0;
- if (un != 0UL)
- return 0;
- if (un < 0UL)
- return 0;
- if (un <= 0UL)
- return 0;
- if (un > 0UL)
- return 0;
- if (un >= 0UL)
- return 0;
-
- if (0UL == un)
- return 0;
- if (0UL != un)
- return 0;
- if (0UL < un)
- return 0;
- if (0UL <= un)
- return 0;
- if (0UL > un)
- return 0;
- if (0UL >= un)
- return 0;
-#endif
signed int a = svalue();
-#ifdef TEST
if (a == 0)
return 0;
if (a != 0)
@@ -201,60 +142,6 @@
return 0;
if (0UL >= a)
return 0;
-#else
-// expected-no-diagnostics
- if (a == 0)
- return 0;
- if (a != 0)
- return 0;
- if (a < 0)
- return 0;
- if (a <= 0)
- return 0;
- if (a > 0)
- return 0;
- if (a >= 0)
- return 0;
-
- if (0 == a)
- return 0;
- if (0 != a)
- return 0;
- if (0 < a)
- return 0;
- if (0 <= a)
- return 0;
- if (0 > a)
- return 0;
- if (0 >= a)
- return 0;
-
- if (a == 0UL)
- return 0;
- if (a != 0UL)
- return 0;
- if (a < 0UL)
- return 0;
- if (a <= 0UL)
- return 0;
- if (a > 0UL)
- return 0;
- if (a >= 0UL)
- return 0;
-
- if (0UL == a)
- return 0;
- if (0UL != a)
- return 0;
- if (0UL < a)
- return 0;
- if (0UL <= a)
- return 0;
- if (0UL > a)
- return 0;
- if (0UL >= a)
- return 0;
-#endif
float fl = 0;
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-verify=unsigned,unsigned-signed,expected %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-verify=unsigned-signed,signed-silence,expected %s
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny added a comment.
Comment at: lib/Frontend/CompilerInvocation.cpp:1095
+ if (Diags) {
+Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+Diags->Report(diag::note_drv_verify_prefix_unique);
hfinkel wrote:
> jdenny wrote:
> > hfinkel wrote:
> > > If this were going to be an error, then it should have error message that
> > > explains the problem (e.g., duplicate --verify prefix). I don't believe
> > > that we should make this an error (i.e., I think that we should allow
> > > duplicate --verify options with the same prefixes).
> > > If this were going to be an error, then it should have error message that
> > > explains the problem (e.g., duplicate --verify prefix).
> >
> > Are you saying you prefer that to be in the error instead of the note
> > (where it is now)?
> >
> > > I don't believe that we should make this an error (i.e., I think that we
> > > should allow duplicate --verify options with the same prefixes).
> >
> > I see two reasons to permit duplicate explicit prefixes:
> >
> > 1. It simplifies the documentation some (see previous comment).
> >
> > 2. Typically, it's convenient to permit command-line options to be repeated
> > so that groups of options can be collected in shell or make variables
> > without having to worry about conflicts between groups. On the other hand,
> > I'm having trouble imagining that use case for -verify options, which I
> > believe would normally appear directly in command lines in test source
> > files. Have you seen use cases (perhaps with FileCheck) where it would be
> > useful?
> >
> > I see three reasons not to permit duplicate explicit prefixes:
> >
> > 1. Not permitting duplicates is consistent with FileCheck's
> > --check-prefix[es].
> >
> > 2. If we change our mind, we can later relax the restriction without
> > breaking backward compatibility, but we cannot go the other direction.
> >
> > 3. Suppose a developer wants to extend an existing test case by adding new
> > -verify prefixes to existing clang command lines that already uses many
> > -verify prefixes. If the developer accidentally duplicates an existing
> > prefix, the test case surely will not behave as expected, but it should be
> > easier to understand what has gone wrong if the compiler complains about
> > duplicate prefixes.
> >
> > I'm not adamant about the current behavior, but I think we should consider
> > these points before deciding.
> >> If this were going to be an error, then it should have error message that
> >> explains the problem (e.g., duplicate --verify prefix).
> > Are you saying you prefer that to be in the error instead of the note
> > (where it is now)?
>
> No, I'm saying that if we retain this as an error at all, then it needs
> better text. I'd prefer it not be an error.
>
> > I see three reasons not to permit duplicate explicit prefixes:
>
> I don't have a strong opinion, but in general, prohibiting duplicating
> command-line options hurts composability of command lines and makes scripting
> more difficult. That's a general statement (i.e., not tied to this use case),
> but as a result, I feel that should be the default unless a good reason
> (technical or otherwise) is presented.
>
> In this case, checking for duplicates adds complexity to the implementation,
> and as far as I can tell, adds little value. Obviously, when writing checks,
> the author should check that they work. Moreover, the implementation will
> already complain if there are unmatched diagnostics, or if nothing matches,
> so the chance of accidentally mistyping the verify prefix seems low.
>
> > 1. Not permitting duplicates is consistent with FileCheck's
> > --check-prefix[es].
>
> I don't find this compelling. In part, this is because FileCheck can't
> complain about unmatched output (that wouldn't make sense), and so the chance
> of error with FileCheck is much higher.
>
> > 2. If we change our mind, we can later relax the restriction without
> > breaking backward compatibility, but we cannot go the other direction.
>
> Not for this kind of option, really. This is a tool for Clang developers. If
> we found in the future that allowing duplicates where a large source of
> errors, we'd just change it.
>
> > 3. Suppose a developer wants to extend an existing test case by adding new
> > -verify prefixes to existing clang command lines that already uses many
> > -verify prefixes. If the developer accidentally duplicates an existing
> > prefix, the test case surely will not behave as expected, but it should be
> > easier to understand what has gone wrong if the compiler complains about
> > duplicate prefixes.
>
> If someone else feels strongly about this, please speak up. I don't see this
> as worth the implementation complexity nor sufficient justification to
> override what I see as the best practice of allowing duplicate options.
>
> >
> > I'm not adamant
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
hfinkel added inline comments.
Comment at: lib/Frontend/CompilerInvocation.cpp:1095
+ if (Diags) {
+Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+Diags->Report(diag::note_drv_verify_prefix_unique);
jdenny wrote:
> hfinkel wrote:
> > If this were going to be an error, then it should have error message that
> > explains the problem (e.g., duplicate --verify prefix). I don't believe
> > that we should make this an error (i.e., I think that we should allow
> > duplicate --verify options with the same prefixes).
> > If this were going to be an error, then it should have error message that
> > explains the problem (e.g., duplicate --verify prefix).
>
> Are you saying you prefer that to be in the error instead of the note (where
> it is now)?
>
> > I don't believe that we should make this an error (i.e., I think that we
> > should allow duplicate --verify options with the same prefixes).
>
> I see two reasons to permit duplicate explicit prefixes:
>
> 1. It simplifies the documentation some (see previous comment).
>
> 2. Typically, it's convenient to permit command-line options to be repeated
> so that groups of options can be collected in shell or make variables without
> having to worry about conflicts between groups. On the other hand, I'm
> having trouble imagining that use case for -verify options, which I believe
> would normally appear directly in command lines in test source files. Have
> you seen use cases (perhaps with FileCheck) where it would be useful?
>
> I see three reasons not to permit duplicate explicit prefixes:
>
> 1. Not permitting duplicates is consistent with FileCheck's
> --check-prefix[es].
>
> 2. If we change our mind, we can later relax the restriction without breaking
> backward compatibility, but we cannot go the other direction.
>
> 3. Suppose a developer wants to extend an existing test case by adding new
> -verify prefixes to existing clang command lines that already uses many
> -verify prefixes. If the developer accidentally duplicates an existing
> prefix, the test case surely will not behave as expected, but it should be
> easier to understand what has gone wrong if the compiler complains about
> duplicate prefixes.
>
> I'm not adamant about the current behavior, but I think we should consider
> these points before deciding.
>> If this were going to be an error, then it should have error message that
>> explains the problem (e.g., duplicate --verify prefix).
> Are you saying you prefer that to be in the error instead of the note (where
> it is now)?
No, I'm saying that if we retain this as an error at all, then it needs better
text. I'd prefer it not be an error.
> I see three reasons not to permit duplicate explicit prefixes:
I don't have a strong opinion, but in general, prohibiting duplicating
command-line options hurts composability of command lines and makes scripting
more difficult. That's a general statement (i.e., not tied to this use case),
but as a result, I feel that should be the default unless a good reason
(technical or otherwise) is presented.
In this case, checking for duplicates adds complexity to the implementation,
and as far as I can tell, adds little value. Obviously, when writing checks,
the author should check that they work. Moreover, the implementation will
already complain if there are unmatched diagnostics, or if nothing matches, so
the chance of accidentally mistyping the verify prefix seems low.
> 1. Not permitting duplicates is consistent with FileCheck's
> --check-prefix[es].
I don't find this compelling. In part, this is because FileCheck can't complain
about unmatched output (that wouldn't make sense), and so the chance of error
with FileCheck is much higher.
> 2. If we change our mind, we can later relax the restriction without breaking
> backward compatibility, but we cannot go the other direction.
Not for this kind of option, really. This is a tool for Clang developers. If we
found in the future that allowing duplicates where a large source of errors,
we'd just change it.
> 3. Suppose a developer wants to extend an existing test case by adding new
> -verify prefixes to existing clang command lines that already uses many
> -verify prefixes. If the developer accidentally duplicates an existing
> prefix, the test case surely will not behave as expected, but it should be
> easier to understand what has gone wrong if the compiler complains about
> duplicate prefixes.
If someone else feels strongly about this, please speak up. I don't see this as
worth the implementation complexity nor sufficient justification to override
what I see as the best practice of allowing duplicate options.
>
> I'm not adamant about the current behavior, but I think we should consider
> these points before deciding.
Sure.
https://reviews.llvm.org/D39694
___
cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny added a comment.
In https://reviews.llvm.org/D39694#944642, @hfinkel wrote:
> I think this is a good idea.
Thanks for the review. I've replied to each comment and will make revisions
after we agree on the behavioral issue you raised.
Comment at: include/clang/Driver/CC1Options.td:407
def verify : Flag<["-"], "verify">,
- HelpText<"Verify diagnostic output using comment directives">;
+ HelpText<"Similar to -verify=expected">;
def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">,
hfinkel wrote:
> "Similar to" seems unfortunately vague. Can it say, "Equivalent to ..."?
I agree I should have made it clearer.
"Equivalent to -verify=expected" works if we decide that duplicate explicit
prefixes are permitted, as you've suggested in a later comment.
With the current implementation, it should be "All occurrences together are
equivalent to one occurrence of -verify=expected". That is, I chose to permit
multiple occurrences of -verify without explicit prefixes for backward
compatibility, but I chose not to permit duplicate explicit prefixes for
reasons I'll discuss in the other comment.
I'll clean up the documentation once we agree on the right behavior.
Comment at: lib/Frontend/CompilerInvocation.cpp:1081
+[](char C){return !isAlphanumeric(C)
+ && C!='-' && C!='_';});
+if (BadChar != Prefix.end() || !isLetter(Prefix[0])) {
hfinkel wrote:
> I'd prefer to have spaces around the != operators.
Will change.
Comment at: lib/Frontend/CompilerInvocation.cpp:1095
+ if (Diags) {
+Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+Diags->Report(diag::note_drv_verify_prefix_unique);
hfinkel wrote:
> If this were going to be an error, then it should have error message that
> explains the problem (e.g., duplicate --verify prefix). I don't believe that
> we should make this an error (i.e., I think that we should allow duplicate
> --verify options with the same prefixes).
> If this were going to be an error, then it should have error message that
> explains the problem (e.g., duplicate --verify prefix).
Are you saying you prefer that to be in the error instead of the note (where it
is now)?
> I don't believe that we should make this an error (i.e., I think that we
> should allow duplicate --verify options with the same prefixes).
I see two reasons to permit duplicate explicit prefixes:
1. It simplifies the documentation some (see previous comment).
2. Typically, it's convenient to permit command-line options to be repeated so
that groups of options can be collected in shell or make variables without
having to worry about conflicts between groups. On the other hand, I'm having
trouble imagining that use case for -verify options, which I believe would
normally appear directly in command lines in test source files. Have you seen
use cases (perhaps with FileCheck) where it would be useful?
I see three reasons not to permit duplicate explicit prefixes:
1. Not permitting duplicates is consistent with FileCheck's --check-prefix[es].
2. If we change our mind, we can later relax the restriction without breaking
backward compatibility, but we cannot go the other direction.
3. Suppose a developer wants to extend an existing test case by adding new
-verify prefixes to existing clang command lines that already uses many -verify
prefixes. If the developer accidentally duplicates an existing prefix, the
test case surely will not behave as expected, but it should be easier to
understand what has gone wrong if the compiler complains about duplicate
prefixes.
I'm not adamant about the current behavior, but I think we should consider
these points before deciding.
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:233
// Return true if string literal is found.
// When true, P marks begin-position of S in content.
+ bool Search(StringRef S, bool EnsureStartOfWord = false,
hfinkel wrote:
> Please document here what FinishDirectiveWord means (and, while you're at it,
> documenting what EnsureStartOfWord means would be nice too).
Sure, I'll work on it.
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:370
+bool NoDiag = false;
+{
+ StringRef DType;
hfinkel wrote:
> This block is to limit the scope of the DType StringRef? That doesn't seem
> worthwhile.
Will change.
https://reviews.llvm.org/D39694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
hfinkel added a comment.
I think this is a good idea.
Comment at: include/clang/Driver/CC1Options.td:407
def verify : Flag<["-"], "verify">,
- HelpText<"Verify diagnostic output using comment directives">;
+ HelpText<"Similar to -verify=expected">;
def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">,
"Similar to" seems unfortunately vague. Can it say, "Equivalent to ..."?
Comment at: lib/Frontend/CompilerInvocation.cpp:1081
+[](char C){return !isAlphanumeric(C)
+ && C!='-' && C!='_';});
+if (BadChar != Prefix.end() || !isLetter(Prefix[0])) {
I'd prefer to have spaces around the != operators.
Comment at: lib/Frontend/CompilerInvocation.cpp:1095
+ if (Diags) {
+Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+Diags->Report(diag::note_drv_verify_prefix_unique);
If this were going to be an error, then it should have error message that
explains the problem (e.g., duplicate --verify prefix). I don't believe that we
should make this an error (i.e., I think that we should allow duplicate
--verify options with the same prefixes).
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:233
// Return true if string literal is found.
// When true, P marks begin-position of S in content.
+ bool Search(StringRef S, bool EnsureStartOfWord = false,
Please document here what FinishDirectiveWord means (and, while you're at it,
documenting what EnsureStartOfWord means would be nice too).
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:370
+bool NoDiag = false;
+{
+ StringRef DType;
This block is to limit the scope of the DType StringRef? That doesn't seem
worthwhile.
https://reviews.llvm.org/D39694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny updated this revision to Diff 125223.
jdenny added a comment.
Rebased on master/trunk fetched today.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/verify-prefixes.c
test/Sema/tautological-constant-compare.c
test/Sema/tautological-constant-enum-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
unsigned uvalue(void);
signed int svalue(void);
@@ -13,13 +13,8 @@
void TFunc() {
// Make sure that we do warn for normal variables in template functions !
unsigned char c = svalue();
-#ifdef TEST
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
return;
-#else
- if (c < 0)
- return;
-#endif
if (c < macro(0))
return;
@@ -39,7 +34,8 @@
unsigned un = uvalue();
-#ifdef TEST
+ // silence-no-diagnostics
+
if (un == 0)
return 0;
if (un != 0)
@@ -91,65 +87,10 @@
return 0;
if (0UL >= un)
return 0;
-#else
-// expected-no-diagnostics
- if (un == 0)
- return 0;
- if (un != 0)
- return 0;
- if (un < 0)
- return 0;
- if (un <= 0)
- return 0;
- if (un > 0)
- return 0;
- if (un >= 0)
- return 0;
-
- if (0 == un)
- return 0;
- if (0 != un)
- return 0;
- if (0 < un)
- return 0;
- if (0 <= un)
- return 0;
- if (0 > un)
- return 0;
- if (0 >= un)
- return 0;
-
- if (un == 0UL)
- return 0;
- if (un != 0UL)
- return 0;
- if (un < 0UL)
- return 0;
- if (un <= 0UL)
- return 0;
- if (un > 0UL)
- return 0;
- if (un >= 0UL)
- return 0;
-
- if (0UL == un)
- return 0;
- if (0UL != un)
- return 0;
- if (0UL < un)
- return 0;
- if (0UL <= un)
- return 0;
- if (0UL > un)
- return 0;
- if (0UL >= un)
- return 0;
-#endif
signed int a = svalue();
-#ifdef TEST
if (a == 0)
return 0;
if (a != 0)
@@ -201,60 +142,6 @@
return 0;
if (0UL >= a)
return 0;
-#else
-// expected-no-diagnostics
- if (a == 0)
- return 0;
- if (a != 0)
- return 0;
- if (a < 0)
- return 0;
- if (a <= 0)
- return 0;
- if (a > 0)
- return 0;
- if (a >= 0)
- return 0;
-
- if (0 == a)
- return 0;
- if (0 != a)
- return 0;
- if (0 < a)
- return 0;
- if (0 <= a)
- return 0;
- if (0 > a)
- return 0;
- if (0 >= a)
- return 0;
-
- if (a == 0UL)
- return 0;
- if (a != 0UL)
- return 0;
- if (a < 0UL)
- return 0;
- if (a <= 0UL)
- return 0;
- if (a > 0UL)
- return 0;
- if (a >= 0UL)
- return 0;
-
- if (0UL == a)
- return 0;
- if (0UL != a)
- return 0;
- if (0UL < a)
- return 0;
- if (0UL <= a)
- return 0;
- if (0UL > a)
- return 0;
- if (0UL >= a)
- return 0;
-#endif
float fl = 0;
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-verify=unsigned,unsigned-signed,expected %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-verify=unsigned-signed,signed-silence,expected %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-Wno-tautological-unsigned-enum-zero-compare \
+// RUN:
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny updated this revision to Diff 123027.
jdenny added a comment.
1. Capitalized some of the new local variables according to coding standards.
2. Rebased on master/trunk fetched today.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/verify-prefixes.c
test/Sema/tautological-constant-compare.c
test/Sema/tautological-constant-enum-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
unsigned uvalue(void);
signed int svalue(void);
@@ -13,13 +13,8 @@
void TFunc() {
// Make sure that we do warn for normal variables in template functions !
unsigned char c = svalue();
-#ifdef TEST
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
return;
-#else
- if (c < 0)
- return;
-#endif
if (c < macro(0))
return;
@@ -39,7 +34,8 @@
unsigned un = uvalue();
-#ifdef TEST
+ // silence-no-diagnostics
+
if (un == 0)
return 0;
if (un != 0)
@@ -91,65 +87,10 @@
return 0;
if (0UL >= un)
return 0;
-#else
-// expected-no-diagnostics
- if (un == 0)
- return 0;
- if (un != 0)
- return 0;
- if (un < 0)
- return 0;
- if (un <= 0)
- return 0;
- if (un > 0)
- return 0;
- if (un >= 0)
- return 0;
-
- if (0 == un)
- return 0;
- if (0 != un)
- return 0;
- if (0 < un)
- return 0;
- if (0 <= un)
- return 0;
- if (0 > un)
- return 0;
- if (0 >= un)
- return 0;
-
- if (un == 0UL)
- return 0;
- if (un != 0UL)
- return 0;
- if (un < 0UL)
- return 0;
- if (un <= 0UL)
- return 0;
- if (un > 0UL)
- return 0;
- if (un >= 0UL)
- return 0;
-
- if (0UL == un)
- return 0;
- if (0UL != un)
- return 0;
- if (0UL < un)
- return 0;
- if (0UL <= un)
- return 0;
- if (0UL > un)
- return 0;
- if (0UL >= un)
- return 0;
-#endif
signed int a = svalue();
-#ifdef TEST
if (a == 0)
return 0;
if (a != 0)
@@ -201,60 +142,6 @@
return 0;
if (0UL >= a)
return 0;
-#else
-// expected-no-diagnostics
- if (a == 0)
- return 0;
- if (a != 0)
- return 0;
- if (a < 0)
- return 0;
- if (a <= 0)
- return 0;
- if (a > 0)
- return 0;
- if (a >= 0)
- return 0;
-
- if (0 == a)
- return 0;
- if (0 != a)
- return 0;
- if (0 < a)
- return 0;
- if (0 <= a)
- return 0;
- if (0 > a)
- return 0;
- if (0 >= a)
- return 0;
-
- if (a == 0UL)
- return 0;
- if (a != 0UL)
- return 0;
- if (a < 0UL)
- return 0;
- if (a <= 0UL)
- return 0;
- if (a > 0UL)
- return 0;
- if (a >= 0UL)
- return 0;
-
- if (0UL == a)
- return 0;
- if (0UL != a)
- return 0;
- if (0UL < a)
- return 0;
- if (0UL <= a)
- return 0;
- if (0UL > a)
- return 0;
- if (0UL >= a)
- return 0;
-#endif
float fl = 0;
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only \
+// RUN:-verify=unsigned,unsigned-signed,expected %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:-verify=unsigned-signed,signed-silence,expected %s
+// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only \
+// RUN:
[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=
jdenny updated this revision to Diff 122172.
jdenny retitled this revision from "[VerifyDiagnosticConsumer] support
-verify=PREFIX" to "[VerifyDiagnosticConsumer] support -verify=".
jdenny edited the summary of this revision.
jdenny added a comment.
1. Extended -verify to accept multiple prefixes, like FileCheck's
--check-prefixes.
2. Demonstrated the benefits of this feature by using it to clean up existing
clang tests.
3. Rebased to a more recent master.
Now that this patch is non-trivial, I'll wait a bit for a review before making
more revisions.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/VerifyDiagnosticConsumer.cpp
test/Frontend/verify-prefixes.c
test/Sema/tautological-constant-compare.c
test/Sema/tautological-constant-enum-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.c
test/Sema/tautological-unsigned-enum-zero-compare.cpp
test/Sema/tautological-unsigned-zero-compare.c
Index: test/Sema/tautological-unsigned-zero-compare.c
===
--- test/Sema/tautological-unsigned-zero-compare.c
+++ test/Sema/tautological-unsigned-zero-compare.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify %s
-// RUN: %clang_cc1 -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-zero-compare -verify=silence -x c++ %s
unsigned uvalue(void);
signed int svalue(void);
@@ -13,13 +13,8 @@
void TFunc() {
// Make sure that we do warn for normal variables in template functions !
unsigned char c = svalue();
-#ifdef TEST
if (c < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
return;
-#else
- if (c < 0)
- return;
-#endif
if (c < macro(0))
return;
@@ -39,7 +34,8 @@
unsigned un = uvalue();
-#ifdef TEST
+ // silence-no-diagnostics
+
if (un == 0)
return 0;
if (un != 0)
@@ -91,65 +87,10 @@
return 0;
if (0UL >= un)
return 0;
-#else
-// expected-no-diagnostics
- if (un == 0)
- return 0;
- if (un != 0)
- return 0;
- if (un < 0)
- return 0;
- if (un <= 0)
- return 0;
- if (un > 0)
- return 0;
- if (un >= 0)
- return 0;
-
- if (0 == un)
- return 0;
- if (0 != un)
- return 0;
- if (0 < un)
- return 0;
- if (0 <= un)
- return 0;
- if (0 > un)
- return 0;
- if (0 >= un)
- return 0;
-
- if (un == 0UL)
- return 0;
- if (un != 0UL)
- return 0;
- if (un < 0UL)
- return 0;
- if (un <= 0UL)
- return 0;
- if (un > 0UL)
- return 0;
- if (un >= 0UL)
- return 0;
-
- if (0UL == un)
- return 0;
- if (0UL != un)
- return 0;
- if (0UL < un)
- return 0;
- if (0UL <= un)
- return 0;
- if (0UL > un)
- return 0;
- if (0UL >= un)
- return 0;
-#endif
signed int a = svalue();
-#ifdef TEST
if (a == 0)
return 0;
if (a != 0)
@@ -201,60 +142,6 @@
return 0;
if (0UL >= a)
return 0;
-#else
-// expected-no-diagnostics
- if (a == 0)
- return 0;
- if (a != 0)
- return 0;
- if (a < 0)
- return 0;
- if (a <= 0)
- return 0;
- if (a > 0)
- return 0;
- if (a >= 0)
- return 0;
-
- if (0 == a)
- return 0;
- if (0 != a)
- return 0;
- if (0 < a)
- return 0;
- if (0 <= a)
- return 0;
- if (0 > a)
- return 0;
- if (0 >= a)
- return 0;
-
- if (a == 0UL)
- return 0;
- if (a != 0UL)
- return 0;
- if (a < 0UL)
- return 0;
- if (a <= 0UL)
- return 0;
- if (a > 0UL)
- return 0;
- if (a >= 0UL)
- return 0;
-
- if (0UL == a)
- return 0;
- if (0UL != a)
- return 0;
- if (0UL < a)
- return 0;
- if (0UL <= a)
- return 0;
- if (0UL > a)
- return 0;
- if (0UL >= a)
- return 0;
-#endif
float fl = 0;
Index: test/Sema/tautological-unsigned-enum-zero-compare.cpp
===
--- test/Sema/tautological-unsigned-enum-zero-compare.cpp
+++ test/Sema/tautological-unsigned-enum-zero-compare.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
+// RUN:
