Allow matching "any file" in `VerifyDiagnosticConsumer`.

[arames]

Summary

This allows specifying * for the filename to match any file. For example:

// expected-note@*:* {{Match this note in any file at any line.}}

[arames]

This is for example useful to add a catch-all clause like // expected-note-re@*:* 1+ {{candidate function {{.+}}}}

[jkorous]

clang/test/Frontend/verify-any-file.c

2

I feel that we should test the output with FileCheck to make it more robust.
Seems like other tests for -verify do that. For example here:
https://github.com/llvm/llvm-project/blob/master/clang/test/Frontend/verify-fatal.c

7

Can we add some scenarios for expected diagnostics not produced?

[arames]

clang/test/Frontend/verify-any-file.c

5

While testing, I realized this cannot be handled reliably, so will require the line number to be * when the filename is *.

I was using the current file and specified line for the SourceLocation in this case. But this fails if the specified line number is greater than the number of lines in the current file.

[arames]

Address review comments.

[jkorous]

We should either simplify the implementation to reflect that we don't support e. g. *:42 (seems preferable to me) or have the codepaths that are currently not accessible through -fverify tested by other means.

clang/include/clang/Frontend/VerifyDiagnosticConsumer.h

207

Should we rename it to MatchAnyFileAndLine?

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

300

Should we make it clear from the interface that MatchAnyFile => MatchAnyLine?

[arames]

In D72100#1855483, @jkorous wrote:

We should either simplify the implementation to reflect that we don't support e. g. *:42 (seems preferable to me) or have the codepaths that are currently not accessible through -fverify tested by other means.

That makes sense. I have updated the naming and added a comment to reflect that. PTAL.
As a note and future reference, the issue was not on the side of the verification; I tested combinations of MatchAnyFile and MatchAnyLine, but the fact that we cannot always create a source location with the appropriate line number when we do not know what file we are dealing with.

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

300

The naming should now make it clear. Additionally, I have added a comment for the implication in Directive.

[arames]

Rename and clarify.

[arames]

Apply clang-format.

[arames]

Rebase on top of tree.

[jkorous]

LGTM! Thanks for the work!

