[FileCheck] Annotate input dump (6/7)

[jdenny]

Summary

This patch implements input annotations for diagnostics reporting
CHECK-DAG discarded matches. These diagnostics are enabled by -vv.
These annotations mark discarded match ranges using !~~ because they
are bad matches even though they are not errors.

CHECK-DAG discarded matches create another case where there can be
multiple match results for the same directive.

For example:

$ FileCheck -dump-input=help
The following description was requested by -dump-input=help to
explain the input annotations printed by -dump-input=always and
-dump-input=fail:

  - L:     labels line number L of the input file
  - T:L    labels the only match result for a pattern of type T from line L of
           the check file
  - T:L'N  labels the Nth match result for a pattern of type T from line L of
           the check file
  - ^~~    marks good match (reported if -v)
  - !~~    marks bad match, such as:
           - CHECK-NEXT on same line as previous match (error)
           - CHECK-NOT found (error)
           - CHECK-DAG overlapping match (discarded, reported if -vv)
  - X~~    marks search range when no match is found, such as:
           - CHECK-NEXT not found (error)
           - CHECK-DAG not found after discarded matches (error)
  - ?      marks fuzzy match when no match is found
  - colors success, error, fuzzy match, discarded match, unmatched input

If you are not seeing color above or in input dumps, try: -color

$ FileCheck -vv -dump-input=always check4 < input4 |& sed -n '/^<<<</,$p'
<<<<<<
         1: abcdef
dag:1       ^~~~
dag:2'0       !~~~ discard: overlaps earlier match
         2: cdefgh
dag:2'1     ^~~~
check:3         X~ error: no match found
>>>>>>

$ cat check4
CHECK-DAG: abcd
CHECK-DAG: cdef
CHECK: efgh

$ cat input4
abcdef
cdefgh

This shows that the line 3 CHECK fails to match even though its
pattern appears in the input because its search range starts after the
line 2 CHECK-DAG's match range. The trouble might be that the line 2
CHECK-DAG's match range is later than expected because its first match
range overlaps with the line 1 CHECK-DAG match range and thus is
discarded.

Because !~~ for CHECK-DAG does not indicate an error, it is not
colored red. Instead, when colors are enabled, it is colored cyan,
which suggests a match that went cold.

[jdenny]

Rebased onto updated patches earlier in series.

[jdenny]

Update for changes earlier in patch series.

[probinson]

You're using !~~~ to flag: (a) CHECK-NOT that has a match, (b) CHECK-NEXT that isn't in the right place, (c) a discarded DAG match.
The first two are errors (which would be reported regardless of annotations) but the third is not. So, I'm not sure why it should have the same kind of annotation.

[jdenny]

In D53898#1317412, @probinson wrote:

You're using !~~~ to flag: (a) CHECK-NOT that has a match, (b) CHECK-NEXT that isn't in the right place, (c) a discarded DAG match.

Right. ^~~ means a match that is good, !~~ means a match that is bad, and X~~ means no match found.

A bad match or a lack of a match is not always an error. A bad match might be discarded, in the case of CHECK-DAG. A lack of a match is desirable in the case of CHECK-NOT.

The first two are errors (which would be reported regardless of annotations) but the third is not.

Colors (when enabled) indicate what happens to the match: green for good, cyan for discarded, and red for error.

If you don't have colors, you can currently tell from the directive name whether !~~ or X~~ is an error. I can't promise that would hold true as FileCheck evolves new features, so color might become more important then.

So, I'm not sure why it should have the same kind of annotation.

I'm open to suggestions for a better scheme.

One possibility is to merge all ^~~ and !~~ cases into just ^~~, so then ^~~ means any match found (whether good or bad) and X~~ means no match found. But that seems less helpful, especially when there's no color.

Another possibility is to split the discarded CHECK-DAG case from !~~ so that !~~ just means a bad match that produces an error. What should the new marker for a discarded match be?

[jdenny]

Previous feedback suggested that the description of annotations shouldn't have two lists of markers. Seeing your confusion over !~~, I'm thinking maybe that suggestion was right. It just confuses things. What if we drop the detailed description and just have:

