Page MenuHomePhabricator

[FileCheck] Annotate input dump (6/7)
ClosedPublic

Authored by jdenny on Oct 30 2018, 2:18 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

jdenny created this revision.Oct 30 2018, 2:18 PM
jdenny updated this revision to Diff 173050.Nov 7 2018, 3:14 PM

Rebased onto updated patches earlier in series.

jdenny updated this revision to Diff 174789.Nov 20 2018, 8:51 AM

Update for changes earlier in patch series.

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 added a comment.Dec 3 2018, 2:14 PM

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 added a comment.Dec 3 2018, 2:31 PM

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 updated this revision to Diff 176903.Dec 5 2018, 4:42 PM
jdenny edited the summary of this revision. (Show Details)
jdenny set the repository for this revision to rL LLVM.

Propagate changes started earlier in patch series.

jdenny updated this revision to Diff 177386.Dec 7 2018, 10:14 PM
jdenny edited the summary of this revision. (Show Details)

Continue changes started earlier in patch series.

jdenny updated this revision to Diff 177944.Dec 12 2018, 3:11 PM
jdenny set the repository for this revision to rL LLVM.

Adjust comment, as suggested by probinson.

probinson accepted this revision.Dec 14 2018, 1:06 PM

LGTM

llvm/test/FileCheck/dump-input-annotations.txt
322 ↗(On Diff #177944)

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

This revision is now accepted and ready to land.Dec 14 2018, 1:06 PM
jdenny added inline comments.Dec 14 2018, 1:12 PM
llvm/test/FileCheck/dump-input-annotations.txt
322 ↗(On Diff #177944)

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

probinson added inline comments.Dec 14 2018, 2:00 PM
llvm/test/FileCheck/dump-input-annotations.txt
322 ↗(On Diff #177944)

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 marked 4 inline comments as done.Dec 15 2018, 5:57 AM
jdenny added inline comments.
llvm/test/FileCheck/dump-input-annotations.txt
322 ↗(On Diff #177944)

Done in D55738.

jdenny marked an inline comment as done.Dec 15 2018, 5:58 AM
This revision was automatically updated to reflect the committed changes.