[VerifyDiagnosticConsumer] support -verify=<prefixes>

[jdenny]

Summary

This mimics FileCheck's --check-prefixes option.

The default prefix is "expected". That is, "-verify" is equivalent to
"-verify=expected".

The goal is to permit exercising a single test suite source file with
different compiler options producing different sets of diagnostics.
While cpp can be combined with the existing -verify to accomplish the
same goal, source is often easier to maintain when it's not cluttered
with preprocessor directives or duplicate passages of code. For
example, this patch also rewrites some existing clang tests to
demonstrate the benefit of this feature.

[jdenny]

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.

[jdenny]

1. Capitalized some of the new local variables according to coding standards.

2. Rebased on master/trunk fetched today.

[jdenny]

Rebased on master/trunk fetched today.

[hfinkel]

I think this is a good idea.

include/clang/Driver/CC1Options.td

407

"Similar to" seems unfortunately vague. Can it say, "Equivalent to ..."?

lib/Frontend/CompilerInvocation.cpp

1081

I'd prefer to have spaces around the != operators.

1095

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

lib/Frontend/VerifyDiagnosticConsumer.cpp

231–232

Please document here what FinishDirectiveWord means (and, while you're at it, documenting what EnsureStartOfWord means would be nice too).

387

This block is to limit the scope of the DType StringRef? That doesn't seem worthwhile.

[jdenny]

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

include/clang/Driver/CC1Options.td

407

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.

lib/Frontend/CompilerInvocation.cpp

1081

Will change.

1095

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.

lib/Frontend/VerifyDiagnosticConsumer.cpp

231–232

Sure, I'll work on it.

387

Will change.

[hfinkel]

lib/Frontend/CompilerInvocation.cpp

1095

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.

[jdenny]

lib/Frontend/CompilerInvocation.cpp

1095

I'm not sure why, but Phabricator doesn't give me the menu option to quote your comment in full this time, so I'll copy and paste just the most important lines.

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.

Does the text for spelling errors, which are diagnosed immediately before this, look good to you?

In this case, checking for duplicates adds complexity to the implementation

But really not very much.

FileCheck can't complain about unmatched output (that wouldn't make sense), and so the chance of error with FileCheck is much higher.

That's an important distinction I hadn't appreciated.

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.

That argument convinces me. Unless someone objects before I get to it, I'll remove the restriction for duplicates.

[jdenny]

This update includes all of Hal's suggestions.

I'm also thinking of converting prefix storage from a std::vector to a std::set so that lookup should be faster during parsing.

[rsmith]

I've not done a detailed review of the string manipulation here, but this looks like a great feature, thanks!

include/clang/Basic/DiagnosticDriverKinds.td

342

Please wrap this to 80 columns.

include/clang/Driver/CC1Options.td

407

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.

[jdenny]

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.

[jdenny]

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

[jdenny]

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

[hfinkel]

lib/Frontend/VerifyDiagnosticConsumer.cpp

398

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.

[jdenny]

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.

[jdenny]

lib/Frontend/VerifyDiagnosticConsumer.cpp

398

Thanks.

[hfinkel]

LGTM

[jdenny]

In D39694#956661, @hfinkel wrote:

LGTM

Thanks for accepting. I don't have commit privileges. Would you please commit for me? This depends on D40995, which also needs to be committed.

include/clang/Basic/DiagnosticDriverKinds.td

 def warn_drv_msvc_not_found : Warning<
   "unable to find a Visual Studio installation; "
   "try running Clang from a developer command prompt">,
   InGroup<DiagGroup<"msvc-not-found">>;
 
 def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
   "option '-ffine-grained-bitfield-accesses' cannot be enabled together with a sanitizer; flag ignored">,
   InGroup<OptionIgnored>;
+
+def note_drv_verify_prefix_spelling : Note<
+  "-verify prefixes must start with a letter and contain only alphanumeric"
+  " characters, hyphens, and underscores">;
 }

include/clang/Basic/DiagnosticOptions.h

 //===--- DiagnosticOptions.h ------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_DIAGNOSTICOPTIONS_H
 #define LLVM_CLANG_BASIC_DIAGNOSTICOPTIONS_H
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include <set>
 #include <string>
 #include <type_traits>
 #include <vector>
 
 namespace clang {
 
 /// \brief Specifies which overload candidates to display when overload
 /// resolution fails.
public:
   /// The list of -W... options used to alter the diagnostic mappings, with the
   /// prefixes removed.
   std::vector<std::string> Warnings;
 
   /// The list of -R... options used to alter the diagnostic mappings, with the
   /// prefixes removed.
   std::vector<std::string> Remarks;
 
+  /// The prefixes for comment directives sought by -verify ("expected" by
+  /// default).
+  std::set<std::string> VerifyPrefixes;
+
 public:
   // Define accessors/mutators for diagnostic options of enumeration type.
 #define DIAGOPT(Name, Bits, Default)
 #define ENUM_DIAGOPT(Name, Type, Bits, Default) \
   Type get##Name() const { return static_cast<Type>(Name); } \
   void set##Name(Type Value) { Name = static_cast<unsigned>(Value); }
 #include "clang/Basic/DiagnosticOptions.def"
 

include/clang/Driver/CC1Options.td

def fconstexpr_backtrace_limit : Separate<["-"], "fconstexpr-backtrace-limit">, MetaVarName<"<N>">,
   HelpText<"Set the maximum number of entries to print in a constexpr evaluation backtrace (0 = no limit).">;
 def fspell_checking_limit : Separate<["-"], "fspell-checking-limit">, MetaVarName<"<N>">,
   HelpText<"Set the maximum number of times to perform spell checking on unrecognized identifiers (0 = no limit).">;
 def fcaret_diagnostics_max_lines :
   Separate<["-"], "fcaret-diagnostics-max-lines">, MetaVarName<"<N>">,
   HelpText<"Set the maximum number of source lines to show in a caret diagnostic">;
 def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"<N>">,
   HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">;
+def verify_EQ : CommaJoined<["-"], "verify=">,
+  MetaVarName<"<prefixes>">,
+  HelpText<"Verify diagnostic output using comment directives that start with prefixes in the comma-separated sequence <prefixes>">;
 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=">,
   HelpText<"Ignore unexpected diagnostic messages">;
 def Wno_rewrite_macros : Flag<["-"], "Wno-rewrite-macros">,
   HelpText<"Silence ObjC rewriting warnings">;
 
 //===----------------------------------------------------------------------===//

lib/Frontend/CompilerInvocation.cpp

if (O.matches(options::OPT_fcolor_diagnostics) ||
         ShowColors = Colors_Auto;
     }
   }
   return ShowColors == Colors_On ||
          (ShowColors == Colors_Auto &&
           llvm::sys::Process::StandardErrHasColors());
 }
 