- L      labels line number L of the input file
- T:L    labels the only match result for a pattern of type T from line L of the check file
- T:L'N  labels the Nth match result for a pattern of type T from line L of the check file
- ^~~    marks good match (requires -v)
- !~~    marks bad match (discarded if CHECK-DAG, or error otherwise)
- X~~    marks search range when no match is found (success if CHECK-NOT, or error otherwise)
- ?      marks fuzzy match when no match is found

[jdenny]

Propagate changes started earlier in patch series.

[jdenny]

Continue changes started earlier in patch series.

[jdenny]

Adjust comment, as suggested by probinson.

[probinson]

LGTM

llvm/test/FileCheck/dump-input-annotations.txt

318

I guess VQ means "-v but not -vv" here?

[jdenny]

llvm/test/FileCheck/dump-input-annotations.txt

318

Yep, I meant Q for quiet. I'm happy to rename or add comments to explain if you think that would be helpful.

[probinson]

llvm/test/FileCheck/dump-input-annotations.txt

318

You have a mix of 5 directive names in the same sequence of checks, I think a bit of commentary would do no harm, :-)

[jdenny]

llvm/test/FileCheck/dump-input-annotations.txt

318

Done in D55738.

llvm/include/llvm/Support/FileCheck.h

 struct FileCheckDiag {
   /// What is the FileCheck directive for this diagnostic?
   Check::FileCheckType CheckTy;
   /// Where is the FileCheck directive for this diagnostic?
   unsigned CheckLine, CheckCol;
   /// What kind of match result does this diagnostic describe?
   ///
   /// There might be more than one of these for the same directive.  For
-  /// example, there might be a fuzzy match after a fail.
+  /// example, there might be several discards before either a final or fail,
+  /// and there might be a fuzzy match after a fail.
   ///
   /// We iterate these types, so they must have contiguous values in
   /// [0, MatchTypeCount).  Moreover, keep match types together if they use the
   /// same mark in annotated input dumps or else printing of the annotation key
   /// will malfunction.
   enum MatchType {
     // TODO: More members will appear with later patches in this series.
     MatchFinalAndExpected, //< the final match for an expected pattern
     MatchTypeFirst = MatchFinalAndExpected,
     MatchFinalButExcluded, //< the final match for an excluded pattern
     MatchFinalButIllegal,  //< the final but illegal match for an expected pattern
+    MatchDiscard,          //< a discarded match for an expected pattern
     MatchNoneButExpected,  //< no match for an expected pattern
     MatchFuzzy,            //< a fuzzy match (because no perfect match)
     MatchTypeCount,
   } MatchTy;
   /// The match range if MatchTy is not MatchNoneButExpected, or the search
   /// range otherwise.
   unsigned InputStartLine, InputStartCol, InputEndLine, InputEndCol;
   FileCheckDiag(const SourceMgr &SM, const Check::FileCheckType &CheckTy,

llvm/lib/Support/FileCheck.cpp

for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
       if (Req.VerboseVerbose) {
         SMLoc OldStart = SMLoc::getFromPointer(Buffer.data() + MI->Pos);
         SMLoc OldEnd = SMLoc::getFromPointer(Buffer.data() + MI->End);
         SMRange OldRange(OldStart, OldEnd);
         SM.PrintMessage(OldStart, SourceMgr::DK_Note,
                         "match discarded, overlaps earlier DAG match here",
                         {OldRange});
         if (Diags)
-          Diags->pop_back();
+          Diags->rbegin()->MatchTy = FileCheckDiag::MatchDiscard;
       }
       MatchPos = MI->End;
     }
     if (!Req.VerboseVerbose)
       PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, VariableTable,
                  MatchPos, MatchLen, Req, Diags);
 
     // Handle the end of a CHECK-DAG group.

llvm/test/FileCheck/dump-input-annotations.txt

 ; NOT2-NEXT:             2: world
 ; NOT2-NEXT:    not:2       !~~~~
 ; NOT2-NEXT:             3: again
 ; NOT2-V-NEXT:  check:3       ^~~
 ; NOT2-NEXT:    >>>>>>
 ; NOT2-NOT:     {{.}}
 
 ;--------------------------------------------------
