[verify] Improve the error messages with multiple active prefixes

clang/include/clang/Basic/DiagnosticFrontendKinds.td

@@ -175,11 +175,11 @@ def err_verify_invalid_content : Error<
 def err_verify_missing_regex : Error<
     "cannot find start of regex ('{{') in %0">;
 def err_verify_inconsistent_diags : Error<
-    "'%0' diagnostics %select{expected|seen}1 but not %select{seen|expected}1: "
-    "%2">;
+    "%select{|'%1-%2' }0diagnostics %select{with '%2' severity |}0"
+    "%select{expected|seen}3 but not %select{seen|expected}3: "
+    "%4">;
 def err_verify_invalid_no_diags : Error<
-    "%select{expected|'%0-no-diagnostics'}1 directive cannot follow "
-    "%select{'%0-no-diagnostics' directive|other expected directives}1">;
+    "'%0' directive cannot follow %select{'%2' directive|other expected directives}1">;
 def err_verify_no_directives : Error<
     "no expected directives found: consider use of '%0-no-diagnostics'">;
 def err_verify_nonconst_addrspace : Error<

clang/include/clang/Frontend/VerifyDiagnosticConsumer.h

@@ -44,15 +44,17 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,
   public:
     static std::unique_ptr<Directive>
     create(bool RegexKind, SourceLocation DirectiveLoc,
-           SourceLocation DiagnosticLoc, bool MatchAnyFileAndLine,
-           bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max);
+           SourceLocation DiagnosticLoc, StringRef Spelling,
+           bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+           unsigned Min, unsigned Max);
 
   public:
     /// Constant representing n or more matches.
     static const unsigned MaxCount = std::numeric_limits<unsigned>::max();
 
     SourceLocation DirectiveLoc;
     SourceLocation DiagnosticLoc;
+    const std::string Spelling;
     const std::string Text;
     unsigned Min, Max;
     bool MatchAnyLine;