+static bool checkVerifyPrefixes(const std::vector<std::string> &VerifyPrefixes,
+                                DiagnosticsEngine *Diags) {
+  bool Success = true;
+  for (const auto &Prefix : VerifyPrefixes) {
+    // Every prefix must start with a letter and contain only alphanumeric
+    // characters, hyphens, and underscores.
+    auto BadChar = std::find_if(Prefix.begin(), Prefix.end(),
+                                [](char C){return !isAlphanumeric(C)
+                                                  && C != '-' && C != '_';});
+    if (BadChar != Prefix.end() || !isLetter(Prefix[0])) {
+      Success = false;
+      if (Diags) {
+        Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+        Diags->Report(diag::note_drv_verify_prefix_spelling);
+      }
+    }
+  }
+  return Success;
+}
+
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 DiagnosticsEngine *Diags,
                                 bool DefaultDiagColor, bool DefaultShowOpt) {
   using namespace options;
   bool Success = true;
 
   Opts.DiagnosticLogFile = Args.getLastArgValue(OPT_diagnostic_log_file);
   if (Arg *A =
           Args.getLastArg(OPT_diagnostic_serialized_file, OPT__serialize_diags))
     Opts.DiagnosticSerializationFile = A->getValue();
   Opts.IgnoreWarnings = Args.hasArg(OPT_w);
   Opts.NoRewriteMacros = Args.hasArg(OPT_Wno_rewrite_macros);
if (Diags)
       Diags->Report(diag::err_drv_invalid_value)
       << Args.getLastArg(OPT_fdiagnostics_format)->getAsString(Args)
       << Format;
   }
 
   Opts.ShowSourceRanges = Args.hasArg(OPT_fdiagnostics_print_source_range_info);
   Opts.ShowParseableFixits = Args.hasArg(OPT_fdiagnostics_parseable_fixits);
   Opts.ShowPresumedLoc = !Args.hasArg(OPT_fno_diagnostics_use_presumed_location);
-  Opts.VerifyDiagnostics = Args.hasArg(OPT_verify);
+  Opts.VerifyDiagnostics = Args.hasArg(OPT_verify) || Args.hasArg(OPT_verify_EQ);
+  auto VerifyPrefixes = Args.getAllArgValues(OPT_verify_EQ);
+  if (Args.hasArg(OPT_verify))
+    VerifyPrefixes.push_back("expected");
+  if (!checkVerifyPrefixes(VerifyPrefixes, Diags)) {
+    Opts.VerifyDiagnostics = false;
+    Success = false;
+  }
+  else {
+    // Convert VerifyPrefixes from vector (for diagnostic order) to set (for
+    // fast lookup).
+    std::copy(VerifyPrefixes.begin(), VerifyPrefixes.end(),
+              std::inserter(Opts.VerifyPrefixes, Opts.VerifyPrefixes.begin()));
+  }
   DiagnosticLevelMask DiagMask = DiagnosticLevelMask::None;
   Success &= parseDiagnosticLevelMask("-verify-ignore-unexpected=",
     Args.getAllArgValues(OPT_verify_ignore_unexpected_EQ),
     Diags, DiagMask);
   if (Args.hasArg(OPT_verify_ignore_unexpected))
     DiagMask = DiagnosticLevelMask::All;
   Opts.setVerifyIgnoreUnexpected(DiagMask);
   Opts.ElideType = !Args.hasArg(OPT_fno_elide_type);

lib/Frontend/VerifyDiagnosticConsumer.cpp

for (; P < End && P[0] >= '0' && P[0] <= '9'; ++P) {
       TMP += P[0] - '0';
     }
     if (P == C)
       return false;
     PEnd = P;
     N = TMP;
     return true;
   }
 