clang/include/clang/Frontend/VerifyDiagnosticConsumer.h

 ///
 class VerifyDiagnosticConsumer: public DiagnosticConsumer,
                                 public CommentHandler {
 public:
   /// Directive - Abstract class representing a parsed verify directive.
   ///
   class Directive {
   public:
-    static std::unique_ptr<Directive> create(bool RegexKind,
-                                             SourceLocation DirectiveLoc,
-                                             SourceLocation DiagnosticLoc,
-                                             bool MatchAnyLine, StringRef Text,
-                                             unsigned Min, unsigned Max);
+    static std::unique_ptr<Directive>
+    create(bool RegexKind, SourceLocation DirectiveLoc,
+           SourceLocation DiagnosticLoc, 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 Text;
     unsigned Min, Max;
     bool MatchAnyLine;
+    bool MatchAnyFileAndLine; // `MatchAnyFileAndLine` implies `MatchAnyLine`.
 
     Directive(const Directive &) = delete;
     Directive &operator=(const Directive &) = delete;
     virtual ~Directive() = default;
 
     // Returns true if directive text is valid.
     // Otherwise returns false and populates E.
     virtual bool isValid(std::string &Error) = 0;
 
     // Returns true on match.
     virtual bool match(StringRef S) = 0;
 
   protected:
     Directive(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-              bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max)
-        : DirectiveLoc(DirectiveLoc), DiagnosticLoc(DiagnosticLoc),
-          Text(Text), Min(Min), Max(Max), MatchAnyLine(MatchAnyLine) {
+              bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+              unsigned Min, unsigned Max)
+        : DirectiveLoc(DirectiveLoc), DiagnosticLoc(DiagnosticLoc), Text(Text),
+          Min(Min), Max(Max), MatchAnyLine(MatchAnyLine || MatchAnyFileAndLine),
+          MatchAnyFileAndLine(MatchAnyFileAndLine) {
       assert(!DirectiveLoc.isInvalid() && "DirectiveLoc is invalid!");
       assert((!DiagnosticLoc.isInvalid() || MatchAnyLine) &&
              "DiagnosticLoc is invalid!");
     }
   };
 
   using DirectiveList = std::vector<std::unique_ptr<Directive>>;
 

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

 using const_diag_iterator = TextDiagnosticBuffer::const_iterator;
 
 namespace {
 
 /// StandardDirective - Directive with string matching.
 class StandardDirective : public Directive {
 public:
   StandardDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-                    bool MatchAnyLine, StringRef Text, unsigned Min,
-                    unsigned Max)
-      : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyLine, Text, Min, Max) {}
+                    bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+                    unsigned Min, unsigned Max)
+      : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyFileAndLine,
+                  MatchAnyLine, Text, Min, Max) {}
 
   bool isValid(std::string &Error) override {
     // all strings are considered valid; even empty ones
     return true;
   }
 
   bool match(StringRef S) override {
     return S.find(Text) != StringRef::npos;
   }
 };
 
 /// RegexDirective - Directive with regular-expression matching.
 class RegexDirective : public Directive {
 public:
   RegexDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-                 bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max,
-                 StringRef RegexStr)
-      : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyLine, Text, Min, Max),
+                 bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+                 unsigned Min, unsigned Max, StringRef RegexStr)
+      : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyFileAndLine,
+                  MatchAnyLine, Text, Min, Max),
         Regex(RegexStr) {}
 
   bool isValid(std::string &Error) override {
     return Regex.isValid(Error);
   }
 
   bool match(StringRef S) override {
     return Regex.match(S);
struct UnattachedDirective {
   SourceLocation DirectivePos, ContentBegin;
   std::string Text;
   unsigned Min = 1, Max = 1;
 };
 
 // Attach the specified directive to the line of code indicated by
 // \p ExpectedLoc.
 void attachDirective(DiagnosticsEngine &Diags, const UnattachedDirective &UD,
-                     SourceLocation ExpectedLoc, bool MatchAnyLine = false) {
+                     SourceLocation ExpectedLoc,
+                     bool MatchAnyFileAndLine = false,
+                     bool MatchAnyLine = false) {
   // Construct new directive.
-  std::unique_ptr<Directive> D =
-      Directive::create(UD.RegexKind, UD.DirectivePos, ExpectedLoc,
-                        MatchAnyLine, UD.Text, UD.Min, UD.Max);
+  std::unique_ptr<Directive> D = Directive::create(
+      UD.RegexKind, UD.DirectivePos, ExpectedLoc, MatchAnyFileAndLine,
 
   std::string Error;
   if (!D->isValid(Error)) {
     Diags.Report(UD.ContentBegin, diag::err_verify_invalid_content)
       << (UD.RegexKind ? "regex" : "string") << Error;
   }
 
   UD.DL->push_back(std::move(D));
for (ParseHelper PH(S); !PH.Done();) {
     // If a directive has been found but we're not interested
     // in storing the directive information, return now.
     if (!D.DL)
       return true;
 
     // Next optional token: @
     SourceLocation ExpectedLoc;
     StringRef Marker;
+    bool MatchAnyFileAndLine = false;
     bool MatchAnyLine = false;
     if (!PH.Next("@")) {
       ExpectedLoc = Pos;
     } else {
       PH.Advance();
       unsigned Line = 0;
       bool FoundPlus = PH.Next("+");
       if (FoundPlus || PH.Next("-")) {
if (!PH.Next("@")) {
           ExpectedLoc = SM.translateLineCol(SM.getFileID(Pos), Line, 1);
       } else if (PH.NextMarker()) {
         Marker = PH.Match();
       } else if (PP && PH.Search(":")) {
         // Specific source file.
         StringRef Filename(PH.C, PH.P-PH.C);
         PH.Advance();
 
+        if (Filename == "*") {
+          MatchAnyFileAndLine = true;
+          if (!PH.Next("*")) {
+            Diags.Report(Pos.getLocWithOffset(PH.C - PH.Begin),
+                         diag::err_verify_missing_line)
+                << "'*'";
+            continue;
+          }
+          MatchAnyLine = true;
+          ExpectedLoc = SourceLocation();
+        } else {
           // Lookup file via Preprocessor, like a #include.
           const DirectoryLookup *CurDir;
           Optional<FileEntryRef> File =
               PP->LookupFile(Pos, Filename, false, nullptr, nullptr, CurDir,
                              nullptr, nullptr, nullptr, nullptr, nullptr);
           if (!File) {
-          Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin),
-                       diag::err_verify_missing_file) << Filename << KindStr;
+            Diags.Report(Pos.getLocWithOffset(PH.C - PH.Begin),
+                         diag::err_verify_missing_file)
+                << Filename << KindStr;
             continue;
           }
 
           const FileEntry *FE = &File->getFileEntry();
           if (SM.translateFile(FE).isInvalid())
             SM.createFileID(FE, Pos, SrcMgr::C_User);
 
           if (PH.Next(Line) && Line > 0)
             ExpectedLoc = SM.translateFileLineCol(FE, Line, 1);
           else if (PH.Next("*")) {
             MatchAnyLine = true;
             ExpectedLoc = SM.translateFileLineCol(FE, 1, 1);
           }
+        }
       } else if (PH.Next("*")) {
         MatchAnyLine = true;
         ExpectedLoc = SourceLocation();
       }
 
       if (ExpectedLoc.isInvalid() && !MatchAnyLine && Marker.empty()) {
         Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin),
                      diag::err_verify_missing_line) << KindStr;
for (ParseHelper PH(S); !PH.Done();) {
 
     // Check that regex directives contain at least one regex.
     if (D.RegexKind && D.Text.find("{{") == StringRef::npos) {
       Diags.Report(D.ContentBegin, diag::err_verify_missing_regex) << D.Text;
       return false;
     }
 
     if (Marker.empty())
-      attachDirective(Diags, D, ExpectedLoc, MatchAnyLine);
+      attachDirective(Diags, D, ExpectedLoc, MatchAnyFileAndLine, MatchAnyLine);
     else
       Markers.addDirective(Marker, D);
     FoundDirective = true;
   }
 
   return FoundDirective;
 }
 
static unsigned PrintExpected(DiagnosticsEngine &Diags,
                               SourceManager &SourceMgr,
                               std::vector<Directive *> &DL, const char *Kind) {
   if (DL.empty())
     return 0;
 
   SmallString<256> Fmt;
   llvm::raw_svector_ostream OS(Fmt);
   for (const auto *D : DL) {
-    if (D->DiagnosticLoc.isInvalid())
+    if (D->DiagnosticLoc.isInvalid() || D->MatchAnyFileAndLine)
       OS << "\n  File *";
     else
       OS << "\n  File " << SourceMgr.getFilename(D->DiagnosticLoc);
     if (D->MatchAnyLine)
       OS << " Line *";
     else
       OS << " Line " << SourceMgr.getPresumedLineNumber(D->DiagnosticLoc);
     if (D->DirectiveLoc != D->DiagnosticLoc)
for (unsigned i = 0; i < D.Max; ++i) {
       DiagList::iterator II, IE;
       for (II = Right.begin(), IE = Right.end(); II != IE; ++II) {
         if (!D.MatchAnyLine) {
           unsigned LineNo2 = SourceMgr.getPresumedLineNumber(II->first);
           if (LineNo1 != LineNo2)
             continue;
         }
 
-        if (!D.DiagnosticLoc.isInvalid() &&
+        if (!D.DiagnosticLoc.isInvalid() && !D.MatchAnyFileAndLine &&
             !IsFromSameFile(SourceMgr, D.DiagnosticLoc, II->first))
           continue;
 
         const std::string &RightText = II->second;
         if (D.match(RightText))
           break;
       }
       if (II == IE) {
#endif // !NDEBUG
   // Reset the buffer, we have processed all the diagnostics in it.
   Buffer.reset(new TextDiagnosticBuffer());
   ED.Reset();
 }
 
 std::unique_ptr<Directive> Directive::create(bool RegexKind,
                                              SourceLocation DirectiveLoc,
                                              SourceLocation DiagnosticLoc,
+                                             bool MatchAnyFileAndLine,
                                              bool MatchAnyLine, StringRef Text,
                                              unsigned Min, unsigned Max) {
   if (!RegexKind)
     return std::make_unique<StandardDirective>(DirectiveLoc, DiagnosticLoc,
-                                                MatchAnyLine, Text, Min, Max);
+                                               MatchAnyFileAndLine,
 
   // Parse the directive into a regular expression.
   std::string RegexStr;
   StringRef S = Text;
   while (!S.empty()) {
     if (S.startswith("{{")) {
       S = S.drop_front(2);
       size_t RegexMatchLength = S.find("}}");
       assert(RegexMatchLength != StringRef::npos);
       // Append the regex, enclosed in parentheses.
       RegexStr += "(";
       RegexStr.append(S.data(), RegexMatchLength);
       RegexStr += ")";
       S = S.drop_front(RegexMatchLength + 2);
     } else {
       size_t VerbatimMatchLength = S.find("{{");
       if (VerbatimMatchLength == StringRef::npos)
         VerbatimMatchLength = S.size();
       // Escape and append the fixed string.
       RegexStr += llvm::Regex::escape(S.substr(0, VerbatimMatchLength));
       S = S.drop_front(VerbatimMatchLength);
     }
   }
 
-  return std::make_unique<RegexDirective>(
-      DirectiveLoc, DiagnosticLoc, MatchAnyLine, Text, Min, Max, RegexStr);
+  return std::make_unique<RegexDirective>(DirectiveLoc, DiagnosticLoc,
+                                          MatchAnyFileAndLine, MatchAnyLine,
+                                          Text, Min, Max, RegexStr);
 }

clang/test/Frontend/verify-any-file.h

+unexpected var;

clang/test/Frontend/verify-any-file.c

+// RUN: not %clang_cc1 -verify %s 2>&1 | FileCheck %s
+
+#include "verify-any-file.h"
+// expected-error@*:* {{unknown type name 'unexpected'}}
+
+// expected-error@*:* {{missing error}}
+
+// expected-error@*:123 {{invalid line : "*" required}}
+//
+//      CHECK: error: 'error' diagnostics expected but not seen:
+// CHECK-NEXT:   File * Line * (directive at {{.*}}verify-any-file.c:6): missing error
+// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT:   File {{.*}}verify-any-file.c Line 8: missing or invalid line number following '@' in expected '*'
+// CHECK-NEXT: 2 errors generated.