@@ -71,10 +73,11 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,
 
   protected:
     Directive(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-              bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
-              unsigned Min, unsigned Max)
-        : DirectiveLoc(DirectiveLoc), DiagnosticLoc(DiagnosticLoc), Text(Text),
-          Min(Min), Max(Max), MatchAnyLine(MatchAnyLine || MatchAnyFileAndLine),
+              StringRef Spelling, bool MatchAnyFileAndLine, bool MatchAnyLine,
+              StringRef Text, unsigned Min, unsigned Max)
+        : DirectiveLoc(DirectiveLoc), DiagnosticLoc(DiagnosticLoc),
+          Spelling(Spelling), Text(Text), Min(Min), Max(Max),
+          MatchAnyLine(MatchAnyLine || MatchAnyFileAndLine),
           MatchAnyFileAndLine(MatchAnyFileAndLine) {
       assert(!DirectiveLoc.isInvalid() && "DirectiveLoc is invalid!");
       assert((!DiagnosticLoc.isInvalid() || MatchAnyLine) &&
@@ -106,6 +109,11 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,
     HasOtherExpectedDirectives
   };
 
+  struct ParsingState {
+    DirectiveStatus Status;
+    std::string FirstNoDiagnosticsDirective;
+  };
+
   class MarkerTracker;
 
 private:
@@ -118,7 +126,7 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,
   const LangOptions *LangOpts = nullptr;
   SourceManager *SrcManager = nullptr;
   unsigned ActiveSourceFiles = 0;
-  DirectiveStatus Status;
+  ParsingState State;
   ExpectedData ED;
 
   void CheckDiagnostics();

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

@@ -89,9 +89,10 @@ namespace {
 class StandardDirective : public Directive {
 public:
   StandardDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-                    bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
-                    unsigned Min, unsigned Max)
-      : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyFileAndLine,
+                    StringRef Spelling, bool MatchAnyFileAndLine,
+                    bool MatchAnyLine, StringRef Text, unsigned Min,
+                    unsigned Max)
+      : Directive(DirectiveLoc, DiagnosticLoc, Spelling, MatchAnyFileAndLine,
                   MatchAnyLine, Text, Min, Max) {}
 
   bool isValid(std::string &Error) override {
@@ -106,9 +107,10 @@ class StandardDirective : public Directive {
 class RegexDirective : public Directive {
 public:
   RegexDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-                 bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
-                 unsigned Min, unsigned Max, StringRef RegexStr)
-      : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyFileAndLine,
+                 StringRef Spelling, bool MatchAnyFileAndLine,
+                 bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max,
+                 StringRef RegexStr)
+      : Directive(DirectiveLoc, DiagnosticLoc, Spelling, MatchAnyFileAndLine,
                   MatchAnyLine, Text, Min, Max),
         Regex(RegexStr) {}
 
@@ -285,6 +287,7 @@ class ParseHelper
 // The information necessary to create a directive.
 struct UnattachedDirective {
   DirectiveList *DL = nullptr;
+  std::string Spelling;
   bool RegexKind = false;
   SourceLocation DirectivePos, ContentBegin;
   std::string Text;
@@ -299,8 +302,8 @@ void attachDirective(DiagnosticsEngine &Diags, const UnattachedDirective &UD,
                      bool MatchAnyLine = false) {
   // Construct new directive.
   std::unique_ptr<Directive> D = Directive::create(
-      UD.RegexKind, UD.DirectivePos, ExpectedLoc, MatchAnyFileAndLine,
-      MatchAnyLine, UD.Text, UD.Min, UD.Max);
+      UD.RegexKind, UD.DirectivePos, ExpectedLoc, UD.Spelling,
+      MatchAnyFileAndLine, MatchAnyLine, UD.Text, UD.Min, UD.Max);
 
   std::string Error;
   if (!D->isValid(Error)) {
@@ -408,7 +411,7 @@ static std::string DetailedErrorString(const DiagnosticsEngine &Diags) {
 /// Returns true if any valid directives were found.
 static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
                            Preprocessor *PP, SourceLocation Pos,
-                           VerifyDiagnosticConsumer::DirectiveStatus &Status,
+                           VerifyDiagnosticConsumer::ParsingState &State,
                            VerifyDiagnosticConsumer::MarkerTracker &Markers) {
   DiagnosticsEngine &Diags = PP ? PP->getDiagnostics() : SM.getDiagnostics();
 
@@ -440,8 +443,9 @@ static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
     StringRef DToken = PH.Match();
     PH.Advance();
 
-    // Default directive kind.
     UnattachedDirective D;
+    D.Spelling = DToken;
+    // Default directive kind.
     const char *KindStr = "string";
 
     // Parse the initial directive token in reverse so we can easily determine
@@ -482,19 +486,24 @@ static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM,
       continue;
 
     if (NoDiag) {
-      if (Status == VerifyDiagnosticConsumer::HasOtherExpectedDirectives)
+      if (State.Status ==
+          VerifyDiagnosticConsumer::HasOtherExpectedDirectives) {
         Diags.Report(Pos, diag::err_verify_invalid_no_diags)
-            << DetailedErrorString(Diags) << /*IsExpectedNoDiagnostics=*/true;
-      else
-        Status = VerifyDiagnosticConsumer::HasExpectedNoDiagnostics;
+            << D.Spelling << /*IsExpectedNoDiagnostics=*/true;
+      } else if (State.Status !=
+                 VerifyDiagnosticConsumer::HasExpectedNoDiagnostics) {
+        State.Status = VerifyDiagnosticConsumer::HasExpectedNoDiagnostics;
+        State.FirstNoDiagnosticsDirective = D.Spelling;
+      }
       continue;
     }
-    if (Status == VerifyDiagnosticConsumer::HasExpectedNoDiagnostics) {
+    if (State.Status == VerifyDiagnosticConsumer::HasExpectedNoDiagnostics) {
       Diags.Report(Pos, diag::err_verify_invalid_no_diags)
-          << DetailedErrorString(Diags) << /*IsExpectedNoDiagnostics=*/false;
+          << D.Spelling << /*IsExpectedNoDiagnostics=*/false
+          << State.FirstNoDiagnosticsDirective;
       continue;
     }
-    Status = VerifyDiagnosticConsumer::HasOtherExpectedDirectives;
+    State.Status = VerifyDiagnosticConsumer::HasOtherExpectedDirectives;
 
     // If a directive has been found but we're not interested
     // in storing the directive information, return now.
@@ -670,7 +679,7 @@ VerifyDiagnosticConsumer::VerifyDiagnosticConsumer(DiagnosticsEngine &Diags_)
     : Diags(Diags_), PrimaryClient(Diags.getClient()),
       PrimaryClientOwner(Diags.takeClient()),
       Buffer(new TextDiagnosticBuffer()), Markers(new MarkerTracker(Diags)),
-      Status(HasNoDirectives) {
+      State{HasNoDirectives, {}} {
   if (Diags.hasSourceManager())
     setSourceManager(Diags.getSourceManager());
 }
@@ -788,7 +797,7 @@ bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP,
   // Fold any "\<EOL>" sequences
   size_t loc = C.find('\\');
   if (loc == StringRef::npos) {
-    ParseDirective(C, &ED, SM, &PP, CommentBegin, Status, *Markers);
+    ParseDirective(C, &ED, SM, &PP, CommentBegin, State, *Markers);
     return false;
   }
 
@@ -818,7 +827,7 @@ bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP,
   }
 
   if (!C2.empty())
-    ParseDirective(C2, &ED, SM, &PP, CommentBegin, Status, *Markers);
+    ParseDirective(C2, &ED, SM, &PP, CommentBegin, State, *Markers);
   return false;
 }
 
@@ -843,8 +852,8 @@ static bool findDirectives(SourceManager &SM, FileID FID,
 
   Token Tok;
   Tok.setKind(tok::comment);
-  VerifyDiagnosticConsumer::DirectiveStatus Status =
-    VerifyDiagnosticConsumer::HasNoDirectives;
+  VerifyDiagnosticConsumer::ParsingState State = {
+      VerifyDiagnosticConsumer::HasNoDirectives, {}};
   while (Tok.isNot(tok::eof)) {
     RawLex.LexFromRawLexer(Tok);
     if (!Tok.is(tok::comment)) continue;
@@ -856,8 +865,8 @@ static bool findDirectives(SourceManager &SM, FileID FID,
     VerifyDiagnosticConsumer::MarkerTracker Markers(SM.getDiagnostics());
 
     // Find first directive.
-    if (ParseDirective(Comment, nullptr, SM, nullptr, Tok.getLocation(),
-                       Status, Markers))
+    if (ParseDirective(Comment, nullptr, SM, nullptr, Tok.getLocation(), State,
+                       Markers))
       return true;
   }
   return false;
@@ -887,10 +896,11 @@ static unsigned PrintUnexpected(DiagnosticsEngine &Diags, SourceManager *SourceM
     OS << ": " << I->second;
   }
 
+  const bool IsSinglePrefix =
+      Diags.getDiagnosticOptions().VerifyPrefixes.size() == 1;
   std::string Prefix = *Diags.getDiagnosticOptions().VerifyPrefixes.begin();
-  std::string KindStr = Prefix + "-" + Kind;
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()
-      << KindStr << /*Unexpected=*/true << OS.str();
+      << IsSinglePrefix << Prefix << Kind << /*Unexpected=*/true << OS.str();
   return std::distance(diag_begin, diag_end);
 }
 
@@ -902,6 +912,9 @@ static unsigned PrintExpected(DiagnosticsEngine &Diags,
   if (DL.empty())
     return 0;
 
+  const bool IsSinglePrefix =
+      Diags.getDiagnosticOptions().VerifyPrefixes.size() == 1;
+
   SmallString<256> Fmt;
   llvm::raw_svector_ostream OS(Fmt);
   for (const auto *D : DL) {
@@ -917,13 +930,14 @@ static unsigned PrintExpected(DiagnosticsEngine &Diags,
       OS << " (directive at "
          << SourceMgr.getFilename(D->DirectiveLoc) << ':'
          << SourceMgr.getPresumedLineNumber(D->DirectiveLoc) << ')';
+    if (!IsSinglePrefix)
+      OS << " \'" << D->Spelling << '\'';
     OS << ": " << D->Text;
   }
 
   std::string Prefix = *Diags.getDiagnosticOptions().VerifyPrefixes.begin();
-  std::string KindStr = Prefix + "-" + Kind;
   Diags.Report(diag::err_verify_inconsistent_diags).setForceEmit()
-      << KindStr << /*Unexpected=*/false << OS.str();
+      << IsSinglePrefix << Prefix << Kind << /*Unexpected=*/false << OS.str();
   return DL.size();
 }
 
@@ -1109,11 +1123,11 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
   if (SrcManager) {
     // Produce an error if no expected-* directives could be found in the
     // source file(s) processed.
-    if (Status == HasNoDirectives) {
+    if (State.Status == HasNoDirectives) {
       Diags.Report(diag::err_verify_no_directives).setForceEmit()
           << DetailedErrorString(Diags);
       ++NumErrors;
-      Status = HasNoDirectivesReported;
+      State.Status = HasNoDirectivesReported;
     }
 
     // Check that the expected diagnostics occurred.
@@ -1142,15 +1156,14 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
   ED.Reset();
 }
 
-std::unique_ptr<Directive> Directive::create(bool RegexKind,
-                                             SourceLocation DirectiveLoc,
-                                             SourceLocation DiagnosticLoc,
-                                             bool MatchAnyFileAndLine,
-                                             bool MatchAnyLine, StringRef Text,
-                                             unsigned Min, unsigned Max) {
+std::unique_ptr<Directive>
+Directive::create(bool RegexKind, SourceLocation DirectiveLoc,
+                  SourceLocation DiagnosticLoc, StringRef Spelling,
+                  bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+                  unsigned Min, unsigned Max) {
   if (!RegexKind)
     return std::make_unique<StandardDirective>(DirectiveLoc, DiagnosticLoc,
-                                               MatchAnyFileAndLine,
+                                               Spelling, MatchAnyFileAndLine,
                                                MatchAnyLine, Text, Min, Max);
 
   // Parse the directive into a regular expression.
@@ -1175,7 +1188,7 @@ std::unique_ptr<Directive> Directive::create(bool RegexKind,
     }
   }
 
-  return std::make_unique<RegexDirective>(DirectiveLoc, DiagnosticLoc,
+  return std::make_unique<RegexDirective>(DirectiveLoc, DiagnosticLoc, Spelling,
                                           MatchAnyFileAndLine, MatchAnyLine,
                                           Text, Min, Max, RegexStr);
 }

clang/test/Frontend/verify-mulptiple-prefixes.c

@@ -0,0 +1,48 @@
+// Test the diagnostic messages of -verify with multiple prefixes.
+// - Expected but not seen errors should contain the prefix of the directive
+// - Seen but not expected errors should not choose an arbitrary prefix
+// - "expected directive cannot follow '<prefix>-no-diagnostics'" should report an actual
+//    expected-no-diagnostics prefix present in the source.
+
+// RUN: not %clang_cc1 -verify=foo,bar %s 2>&1 | FileCheck %s --check-prefix=CHECK1
+// RUN: not %clang_cc1 -verify=bar,foo %s 2>&1 | FileCheck %s --check-prefix=CHECK1
+
+undefined_type x; // #1
+
+// foo-error{{there is no error here}}
+// bar-error{{error not seen}}
+// bar-note{{declared here}}
+// bar-error{{another error not seen}}
+// bar-error-re{{regex error{{}} not present}}
+
+// CHECK1: error: diagnostics with 'error' severity expected but not seen: 
+// CHECK1:   Line 12 'foo-error': there is no error here
+// CHECK1:   Line 13 'bar-error': error not seen
+// CHECK1:   Line 15 'bar-error': another error not seen
+// CHECK1:   Line 16 'bar-error-re': regex error{{{{[}][}]}} not present
+// CHECK1: error: diagnostics with 'error' severity seen but not expected: 
+// CHECK1:   Line 10: unknown type name 'undefined_type'
+// CHECK1: error: diagnostics with 'note' severity expected but not seen: 
+// CHECK1:   Line 14 'bar-note': declared here
+// CHECK1: 6 errors generated.
+
+// RUN: not %clang_cc1 -verify=baz,qux,quux %s 2>&1 | FileCheck %s --check-prefix=CHECK2
+
+// qux-no-diagnostics
+// baz-error@#1{{unknown type name 'undefined_type'}}
+// quux-no-diagnostics
+// qux-error-re@#1{{unknown type name 'undefined_type'}}
+
+// CHECK2: error: diagnostics with 'error' severity seen but not expected: 
+// CHECK2:   Line 10: unknown type name 'undefined_type'
+// CHECK2:   Line 32: 'baz-error' directive cannot follow 'qux-no-diagnostics' directive
+// CHECK2:   Line 34: 'qux-error-re' directive cannot follow 'qux-no-diagnostics' directive
+
+// RUN: not %clang_cc1 -verify=spam,eggs %s 2>&1 | FileCheck %s --check-prefix=CHECK3
+
+// eggs-error@#1{{unknown type name 'undefined_type'}}
+// spam-no-diagnostics
+
+// CHECK3: error: diagnostics with 'error' severity seen but not expected: 
+// CHECK3:   Line 44: 'spam-no-diagnostics' directive cannot follow other expected directives
+// CHECK3: 1 error generated.

clang/test/Frontend/verify.c

@@ -201,7 +201,7 @@ unexpected b; // expected-error@33 1-1 {{unknown type}}
 // foo-note {{}}
 
 //      CHECK11: error: 'foo-error' diagnostics seen but not expected:
-// CHECK11-NEXT:   Line 201: expected directive cannot follow 'foo-no-diagnostics' directive
+// CHECK11-NEXT:   Line 201: 'foo-note' directive cannot follow 'foo-no-diagnostics' directive
 // CHECK11-NEXT: 1 error generated.
 #endif
 

clang/test/Frontend/verify3.c

@@ -8,7 +8,7 @@
 // expected-note {{}}
 
 //      CHECK1: error: 'expected-error' diagnostics seen but not expected:
-// CHECK1-NEXT:   Line 8: expected directive cannot follow 'expected-no-diagnostics' directive
+// CHECK1-NEXT:   Line 8: 'expected-note' directive cannot follow 'expected-no-diagnostics' directive
 // CHECK1-NEXT: 1 error generated.
 #endif