-  // Return true if string literal is found.
-  // When true, P marks begin-position of S in content.
-  bool Search(StringRef S, bool EnsureStartOfWord = false) {
+  // Return true if string literal S is matched in content.
+  // When true, P marks begin-position of the match, and calling Advance sets C
+  // to end-position of the match.
+  // If S is the empty string, then search for any letter instead (makes sense
+  // with FinishDirectiveToken=true).
+  // If EnsureStartOfWord, then skip matches that don't start a new word.
+  // If FinishDirectiveToken, then assume the match is the start of a comment
+  // directive for -verify, and extend the match to include the entire first
+  // token of that directive.
+  bool Search(StringRef S, bool EnsureStartOfWord = false,
+              bool FinishDirectiveToken = false) {
     do {
+      if (!S.empty()) {
         P = std::search(C, End, S.begin(), S.end());
         PEnd = P + S.size();
+      }
+      else {
+        P = C;
+        while (P != End && !isLetter(*P))
+          ++P;
+        PEnd = P + 1;
+      }
       if (P == End)
         break;
-      if (!EnsureStartOfWord
+      // If not start of word but required, skip and search again.
+      if (EnsureStartOfWord
                // Check if string literal starts a new word.
-            || P == Begin || isWhitespace(P[-1])
+          && !(P == Begin || isWhitespace(P[-1])
                // Or it could be preceded by the start of a comment.
                || (P > (Begin + 1) && (P[-1] == '/' || P[-1] == '*')
-                                &&  P[-2] == '/'))
-        return true;
-      // Otherwise, skip and search again.
+                                   &&  P[-2] == '/')))
+        continue;
+      if (FinishDirectiveToken) {
+        while (PEnd != End && (isAlphanumeric(*PEnd)
+                               || *PEnd == '-' || *PEnd == '_'))
+          ++PEnd;
+        // Put back trailing digits and hyphens to be parsed later as a count
+        // or count range.  Because -verify prefixes must start with letters,
+        // we know the actual directive we found starts with a letter, so
+        // we won't put back the entire directive word and thus record an empty
+        // string.
+        assert(isLetter(*P) && "-verify prefix must start with a letter");
+        while (isDigit(PEnd[-1]) || PEnd[-1] == '-')
+          --PEnd;
+      }
     } while (Advance());
     return false;
   }
 
   // Return true if a CloseBrace that closes the OpenBrace at the current nest
   // level is found. When true, P marks begin-position of CloseBrace.
   bool SearchClosingBrace(StringRef OpenBrace, StringRef CloseBrace) {
     unsigned Depth = 1;

 static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
                            Preprocessor *PP, SourceLocation Pos,
                            VerifyDiagnosticConsumer::DirectiveStatus &Status) {
   DiagnosticsEngine &Diags = PP ? PP->getDiagnostics() : SM.getDiagnostics();
 
   // A single comment may contain multiple directives.
   bool FoundDirective = false;
   for (ParseHelper PH(S); !PH.Done();) {
-    // Search for token: expected
-    if (!PH.Search("expected", true))
+    // Search for the initial directive token.
+    // If one prefix, save time by searching only for its directives.
+    // Otherwise, search for any potential directive token and check it later.
+    const auto &Prefixes = Diags.getDiagnosticOptions().VerifyPrefixes;
+    if (!(Prefixes.size() == 1 ? PH.Search(*Prefixes.begin(), true, true)
+                               : PH.Search("", true, true)))
       break;
     PH.Advance();
 
-    // Next token: -
-    if (!PH.Next("-"))
-      continue;
-    PH.Advance();
+    // Default directive kind.
+    bool RegexKind = false;
+    const char* KindStr = "string";
+
+    // Parse the initial directive token in reverse so we can easily determine
+    // its exact actual prefix.  If we were to parse it from the front instead,
+    // it would be harder to determine where the prefix ends because there
+    // might be multiple matching -verify prefixes because some might prefix
+    // others.
+    StringRef DToken(PH.P, PH.C - PH.P);
 
-    // Next token: { error | warning | note }
+    // Regex in initial directive token: -re
+    if (DToken.endswith("-re")) {
+      RegexKind = true;
+      KindStr = "regex";
+      DToken = DToken.substr(0, DToken.size()-3);
+    }
+
+    // Type in initial directive token: -{error|warning|note|no-diagnostics}
     DirectiveList *DL = nullptr;
-    if (PH.Next("error"))
+    bool NoDiag = false;
+    StringRef DType;
+    if (DToken.endswith(DType="-error"))
       DL = ED ? &ED->Errors : nullptr;
-    else if (PH.Next("warning"))
+    else if (DToken.endswith(DType="-warning"))
       DL = ED ? &ED->Warnings : nullptr;
-    else if (PH.Next("remark"))
+    else if (DToken.endswith(DType="-remark"))
       DL = ED ? &ED->Remarks : nullptr;
-    else if (PH.Next("note"))
+    else if (DToken.endswith(DType="-note"))
       DL = ED ? &ED->Notes : nullptr;
-    else if (PH.Next("no-diagnostics")) {
+    else if (DToken.endswith(DType="-no-diagnostics")) {
+      NoDiag = true;
+      if (RegexKind)
+        continue;
+    }
+    else
+      continue;
+    DToken = DToken.substr(0, DToken.size()-DType.size());
+
+    // What's left in DToken is the actual prefix.  That might not be a -verify
+    // prefix even if there is only one -verify prefix (for example, the full
+    // DToken is foo-bar-warning, but foo is the only -verify prefix).
+    if (Prefixes.end() == std::find(Prefixes.begin(), Prefixes.end(), DToken))
+      continue;
+
+    if (NoDiag) {
       if (Status == VerifyDiagnosticConsumer::HasOtherExpectedDirectives)
         Diags.Report(Pos, diag::err_verify_invalid_no_diags)
           << /*IsExpectedNoDiagnostics=*/true;
       else
         Status = VerifyDiagnosticConsumer::HasExpectedNoDiagnostics;
       continue;
-    } else
-      continue;
-    PH.Advance();
-
+    }
     if (Status == VerifyDiagnosticConsumer::HasExpectedNoDiagnostics) {
       Diags.Report(Pos, diag::err_verify_invalid_no_diags)
         << /*IsExpectedNoDiagnostics=*/false;
       continue;
     }
     Status = VerifyDiagnosticConsumer::HasOtherExpectedDirectives;
 
     // If a directive has been found but we're not interested
     // in storing the directive information, return now.
     if (!DL)
       return true;
 
-    // Default directive kind.
-    bool RegexKind = false;
-    const char* KindStr = "string";
-
-    // Next optional token: -
-    if (PH.Next("-re")) {
-      PH.Advance();
-      RegexKind = true;
-      KindStr = "regex";
-    }
-
     // Next optional token: @
     SourceLocation ExpectedLoc;
     bool MatchAnyLine = false;
     if (!PH.Next("@")) {
       ExpectedLoc = Pos;
     } else {
       PH.Advance();
       unsigned Line = 0;

test/Frontend/diagnostics-order.c

 // Make sure a note stays with its associated command-line argument diagnostic.
 // Previously, these diagnostics were grouped by diagnostic level with all
 // notes last.
 //
-// RUN: not %clang_cc1 -O999 -std=bogus %s 2> %t
+// RUN: not %clang_cc1 -O999 -std=bogus -verify=-foo %s 2> %t
 // RUN: FileCheck < %t %s
 //
-// CHECK: warning: optimization level '-O999' is not supported
+// CHECK:      error: invalid value '-foo' in '-verify='
+// CHECK-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
+// CHECK-NEXT: warning: optimization level '-O999' is not supported
 // CHECK-NEXT: error: invalid value 'bogus' in '-std=bogus'
 // CHECK-NEXT: note: use {{.*}} for {{.*}} standard

test/Frontend/verify-prefixes.c

+#if GC
+# define GCONST const
+#else
+# define GCONST
+#endif
+
+// gconst-note@8 {{variable 'glb' declared const here}}
+GCONST int glb = 5;
+
+
+// Check various correct prefix spellings and combinations.
+//
+// RUN: %clang_cc1             -DGC           -verify=gconst                 %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC      -verify=lconst                 %s
+// RUN: %clang_cc1                       -DSC -verify=expected               %s
+// RUN: %clang_cc1                       -DSC -verify                        %s
+// RUN: %clang_cc1                       -DSC -verify -verify                %s
+// RUN: %clang_cc1                            -verify=nconst                 %s
+// RUN: %clang_cc1                            -verify=n-const                %s
+// RUN: %clang_cc1                            -verify=n_const                %s
+// RUN: %clang_cc1                            -verify=NConst                 %s
+// RUN: %clang_cc1                            -verify=NConst2                %s
+// RUN: %clang_cc1 -Wcast-qual -DGC -DLC      -verify=gconst,lconst          %s
+// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -DSC -verify=gconst,lconst,expected %s
+// RUN: %clang_cc1 -Wcast-qual -DGC -DLC      -verify=gconst -verify=lconst  %s
+// RUN: %clang_cc1 -Wcast-qual -DGC -DLC -DSC -verify=gconst,lconst -verify  %s
+// RUN: %clang_cc1             -DGC      -DSC -verify -verify=gconst -verify %s
+//
+// Duplicate prefixes.
+// RUN: %clang_cc1 -Wcast-qual -DGC -DLC      -verify=gconst,lconst,gconst         %s
+// RUN: %clang_cc1             -DGC           -verify=gconst -verify=gconst,gconst %s
+// RUN: %clang_cc1                       -DSC -verify=expected -verify=expected    %s
+// RUN: %clang_cc1                       -DSC -verify -verify=expected             %s
+//
+// Various tortured cases: multiple directives with different prefixes per
+// line, prefixes used as comments, prefixes prefixing prefixes, and prefixes
+// with special suffixes.
+// RUN: %clang_cc1 -Wcast-qual      -DLC      -verify=foo                    %s
+// RUN: %clang_cc1                       -DSC -verify=bar                    %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=foo,bar                %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=bar,foo                %s
+// RUN: %clang_cc1                       -DSC -verify=foo-bar                %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC      -verify=bar-foo                %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=foo,foo-bar            %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=foo-bar,foo            %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=bar,bar-foo            %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=bar-foo,bar            %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=foo-bar,bar-foo        %s
+// RUN: %clang_cc1                       -DSC -verify=foo-warning            %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC      -verify=bar-warning-re         %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=foo,foo-warning        %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=foo-warning,foo        %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=bar,bar-warning-re     %s
+// RUN: %clang_cc1 -Wcast-qual      -DLC -DSC -verify=bar-warning-re,bar     %s
+
+
+// Check invalid prefixes.  Check that there's no additional output, which
+// might indicate that diagnostic verification became enabled even though it
+// was requested incorrectly.  Check that prefixes are reported in command-line
+// order.
+//
+// RUN: not %clang_cc1 -verify=5abc,-xy,foo,_k -verify='#a,b$' %s 2> %t
+// RUN: FileCheck --check-prefixes=ERR %s < %t
+//
+// ERR-NOT:  {{.}}
+// ERR:      error: invalid value '5abc' in '-verify='
+// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
+// ERR-NEXT: error: invalid value '-xy' in '-verify='
+// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
+// ERR-NEXT: error: invalid value '_k' in '-verify='
+// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
+// ERR-NEXT: error: invalid value '#a' in '-verify='
+// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
+// ERR-NEXT: error: invalid value 'b$' in '-verify='
+// ERR-NEXT: note: -verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores
+// ERR-NOT:  {{.}}
+
+
+// Check that our test code actually has expected diagnostics when there's no
+// -verify.
+//
+// RUN: not %clang_cc1 -Wcast-qual -DGC -DLC -DSC %s 2> %t
+// RUN: FileCheck --check-prefix=ALL %s < %t
+//
+// ALL: cannot assign to variable 'glb' with const-qualified type 'const int'
+// ALL: variable 'glb' declared const here
+// ALL: cast from 'const int *' to 'int *' drops const qualifier
+// ALL: initializing 'int *' with an expression of type 'const int *' discards qualifiers
+
+
+#if LC
+# define LCONST const
+#else
+# define LCONST
+#endif
+
+#if SC
+# define SCONST const
+#else
+# define SCONST
+#endif
+
+void foo() {
+  LCONST int loc = 5;
+  SCONST static int sta = 5;
+  // We don't actually expect 1-2 occurrences of this error.  We're just
+  // checking the parsing.
+  glb = 6; // gconst-error1-2 {{cannot assign to variable 'glb' with const-qualified type 'const int'}}
+  *(int*)(&loc) = 6; // lconst-warning {{cast from 'const int *' to 'int *' drops const qualifier}}
+  ; // Code, comments, and many directives with different prefixes per line, including cases where some prefixes (foo and bar) prefix others (such as foo-bar and bar-foo), such that some prefixes appear as normal comments and some have special suffixes (-warning and -re): foo-warning@-1 {{cast from 'const int *' to 'int *' drops const qualifier}} foo-bar-warning@+1 {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}} foo-warning-warning@+1 {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}} bar-warning-re-warning@-1 {{cast from 'const int *' to 'int *' drops const qualifier}} bar-foo-warning@-1 {{cast from 'const int *' to 'int *' drops const qualifier}} bar-warning@+1 {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+  int *p = &sta; // expected-warning {{initializing 'int *' with an expression of type 'const int *' discards qualifiers}}
+}
+
+// nconst-no-diagnostics
+// n-const-no-diagnostics
+// n_const-no-diagnostics
+// NConst-no-diagnostics
+// NConst2-no-diagnostics

test/Sema/tautological-constant-compare.c

-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DTEST -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wno-tautological-constant-compare -verify=silence -x c++ %s
 
 int value(void);
 
 #define macro(val) val
 
 #ifdef __cplusplus
 template<typename T>
 void TFunc() {
   // Make sure that we do warn for normal variables in template functions !
   unsigned char c = value();
-#ifdef TEST
   if (c > 255) // expected-warning {{comparison 'unsigned char' > 255 is always false}}
       return;
-#else
-  if (c > 255)
-      return;
-#endif
 
   if (c > macro(255))
       return;
 
   T v = value();
   if (v > 255)
       return;
   if (v > 32767)
       return;
 }
 #endif
 
 int main()
 {
 #ifdef __cplusplus
   TFunc<unsigned char>();
   TFunc<signed short>();
 #endif
 
   short s = value();
 
-#ifdef TEST
+  // silence-no-diagnostics
+
   if (s == 32767)
       return 0;
   if (s != 32767)
       return 0;
   if (s < 32767)
       return 0;
   if (s <= 32767) // expected-warning {{comparison 'short' <= 32767 is always true}}
       return 0;
#endif
   if (-32768L < s)
       return 0;
   if (-32768L <= s) // expected-warning {{comparison -32768 <= 'short' is always true}}
       return 0;
   if (-32768L > s) // expected-warning {{comparison -32768 > 'short' is always false}}
       return 0;
   if (-32768L >= s)
       return 0;
-#else
-  // expected-no-diagnostics
-  if (s == 32767)
-    return 0;
-  if (s != 32767)
-    return 0;
-  if (s < 32767)
-    return 0;
-  if (s <= 32767)
-    return 0;
-  if (s > 32767)
-    return 0;
-  if (s >= 32767)
-    return 0;
-
-  if (32767 == s)
-    return 0;
-  if (32767 != s)
-    return 0;
-  if (32767 < s)
-    return 0;
-  if (32767 <= s)
-    return 0;
-  if (32767 > s)
-    return 0;
-  if (32767 >= s)
-    return 0;
-
-  // FIXME: assumes two's complement
-  if (s == -32768)
-    return 0;
-  if (s != -32768)
-    return 0;
-  if (s < -32768)
-    return 0;
-  if (s <= -32768)
-    return 0;
-  if (s > -32768)
-    return 0;
-  if (s >= -32768)
-    return 0;
-
-  if (-32768 == s)
-    return 0;
-  if (-32768 != s)
-    return 0;
-  if (-32768 < s)
-    return 0;
-  if (-32768 <= s)
-    return 0;
-  if (-32768 > s)
-    return 0;
-  if (-32768 >= s)
-    return 0;
-
-  if (s == 32767UL)
-    return 0;
-  if (s != 32767UL)
-    return 0;
-  if (s < 32767UL)
-    return 0;
-  if (s <= 32767UL)
-    return 0;
-  if (s > 32767UL)
-    return 0;
-  if (s >= 32767UL)
-    return 0;
-
-  if (32767UL == s)
-    return 0;
-  if (32767UL != s)
-    return 0;
-  if (32767UL < s)
-    return 0;
-  if (32767UL <= s)
-    return 0;
-  if (32767UL > s)
-    return 0;
-  if (32767UL >= s)
-    return 0;
-
-  // FIXME: assumes two's complement
-  if (s == -32768L)
-    return 0;
-  if (s != -32768L)
-    return 0;
-  if (s < -32768L)
-    return 0;
-  if (s <= -32768L)
-    return 0;
-  if (s > -32768L)
-    return 0;
-  if (s >= -32768L)
-    return 0;
-
-  if (-32768L == s)
-    return 0;
-  if (-32768L != s)
-    return 0;
-  if (-32768L < s)
-    return 0;
-  if (-32768L <= s)
-    return 0;
-  if (-32768L > s)
-    return 0;
-  if (-32768L >= s)
-    return 0;
-#endif
 
   if (s == 0)
     return 0;
   if (s != 0)
     return 0;
   if (s < 0)
     return 0;
   if (s <= 0)
if (0 > s)
     return 0;
   if (0 >= s)
     return 0;
 
   // However the comparison with 0U would warn
 
   unsigned short us = value();
 
-#ifdef TEST
   if (us == 65535)
       return 0;
   if (us != 65535)
       return 0;
   if (us < 65535)
       return 0;
   if (us <= 65535) // expected-warning {{comparison 'unsigned short' <= 65535 is always true}}
       return 0;
#endif
   if (65535UL < us) // expected-warning {{comparison 65535 < 'unsigned short' is always false}}
       return 0;
   if (65535UL <= us)
       return 0;
   if (65535UL > us)
       return 0;
   if (65535UL >= us) // expected-warning {{comparison 65535 >= 'unsigned short' is always true}}
       return 0;
-#else
-  // expected-no-diagnostics
-  if (us == 65535)
-      return 0;
-  if (us != 65535)
-      return 0;
-  if (us < 65535)
-      return 0;
-  if (us <= 65535)
-      return 0;
-  if (us > 65535)
-      return 0;
-  if (us >= 65535)
-      return 0;
-
-  if (65535 == us)
-      return 0;
-  if (65535 != us)
-      return 0;
-  if (65535 < us)
-      return 0;
-  if (65535 <= us)
-      return 0;
-  if (65535 > us)
-      return 0;
-  if (65535 >= us)
-      return 0;
-
-  if (us == 65535UL)
-      return 0;
-  if (us != 65535UL)
-      return 0;
-  if (us < 65535UL)
-      return 0;
-  if (us <= 65535UL)
-      return 0;
-  if (us > 65535UL)
-      return 0;
-  if (us >= 65535UL)
-      return 0;
-
-  if (65535UL == us)
-      return 0;
-  if (65535UL != us)
-      return 0;
-  if (65535UL < us)
-      return 0;
-  if (65535UL <= us)
-      return 0;
-  if (65535UL > us)
-      return 0;
-  if (65535UL >= us)
-      return 0;
-#endif
 
   if (us == 32767)
     return 0;
   if (us != 32767)
     return 0;
   if (us < 32767)
     return 0;
   if (us <= 32767)

test/Sema/tautological-constant-enum-compare.c

 // 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-linux-gnu -fsyntax-only -DUNSIGNED -DSILENCE -Wno-tautological-constant-compare -verify %s
-// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -DSILENCE -Wno-tautological-constant-compare -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -Wno-tautological-constant-compare -verify=silence %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -Wno-tautological-constant-compare -verify=silence %s
 
 int main() {
   enum A { A_a = 2 };
   enum A a;
 
-#ifdef SILENCE
-  // expected-no-diagnostics
-#endif
+// silence-no-diagnostics
 
 #ifdef UNSIGNED
-#ifndef SILENCE
   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;
#ifdef UNSIGNED
   if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}
     return 0;
   if (4294967295U > a)
     return 0;
   if (a >= 4294967295U)
     return 0;
   if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}
     return 0;
-#else // SILENCE
-  if (a < 0)
-    return 0;
-  if (0 >= a)
-    return 0;
-  if (a > 0)
-    return 0;
-  if (0 <= a)
-    return 0;
-  if (a <= 0)
-    return 0;
-  if (0 > a)
-    return 0;
-  if (a >= 0)
-    return 0;
-  if (0 < a)
-    return 0;
-
-  if (a < 0U)
-    return 0;
-  if (0U >= a)
-    return 0;
-  if (a > 0U)
-    return 0;
-  if (0U <= a)
-    return 0;
-  if (a <= 0U)
-    return 0;
-  if (0U > a)
-    return 0;
-  if (a >= 0U)
-    return 0;
-  if (0U < a)
-    return 0;
-
-  if (a < 4294967295)
-    return 0;
-  if (4294967295 >= a)
-    return 0;
-  if (a > 4294967295)
-    return 0;
-  if (4294967295 <= a)
-    return 0;
-  if (a <= 4294967295)
-    return 0;
-  if (4294967295 > a)
-    return 0;
-  if (a >= 4294967295)
-    return 0;
-  if (4294967295 < a)
-    return 0;
-
-  if (a < 4294967295U)
-    return 0;
-  if (4294967295U >= a)
-    return 0;
-  if (a > 4294967295U)
-    return 0;
-  if (4294967295U <= a)
-    return 0;
-  if (a <= 4294967295U)
-    return 0;
-  if (4294967295U > a)
-    return 0;
-  if (a >= 4294967295U)
-    return 0;
-  if (4294967295U < a)
-    return 0;
-#endif
 #elif defined(SIGNED)
-#ifndef SILENCE
   if (a < -2147483648) // expected-warning {{comparison 'enum A' < -2147483648 is always false}}
     return 0;
   if (-2147483648 >= a)
     return 0;
   if (a > -2147483648)
     return 0;
   if (-2147483648 <= a) // expected-warning {{comparison -2147483648 <= 'enum A' is always true}}
     return 0;
#elif defined(SIGNED)
   if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= 2147483647 is always true}}
     return 0;
   if (2147483647U > a)
     return 0;
   if (a >= 2147483647U)
     return 0;
   if (2147483647U < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}
     return 0;
-#else // SILENCE
-  if (a < -2147483648)
-    return 0;
-  if (-2147483648 >= a)
-    return 0;
-  if (a > -2147483648)
-    return 0;
-  if (-2147483648 <= a)
-    return 0;
-  if (a <= -2147483648)
-    return 0;
-  if (-2147483648 > a)
-    return 0;
-  if (a >= -2147483648)
-    return 0;
-  if (-2147483648 < a)
-    return 0;
-
-  if (a < 2147483647)
-    return 0;
-  if (2147483647 >= a)
-    return 0;
-  if (a > 2147483647)
-    return 0;
-  if (2147483647 <= a)
-    return 0;
-  if (a <= 2147483647)
-    return 0;
-  if (2147483647 > a)
-    return 0;
-  if (a >= 2147483647)
-    return 0;
-  if (2147483647 < a)
-    return 0;
-
-  if (a < 2147483647U)
-    return 0;
-  if (2147483647U >= a)
-    return 0;
-  if (a > 2147483647U)
-    return 0;
-  if (2147483647U <= a)
-    return 0;
-  if (a <= 2147483647U)
-    return 0;
-  if (2147483647U > a)
-    return 0;
-  if (a >= 2147483647U)
-    return 0;
-  if (2147483647U < a)
-    return 0;
-#endif
 #endif
 
   return 1;
 }
 
 // https://bugs.llvm.org/show_bug.cgi?id=35009
 int PR35009() {
   enum A { A_a = 2 };

test/Sema/tautological-unsigned-enum-zero-compare.c

-// 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.
 // On windows, it is signed by default. We do not want to warn in that case.
 
 int main() {
   enum A { A_a = 0 };
   enum A a;
   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 {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < a)
-    return 0;
-
-  if (b < 0)
-    return 0;
-  if (0 >= b)
-    return 0;
-  if (b > 0)
-    return 0;
-  if (0 <= b)
-    return 0;
-  if (b <= 0)
-    return 0;
-  if (0 > b)
-    return 0;
-  if (b >= 0)
-    return 0;
-  if (0 < b)
-    return 0;
-
-  if (b < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-    return 0;
-  if (0U >= b)
-    return 0;
-  if (b > 0U)
-    return 0;
-  if (0U <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-    return 0;
-  if (b <= 0U)
-    return 0;
-  if (0U > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-    return 0;
-  if (b >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < b)
-    return 0;
-#elif defined(SIGNED)
-  if (a < 0)
-    return 0;
-  if (0 >= a)
-    return 0;
-  if (a > 0)
-    return 0;
-  if (0 <= a)
-    return 0;
-  if (a <= 0)
-    return 0;
-  if (0 > a)
-    return 0;
-  if (a >= 0)
-    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 {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < a)
-    return 0;
-
-  if (b < 0)
-    return 0;
-  if (0 >= b)
-    return 0;
-  if (b > 0)
-    return 0;
-  if (0 <= b)
-    return 0;
-  if (b <= 0)
-    return 0;
-  if (0 > b)
-    return 0;
-  if (b >= 0)
-    return 0;
-  if (0 < b)
-    return 0;
-
-  if (b < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-    return 0;
-  if (0U >= b)
-    return 0;
-  if (b > 0U)
-    return 0;
-  if (0U <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-    return 0;
-  if (b <= 0U)
-    return 0;
-  if (0U > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-    return 0;
-  if (b >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < b)
-    return 0;
-#else
-  // expected-no-diagnostics
+  // silence-no-diagnostics
 
-  if (a < 0)
+  if (a < 0) // unsigned-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)
+  if (0 <= a) // unsigned-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
   if (a <= 0)
     return 0;
-  if (0 > a)
+  if (0 > a) // unsigned-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (a >= 0)
+  if (a >= 0) // unsigned-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0 < a)
     return 0;
 
-  if (a < 0U)
+  if (a < 0U) // unsigned-signed-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)
+  if (0U <= a) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
   if (a <= 0U)
     return 0;
-  if (0U > a)
+  if (0U > a) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (a >= 0U)
+  if (a >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0U < a)
     return 0;
 
   if (b < 0)
     return 0;
   if (0 >= b)
     return 0;
   if (b > 0)
     return 0;
   if (0 <= b)
     return 0;
   if (b <= 0)
     return 0;
   if (0 > b)
     return 0;
   if (b >= 0)
     return 0;
   if (0 < b)
     return 0;
 
-  if (b < 0U)
+  if (b < 0U) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
   if (0U >= b)
     return 0;
   if (b > 0U)
     return 0;
-  if (0U <= b)
+  if (0U <= b) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
   if (b <= 0U)
     return 0;
-  if (0U > b)
+  if (0U > b) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (b >= 0U)
+  if (b >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0U < b)
     return 0;
-#endif
 
   if (a == 0)
     return 0;
   if (0 != a)
     return 0;
   if (a != 0)
     return 0;
   if (0 == a)

test/Sema/tautological-unsigned-enum-zero-compare.cpp

-// 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:            -Wno-tautological-unsigned-enum-zero-compare \
+// RUN:            -verify=silence %s
+
+// silence-no-diagnostics
 
 int main() {
   // On Windows, all enumerations have a fixed underlying type, which is 'int'
   // if not otherwise specified, so A is identical to C on Windows. Otherwise,
   // we follow the C++ rules, which say that the only valid values of A are 0
   // and 1.
   enum A { A_foo = 0, A_bar, };
   enum A a;
 
   enum B : unsigned { B_foo = 0, B_bar, };
   enum B b;
 
   enum C : signed { C_foo = 0, C_bar, };
   enum C c;
 
-#ifdef UNSIGNED
-  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (a < 0) // unsigned-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}}
+  if (0 <= a) // unsigned-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}}
+  if (0 > a) // unsigned-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}}
+  if (a >= 0) // unsigned-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}}
+  // FIXME: As below, the issue here is that the enumeration is promoted to
+  // unsigned.
+  if (a < 0U) // unsigned-signed-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}}
+  if (0U <= a) // unsigned-signed-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}}
+  if (0U > a) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (a >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0U < a)
     return 0;
 
-  if (b < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (b < 0) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
   if (0 >= b)
     return 0;
   if (b > 0)
     return 0;
-  if (0 <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  if (0 <= b) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
   if (b <= 0)
     return 0;
-  if (0 > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  if (0 > b) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (b >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (b >= 0) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0 < b)
     return 0;
 
-  if (b < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (b < 0U) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
   if (0U >= b)
     return 0;
   if (b > 0U)
     return 0;
-  if (0U <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  if (0U <= b) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
   if (b <= 0U)
     return 0;
-  if (0U > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  if (0U > b) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (b >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (b >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0U < b)
     return 0;
 
   if (c < 0)
     return 0;
   if (0 >= c)
     return 0;
   if (c > 0)
     return 0;
   if (0 <= c)
     return 0;
   if (c <= 0)
     return 0;
   if (0 > c)
     return 0;
   if (c >= 0)
     return 0;
   if (0 < c)
     return 0;
 
   // FIXME: These diagnostics are terrible. The issue here is that the signed
   // enumeration value was promoted to an unsigned type.
-  if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-    return 0;
-  if (0U >= c)
-    return 0;
-  if (c > 0U)
-    return 0;
-  if (0U <= c) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-    return 0;
-  if (c <= 0U)
-    return 0;
-  if (0U > c) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-    return 0;
-  if (c >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < c)
-    return 0;
-#elif defined(SIGNED)
-  if (a < 0)
-    return 0;
-  if (0 >= a)
-    return 0;
-  if (a > 0)
-    return 0;
-  if (0 <= a)
-    return 0;
-  if (a <= 0)
-    return 0;
-  if (0 > a)
-    return 0;
-  if (a >= 0)
-    return 0;
-  if (0 < a)
-    return 0;
-
-  // FIXME: As above, the issue here is that the enumeration is promoted to
-  // unsigned.
-  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 {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < a)
-    return 0;
-
-  if (b < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-    return 0;
-  if (0 >= b)
-    return 0;
-  if (b > 0)
-    return 0;
-  if (0 <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-    return 0;
-  if (b <= 0)
-    return 0;
-  if (0 > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-    return 0;
-  if (b >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0 < b)
-    return 0;
-
-  if (b < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-    return 0;
-  if (0U >= b)
-    return 0;
-  if (b > 0U)
-    return 0;
-  if (0U <= b) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
-    return 0;
-  if (b <= 0U)
-    return 0;
-  if (0U > b) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
-    return 0;
-  if (b >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
-    return 0;
-  if (0U < b)
-    return 0;
-
-  if (c < 0)
-    return 0;
-  if (0 >= c)
-    return 0;
-  if (c > 0)
-    return 0;
-  if (0 <= c)
-    return 0;
-  if (c <= 0)
-    return 0;
-  if (0 > c)
-    return 0;
-  if (c >= 0)
-    return 0;
-  if (0 < c)
-    return 0;
-
-  if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+  if (c < 0U) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
   if (0U >= c)
     return 0;
   if (c > 0U)
     return 0;
-  if (0U <= c) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+  if (0U <= c) // unsigned-signed-warning {{comparison of 0 <= unsigned enum expression is always true}}
     return 0;
   if (c <= 0U)
     return 0;
-  if (0U > c) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+  if (0U > c) // unsigned-signed-warning {{comparison of 0 > unsigned enum expression is always false}}
     return 0;
-  if (c >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+  if (c >= 0U) // unsigned-signed-warning {{comparison of unsigned enum expression >= 0 is always true}}
     return 0;
   if (0U < c)
     return 0;
-#else
-  // expected-no-diagnostics
-  if (a < 0)
-    return 0;
-  if (0 >= a)
-    return 0;
-  if (a > 0)
-    return 0;
-  if (0 <= a)
-    return 0;
-  if (a <= 0)
-    return 0;
-  if (0 > a)
-    return 0;
-  if (a >= 0)
-    return 0;
-  if (0 < a)
-    return 0;
-
-  if (a < 0U)
-    return 0;
-  if (0U >= a)
-    return 0;
-  if (a > 0U)
-    return 0;
-  if (0U <= a)
-    return 0;
-  if (a <= 0U)
-    return 0;
-  if (0U > a)
-    return 0;
-  if (a >= 0U)
-    return 0;
-  if (0U < a)
-    return 0;
-
-  if (b < 0)
-    return 0;
-  if (0 >= b)
-    return 0;
-  if (b > 0)
-    return 0;
-  if (0 <= b)
-    return 0;
-  if (b <= 0)
-    return 0;
-  if (0 > b)
-    return 0;
-  if (b >= 0)
-    return 0;
-  if (0 < b)
-    return 0;
-
-  if (b < 0U)
-    return 0;
-  if (0U >= b)
-    return 0;
-  if (b > 0U)
-    return 0;
-  if (0U <= b)
-    return 0;
-  if (b <= 0U)
-    return 0;
-  if (0U > b)
-    return 0;
-  if (b >= 0U)
-    return 0;
-  if (0U < b)
-    return 0;
-
-  if (c < 0)
-    return 0;
-  if (0 >= c)
-    return 0;
-  if (c > 0)
-    return 0;
-  if (0 <= c)
-    return 0;
-  if (c <= 0)
-    return 0;
-  if (0 > c)
-    return 0;
-  if (c >= 0)
-    return 0;
-  if (0 < c)
-    return 0;
-
-  if (c < 0U)
-    return 0;
-  if (0U >= c)
-    return 0;
-  if (c > 0U)
-    return 0;
-  if (0U <= c)
-    return 0;
-  if (c <= 0U)
-    return 0;
-  if (0U > c)
-    return 0;
-  if (c >= 0U)
-    return 0;
-  if (0U < c)
-    return 0;
-#endif
 
   return 1;
 }
 
 namespace crash_enum_zero_width {
 int test() {
   enum A : unsigned {
     A_foo = 0
   };
   enum A a;
 
   // used to crash in llvm::APSInt::getMaxValue()
-#ifndef SILENCE
-  if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
-#else
-  if (a > 0)
-#endif
+  if (a < 0) // unsigned-signed-warning {{comparison of unsigned enum expression < 0 is always false}}
     return 0;
 
   return 1;
 }
 } // namespace crash_enum_zero_width

test/Sema/tautological-unsigned-zero-compare.c

-// 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);
 
 #define macro(val) val
 
 #ifdef __cplusplus
 template<typename T>
 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;
 
   T v = svalue();
   if (v < 0)
       return;
 }
 #endif
 
 int main()
 {
 #ifdef __cplusplus
   TFunc<unsigned char>();
   TFunc<unsigned short>();
 #endif
 
   unsigned un = uvalue();
 
-#ifdef TEST
+  // silence-no-diagnostics
+
   if (un == 0)
       return 0;
   if (un != 0)
       return 0;
   if (un < 0) // expected-warning {{comparison of unsigned expression < 0 is always false}}
       return 0;
   if (un <= 0)
       return 0;
#endif
   if (0UL < un)
       return 0;
   if (0UL <= un) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
       return 0;
   if (0UL > un) // expected-warning {{comparison of 0 > unsigned expression is always false}}
       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)
       return 0;
   if (a < 0)
       return 0;
   if (a <= 0)
       return 0;
#endif
   if (0UL < a)
       return 0;
   if (0UL <= a) // expected-warning {{comparison of 0 <= unsigned expression is always true}}
       return 0;
   if (0UL > a) // expected-warning {{comparison of 0 > unsigned expression is always false}}
       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;
 
   if (fl == 0)
       return 0;
   if (fl != 0)
       return 0;