-; CHECK-DAG (also: matches in different order than directives)
+; CHECK-DAG (also: matches in different order than directives, discarded match)
 ;--------------------------------------------------
 
 ; Good match, discarded match plus good match, and no match.
 
 ; RUN: echo 'abc' > %t.in
 ; RUN: echo 'def' >> %t.in
 ; RUN: echo 'abc' >> %t.in
 
 ; RUN: echo 'CHECK-DAG: def' > %t.chk
 ; RUN: echo 'CHECK-DAG: abc' >> %t.chk
 ; RUN: echo 'CHECK-DAG: abc' >> %t.chk
 ; RUN: echo 'CHECK-DAG: def' >> %t.chk
 
 ; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
 ; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG,DAG-Q
 ; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
-; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG,DAG-V
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG,DAG-V,DAG-VQ
 ; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
 ; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG,DAG-V,DAG-VV
 
 ; DAG:         <<<<<<
 ; DAG-NEXT:             1: abc
 ; DAG-V-NEXT:  dag:2       ^~~
+; DAG-VV-NEXT: dag:3'0     !~~
 ; DAG-NEXT:             2: def
 ; DAG-V-NEXT:  dag:1       ^~~
+; DAG-VV-NEXT: dag:4'0     !~~
 ; DAG-NEXT:             3: abc
-; DAG-V-NEXT:  dag:3       ^~~
-; DAG-NEXT:    dag:4       X~~
+; DAG-VQ-NEXT: dag:3       ^~~
+; DAG-VV-NEXT: dag:3'1     ^~~
+; DAG-Q-NEXT:  dag:4       X~~
+; DAG-VQ-NEXT: dag:4       X~~
+; DAG-VV-NEXT: dag:4'1     X~~
 ; DAG-NEXT:    >>>>>>
 ; DAG-NOT:     {{.}}
 
 ;--------------------------------------------------
 ; CHECK-LABEL
 ;
 ; FIXME: Labels sometimes produce redundant diagnostics for good matches.
 ; That bug is independent of but affects -dump-input.

llvm/utils/FileCheck/FileCheck.cpp

static MatchTypeStyle GetMatchTypeStyle(unsigned MatchTy) {
   case FileCheckDiag::MatchFinalButExcluded:
     return MatchTypeStyle('!', true, raw_ostream::RED, MatchTypeStyle::Quiet,
                           "the final match for an excluded pattern (e.g., "
                           "CHECK-NOT)");
   case FileCheckDiag::MatchFinalButIllegal:
     return MatchTypeStyle('!', true, raw_ostream::RED, MatchTypeStyle::Quiet,
                           "the final but illegal match for an expected "
                           "pattern (e.g., CHECK-NEXT)");
+  case FileCheckDiag::MatchDiscard:
+    return MatchTypeStyle('!', true, raw_ostream::CYAN,
+                          MatchTypeStyle::VerboseVerbose,
+                          "a discarded match for an expected pattern (e.g., "
+                          "CHECK-DAG)");
   case FileCheckDiag::MatchNoneButExpected:
     return MatchTypeStyle('X', true, raw_ostream::RED, MatchTypeStyle::Quiet,
                           "the search range for an unmatched expected "
                           "pattern (e.g., CHECK)");
   case FileCheckDiag::MatchFuzzy:
     return MatchTypeStyle('?', false, raw_ostream::MAGENTA,
                           MatchTypeStyle::Quiet,
                           "a fuzzy match start for an otherwise unmatched "
if (WithColor(OS).colorsEnabled()) {
     if (Req.Verbose) {
       WithColor(OS, raw_ostream::GREEN, true) << "success";
       OS << ", ";
     }
     WithColor(OS, raw_ostream::RED, true) << "error";
     if (Req.Verbose) {
       OS << ", ";
       WithColor(OS, raw_ostream::CYAN, true, true) << "unmatched";
+      if (Req.VerboseVerbose) {
+        OS << ", ";
+        WithColor(OS, raw_ostream::CYAN, true, false) << "discarded";
+      }
     }
     OS << ", ";
     WithColor(OS, raw_ostream::MAGENTA, true) << "fuzzy";
     OS << '\n';
   }
 
   OS << "\nDetailed description of currently enabled markers:\n\n";