[FileCheck] Annotate input dump (1/7)

[jdenny]

Summary

Extend FileCheck to dump its input annotated with FileCheck's
diagnostics: errors, good matches if -v, and additional information if
-vv. The goal is to make it easier to visualize FileCheck's matching
behavior when debugging.

Each patch in this series implements input annotations for a
particular category of FileCheck diagnostics. While the first few
patches alone are somewhat useful, the annotations become much more
useful as later patches implement annotations for -v and -vv
diagnostics, which show the matching behavior leading up to the error.

This first patch implements boilerplate plus input annotations for
error diagnostics reporting that no matches were found for a
directive. These annotations mark the search ranges of the failed
directives. Instead of using the usual ^~~, which is used by later
patches for good matches, these annotations use X~~ so that this
category of errors is visually distinct.

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 match result for a pattern of type T from line L of
           the check file
  - X~~    marks search range when no match is found
  - colors error

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

$ FileCheck -v -dump-input=always check1 < input1 |& sed -n '/^Input file/,$p'
Input file: <stdin>
Check file: check1

-dump-input=help describes the format of the following dump.

Full input was:
<<<<<<
        1: ; abc def
        2: ; ghI jkl
next:3     X~~~~~~~~ error: no match found
>>>>>>

$ cat check1
CHECK: abc
CHECK-SAME: def
CHECK-NEXT: ghi
CHECK-SAME: jkl

$ cat input1
; abc def
; ghI jkl

Some additional details related to the boilerplate:

• Enabling: The annotated input dump is enabled by -dump-input, which can also be set via the FILECHECK_OPTS environment variable. Accepted values are help, always, fail, or never. As shown above, help describes the format of the dump. always is helpful when you want to investigate a successful FileCheck run, perhaps for an unexpected pass. -dump-input-on-failure and FILECHECK_DUMP_INPUT_ON_FAILURE remain as a deprecated alias for -dump-input=fail.

• Diagnostics: The usual diagnostics are not suppressed in this mode and are printed first. For brevity in the example above, I've omitted them using a sed command. Sometimes they're perfectly sufficient, and then they make debugging quicker than if you were forced to hunt through a dump of long input looking for the error. If you think they'll get in the way sometimes, keep in mind that it's pretty easy to grep for the start of the input dump, which is <<<.

• Colored Annotations: The annotated input is colored if colors are enabled (enabling colors can be forced using -color). For example, errors are red. However, as in the above example, colors are not vital to reading the annotations.

I don't know how to test color in the output, so any hints here would
be appreciated.

[george.karpenkov]

I don't know how to test color in the output, so any hints here would be appreciated.

I think almost everyone reads FileCheck output through lit, which strips away all color =(

[george.karpenkov]

I am also somewhat uneasy about renaming the option, since it is already contained in some configurations. Could we accept both?

In the long term, I think we should introduce the LIT_DEBUG option, which would set both, so typing a long option name would not be necessary.

[jdenny]

In D52999#1258216, @george.karpenkov wrote:

I don't know how to test color in the output, so any hints here would be appreciated.

I think almost everyone reads FileCheck output through lit, which strips away all color =(

When I'm really baffled by a FileCheck diagnostic, I find myself running the commands manually, so color is useful then. We could also consider teaching lit not to strip away color. In any case, the annotations are usable without color. Color just makes them easier to read.

But does anyone know of a good way to test colored output? My test cases here currently only exercise the dump without color.

In D52999#1258217, @george.karpenkov wrote:

I am also somewhat uneasy about renaming the option, since it is already contained in some configurations. Could we accept both?

I'm fine with that. To avoid questions of precedence, we could make it an error to specify both versions at the same time.

In the long term, I think we should introduce the LIT_DEBUG option, which would set both, so typing a long option name would not be necessary.

Sure. In this patch, I wasn't worried about the length of the option so much. It's just that the old name doesn't make sense for always mode.

[george.karpenkov]

I've added some nits inline.
Overall, this is a huge patch, and I still have trouble understanding all of what it is doing, so I think it should be broken up.

As to testing colors, I think that if the platform is fixed, just checking the color codes should be fine?

llvm/include/llvm/Support/FileCheck.h

131

IMO to be consistent, this parameter should also be a reference

llvm/lib/Support/FileCheck.cpp

1062

Five lines seem to be duplicated with the section above. Could that be extracted to a function? I also think that braces should not be avoided when "else" is present.

llvm/utils/FileCheck/FileCheck.cpp

136

Should this be an enum then?

183

with / changeColor block is duplicated many times, should be extracted into a function.

[jdenny]

In D52999#1258255, @george.karpenkov wrote:

I've added some nits inline.

Thanks for reviewing.

Overall, this is a huge patch, and I still have trouble understanding all of what it is doing, so I think it should be broken up.

I'm happy to do so, but let's agree on the right way to break it up before I put in the effort. Here's my proposed patch sequence:

1. Make changes to include/llvm/Support/FileCheck.h and lib/Support/FileCheck.cpp, which gather the list of diagnostics.
2. Make changes to utils/FileCheck/FileCheck.cpp, which converts the list of diagnostics to annotations on the input.
3. Adjust command-line option and env var.
4. Expose colorsEnabled in WithColor.h.

On second thought, these are not logically distinct changes as none of them really has much purpose on its own. You might just consider reviewing files in the above order. Your call. Let me know.

As to testing colors, I think that if the platform is fixed, just checking the color codes should be fine?

You mean ANSI escape sequences? That would make for some rather unreadable expected output. There must be a better way. If only we had a tool that converts those sequences into something human-readable, like XML tags.

llvm/include/llvm/Support/FileCheck.h

131

It might be a nullptr.

llvm/lib/Support/FileCheck.cpp

1062

Five lines seem to be duplicated with the section above. Could that be extracted to a function?

Will do.

I also think that braces should not be avoided when "else" is present.

That's a new rule to me. It doesn't appear to be followed elsewhere in this file. Is it followed elsewhere in LLVM?

llvm/utils/FileCheck/FileCheck.cpp

136

Will do.

183

Will do.

[jdenny]

Made most of the reviewer suggestions.

[jdenny]

In D52999#1258477, @jdenny wrote:

1. Make changes to include/llvm/Support/FileCheck.h and lib/Support/FileCheck.cpp, which gather the list of diagnostics.
2. Make changes to utils/FileCheck/FileCheck.cpp, which converts the list of diagnostics to annotations on the input.
3. Adjust command-line option and env var.
4. Expose colorsEnabled in WithColor.h.

On second thought, these are not logically distinct changes as none of them really has much purpose on its own. You might just consider reviewing files in the above order. Your call. Let me know.

Actually, 3 and 4 are logically distinct. There just isn't much code associated with them, so I'm not sure splitting them off would address your concern. Still, let me know what would help you to review.

[jdenny]

Restore -dump-input-on-failure as requested, and clean up a little.

I decided it's easiest to keep -dump-input-on-failure as an independent option that does not include annotations. I marked it deprecated.

[george.karpenkov]

@jdenny Sorry I'm still struggling to understand what exactly are you doing.
You have one example in the revision description, and some examples in tests. The examples in tests are hard to read because they are also tested using FileCheck.

Could you add a few more examples in the revision description, and add a detailed explanation of what your feature is doing, and what is the exact semantics of added annotations?
(what exactly is "chk"? I guess short for "check"? And "sam" short for "same"? The semantics of ~, ^ and x also eludes me).

[jdenny]

In D52999#1259882, @george.karpenkov wrote:

@jdenny Sorry I'm still struggling to understand what exactly are you doing.

Sorry. It's obviously not as self-explanatory as I was hoping. I've probably been staring at it too long. Thanks for letting me know.

You have one example in the revision description, and some examples in tests. The examples in tests are hard to read because they are also tested using FileCheck.

Could you add a few more examples in the revision description, and add a detailed explanation of what your feature is doing, and what is the exact semantics of added annotations?
(what exactly is "chk"? I guess short for "check"? And "sam" short for "same"? The semantics of ~, ^ and x also eludes me).

I've extended the description as you suggest. Please let me know if it's still confusing.

[george.karpenkov]

OK that's somewhat more clear, but I'm still somewhat confused. Line by line:

$ FileCheck -v -dump-input=always checks1 < input1 |& sed -n '/^Key for/,$p'

I assume sed is there to suppress all output before the legend is printed?

• Should legend be always printed? Wouldn't it make more sense to dump it only if requested?

Key for input dump annotations:

  - L:     labels input line number L

In general, from your legend it's hard to figure out what line refers to what.
Here I assume that this item refers to line numbers from the matched files,
but it takes some guessing and looking at the output.

- T:L    labels the only match result for a pattern of type T from line L
- T:L'N  labels the Nth match result for a pattern of type T from line L

I do not understand what N'th match is.

- ^~~    marks good match (requires -v)
- !~~    marks bad match

I could not understand what "bad match" is, could only get it from a more detailed description later

- X~~    marks search range when no match

"when no match is found" ?

- ?      marks fuzzy match when no match

I don't understand this line. Is it a best-effort match when no match is found? Where the question mark is situated then?

Detailed description of currently enabled markers:

Should the description of markers be always duplicated?

- ^~~    marks the final match for an expected pattern
- !~~    marks either:
         - the final match for an excluded pattern
         - the final but illegal match for an expected pattern

The explanation is not clear. What is "final"? It's better to clarify that excluded means supplied with "NOT".
It's not clear what "illegal" means here either.

- X~~    marks the search range for an unmatched expected pattern

Where is X located? Just at the start of the range?

- ?      marks a fuzzy match start for an otherwise unmatched pattern

What's the difference between X and a question mark?

Full input was:
<<<<<<
         1: ; abc def

Line numbering may become ambiguous with the input, especially the space after the colon.
Is line numbering required? Should there be a better separation?

chk:1         ^~~

It's confusing that (from my understanding) line numbers above refer to line numbers in the input document,
but line numbers here refer to line numbers in the file with FileCheck directives.

sam:2             ^~~

sam is not the best abbreviation for "SAME". Maybe spare another letter? Or use "sme" or something?

         2: ; ghI jkl
nxt:3'0     X~~~~~~~~

I don't understand the semantics here. What's '0?
Why X is below the semicolon? If it's always at the start of the line, should it be there at all?

nxt:3'1       ?

What is the purpose of this question mark? If we have already failed the search at this point because the previous pattern failed,
does it convey any information to put the question mark at the start of the line?
I don't understand what '1 means here either.

>>>>>>

$ cat checks1
CHECK: abc
CHECK-SAME: def
CHECK-NEXT: ghi
CHECK-SAME: jkl

$ cat input1
; abc def
; ghI jkl

[jdenny]

In D52999#1260736, @george.karpenkov wrote:

OK that's somewhat more clear, but I'm still somewhat confused. Line by line:

$ FileCheck -v -dump-input=always checks1 < input1 |& sed -n '/^Key for/,$p'

I assume sed is there to suppress all output before the legend is printed?

Yes. If you think all the usual diagnostics should be included here, I can remove the sed command.

• Should legend be always printed? Wouldn't it make more sense to dump it only if requested?

I considered that. However, the output is already typically long given that we're dumping the input and probably have -v enabled. The legend doesn't add much more, so I don't see the harm in including it always. Why make life more difficult by forcing the user to try again with another option? It seems best to always put the information close to where the user needs it.

Key for input dump annotations:

  - L:     labels input line number L

In general, from your legend it's hard to figure out what line refers to what.
Here I assume that this item refers to line numbers from the matched files,
but it takes some guessing and looking at the output.

It says "input line", so I thought that would make it clear.

- T:L    labels the only match result for a pattern of type T from line L

Maybe it would be clearer if this were to say "checks line"?

• T:L'N labels the Nth match result for a pattern of type T from line L

I do not understand what N'th match is.

It's an Nth match *result*. For example, one result for a pattern might be that it has no match in a particular search range. Another result might be the fuzzy match that FileCheck sometimes reports after that. For CHECK-DAG and -vv, you can also have a series of discarded matches.

Should it say "diagnostic" instead of "match result"?

- ^~~    marks good match (requires -v)
- !~~    marks bad match

I could not understand what "bad match" is, could only get it from a more detailed description later

Yes, I meant this as a high-level summary before you read all the details. I also thought it would be a nice reminder for someone who has read all this before. If you have suggestions on a better organization, please let me know.

- X~~    marks search range when no match

"when no match is found" ?

Just trying to be succinct. I can spell it out if that helps.

- ?      marks fuzzy match when no match

I don't understand this line. Is it a best-effort match when no match is found?

Yes. FileCheck already produces all these diagnostics. I'm just representing them as annotations.

Where the question mark is situated then?

The start of the fuzzy match. FileCheck currently doesn't report the full range of a fuzzy match, and I didn't try to change that.

Detailed description of currently enabled markers:

Should the description of markers be always duplicated?

I originally just had the detailed description but felt it was too intimidating, and I thought the short version before it helped to give a sense of the markers before the details. Again, I'm open to suggestions on how to improve it.

- ^~~    marks the final match for an expected pattern
- !~~    marks either:
         - the final match for an excluded pattern
         - the final but illegal match for an expected pattern

The explanation is not clear. What is "final"?

Not a discarded match, which sometimes happens for CHECK-DAG.

It's better to clarify that excluded means supplied with "NOT".

I was trying to keep it general in case we grow other directives with excluded patterns.

It's not clear what "illegal" means here either.

CHECK-NEXT and CHECK-SAME can match patterns and then complain they're illegal.

For those cases, I'll add examples of directives that are affected.

- X~~    marks the search range for an unmatched expected pattern

Where is X located? Just at the start of the range?

Yes. I thought it would be intuitive to LLVM developers (especially those who have read FileCheck's existing diagnostics, which already include ^~~) that X~~ and !~~ mark ranges like ^~~ but for different purposes.

- ?      marks a fuzzy match start for an otherwise unmatched pattern

What's the difference between X and a question mark?

Search range vs fuzzy match.

Full input was:
<<<<<<
         1: ; abc def

Line numbering may become ambiguous with the input, especially the space after the colon.

How? Every input line begins with a line number, colon, and space. It's so unambiguous, you can write a simple sed expression to extract just the input text from the annotated dump. Maybe you're saying I should mention the space in the legend?

Is line numbering required?

Without line numbering, you cannot always distinguish input lines and annotation lines. Moreover, the line numbers help when you see a diagnostic before the dump and want to search for the corresponding line.

Should there be a better separation?

I'm open to suggestions, but this seems to me like the most obvious way to do it.

chk:1         ^~~

It's confusing that (from my understanding) line numbers above refer to line numbers in the input document,
but line numbers here refer to line numbers in the file with FileCheck directives.

That's true throughout existing FileCheck diagnostics, and I don't know what to do about it. The user must remember that input and checks are (sometimes) in different files.

sam:2             ^~~

sam is not the best abbreviation for "SAME". Maybe spare another letter? Or use "sme" or something?

sme is fine by me.

         2: ; ghI jkl
nxt:3'0     X~~~~~~~~

I don't understand the semantics here. What's '0?

Diagnostic 0 for the CHECK-NEXT on line 3.

Why X is below the semicolon?

Because that's the start of the search range.

If it's always at the start of the line, should it be there at all?

Sometimes the search range doesn't start at the start of the line because the last match didn't end at the end of the previous line.

nxt:3'1       ?

What is the purpose of this question mark? If we have already failed the search at this point because the previous pattern failed,
does it convey any information to put the question mark at the start of the line?

It's not at the start of the line. It's at the start of the fuzzy match, which is later.

I don't understand what '1 means here either.

Diagnostic 1 for the CHECK-NEXT on line 3.

>>>>>>

$ cat checks1
CHECK: abc
CHECK-SAME: def
CHECK-NEXT: ghi
CHECK-SAME: jkl

$ cat input1
; abc def
; ghI jkl

Thanks for all your helpful comments. Please keep them coming, and I'll apply changes as we agree on them.

[jdenny]

Address reviewer concerns:

• Add examples of match result types.
• Document space that follows an input line number.
• Clarify input file vs. check file for line numbers.
• "when no match" -> "when no match is found".
• Don't abbreviate directive types.

That's everything I know to do for now.

[jdenny]

Adjusted to take advantage of D53419, which extends WithColor.

I think that finishes separating out all logically distinct change sets. I'm willing to split up the patch more if it helps the review.

[george.karpenkov]

@jdenny

I'm willing to split up the patch more if it helps the review

Yes please, to be honest I can't still fully understand the feature description, let alone the code.
I have asked my colleague to take a glance, and his verdict was the same, so I'm not alone there.

You have a lot of new features in your FileCheck output; could you think of a way to separate them into simple, small, understandable chunks?

[jdenny]

In D52999#1271022, @george.karpenkov wrote:

@jdenny

I'm willing to split up the patch more if it helps the review

Yes please, to be honest I can't still fully understand the feature description, let alone the code.
I have asked my colleague to take a glance, and his verdict was the same, so I'm not alone there.

You have a lot of new features in your FileCheck output; could you think of a way to separate them into simple, small, understandable chunks?

OK, but please work with me to plan a way to break up the series that would actually prove helpful to you. Here are two proposals:

1. Here's the plan I proposed in an earlier comment, but I've updated it for recent changes:
   1. Make changes to include/llvm/Support/FileCheck.h and lib/Support/FileCheck.cpp, which gather the list of diagnostics.
   2. Make changes to utils/FileCheck/FileCheck.cpp, which converts the list of diagnostics to annotations on the input. Include test and doc changes in this patch because this is where they become effective.
      
      Because you see FileCheck's *output* as many new features, I'm afraid all the parts you find confusing would all appear in the second patch, so I'm not sure this plan would prove helpful to you.

2. This patch introduces input annotations for existing diagnostics. Those diagnostics are enumerated in MatchType in FileCheckDiag in llvm/include/llvm/Support/FileCheck.h. Would it be helpful to have each patch introduce annotations for one MatchType member? The first patch would likely be the largest as it would introduce boilerplate.

Please let me know which of these proposals you prefer, or propose something else.

Thanks for your help.

[jdenny]

In D52999#1271097, @jdenny wrote:

2. This patch introduces input annotations for existing diagnostics. Those diagnostics are enumerated in MatchType in FileCheckDiag in llvm/include/llvm/Support/FileCheck.h. Would it be helpful to have each patch introduce annotations for one MatchType member? The first patch would likely be the largest as it would introduce boilerplate.

Unless someone (immediately) says it won't be helpful, I'm going to put some effort into this approach at splitting up the patch.

[jdenny]

• Split up the patch. This is now the first in the series. Each patch has a description and example of the diagnostics for which it implements input annotations.
• Add the input file name and check file name to the annotations description to further address a reviewer concern.
• Don't use the word "key" to identify the annotations description because technically that's not what it is.

[zturner]

llvm/docs/CommandGuide/FileCheck.rst

83–85

I haven't been following this too closely, but I'm wondering, are the 3 modes actually necessary? It sounds like the main use case here is that the user has a failure and wants to get more info. So they will set it to either fail or always. But do they care which? Basically, what I'm wondering is why not just make this be a binary on flag?

It just seems simpler to say that if you want to dump the input, pass --dump-input, and if you don't want to dump the input, pass nothing.

llvm/test/FileCheck/match-full-lines.txt

4–7 ↗

(On Diff #171782)

You could also achieve this without the command line flag by writing env FILECHECK_DUMP_INPUT= not FileCheck .... Pretty sure you can unset a variable and run a command this way.

[jdenny]

llvm/docs/CommandGuide/FileCheck.rst

83–85

-dump-input=always is helpful when you're debugging individual tests and encounter FileCheck successes you don't understand.

I was thinking -dump-input=fail (via an env var) is better when running full test suites, perhaps from a bot or IDE. Imagine a test with many successful FileCheck commands before the failed one. -dump-input=always might produce massive output that will be logged and that must be scrolled/grepped through to find the failure, depending on how you like to interact with test suite logs.

In any case, -dump-input=fail was inspired by the existing -dump-input-on-failure, which I believe @george.karpenkov said is used in bots. Is that right, George?

llvm/test/FileCheck/match-full-lines.txt

4–7 ↗

(On Diff #171782)

I don't recall why I did it that way, and your way does seem more obvious. I'll change it.

While we're talking about this, I'd eventually like to handle this issue in another way entirely. In FileCheck's own test suite, I'd like to ban direct FileCheck calls. Instead, I'd like to require test authors to choose one of the following for each call:

• %FileCheckee: Used for FileCheck calls whose exit status and output are being checked. Normal FILECHECK_* environment variables are cleared within these calls. This avoids problems where people have these variables set for general testing purposes and thus change the FileCheck output being tested, creating spurious fails or passes. If these calls need to test env vars, then we could just have an alternate set of environment variables (FILECHECKEE_* maybe) that %FileCheckee copies to the normal variables.

• %FileChecker: Used for FileCheck calls that check the output of %FileCheckee calls (or any other output under test). FILECHECK_* environment variables are obeyed just as they are obeyed for FileCheck calls in other test suites.

Of course, this would be another patch, which I don't have time for right now. Do you think the idea has merit?

[jdenny]

• Address zturner's comment: prefer FILECHECK_DUMP_INPUT=never over -dump-input=never when avoiding user environment variables.
• Set FILECHECK_DUMP_INPUT=never in test/FileCheck/verbose.txt as well. Somehow I didn't notice this one was failing before.

[jdenny]

• Rebased.
• Removed FILECHECK_DUMP_INPUT env var. The new FILECHECK_OPTS is sufficient.
• Removed unsetting of FILECHECK_* env vars in tests. That should be handled more carefully in a separate patch.

[probinson]

It really seems like DiagList and AnnotationList ought to be vectors, not lists. They are append-only, and AnnotationList gets converted to a vector anyway to sort it. The code doesn't depend on the element-pointer stability guarantee of a list, except in one place noted below which can be fixed.
It's quite possible a vector would perform less well, in the face of many diags/annotations, but as the diags/annotations are the unusual case, performance is not really a big consideration.

llvm/utils/FileCheck/FileCheck.cpp

245

range-for here?

283

This appears to be the only place that functionally depends on AnnotationList being a <list>. But if you built B as a stack instance first, then you can push_back when you're done, and then AnnotationList can be a <vector> instead.

[probinson]

Regarding the bit about environment variables, probably the right thing to do is add a lit.local.cfg to the FileCheck test directory, that zaps the environment variables. Then most individual FileCheck tests can assume a default environment, and tests for specific non-defaults can set the environment variables directly in the test file.
It would mean you can't run the FileCheck tests *from lit* with an environment variable set. Unless we invented another env var that lit.local.cfg would look for... but that's for another patch, at this point.

[jdenny]

In D52999#1301285, @probinson wrote:

It really seems like DiagList and AnnotationList ought to be vectors, not lists. They are append-only, and AnnotationList gets converted to a vector anyway to sort it. The code doesn't depend on the element-pointer stability guarantee of a list, except in one place noted below which can be fixed.
It's quite possible a vector would perform less well, in the face of many diags/annotations, but as the diags/annotations are the unusual case, performance is not really a big consideration.

I was thinking of many diags (-v with many checks) because that's where the annotations have proven most helpful to me as a visualization. I was thinking the conversion to vector would be a small penalty in comparison to the potentially many copies during reiszing the vector. I was more concerned about the time impact than the space impact, which I guessed would be small on a modern system.

Having said all that, I don't have a strong opinion here, and we can change it later if someone does some performance measurements, so I'd be happy to use vector now with no further discussion if my above arguments aren't convincing. Just say the word.

In D52999#1301364, @probinson wrote:

Regarding the bit about environment variables, probably the right thing to do is add a lit.local.cfg to the FileCheck test directory, that zaps the environment variables. Then most individual FileCheck tests can assume a default environment, and tests for specific non-defaults can set the environment variables directly in the test file.
It would mean you can't run the FileCheck tests *from lit* with an environment variable set. Unless we invented another env var that lit.local.cfg would look for... but that's for another patch, at this point.

That's fine as a first step to addressing that issue. However we do it, I think it should move to a separate patch as the issue exists independently of this patch series. Agreed?

By the way, this issue also exists in the lit test suite.

llvm/utils/FileCheck/FileCheck.cpp

245

D53893 (later patch in this series) makes use of the iterators. Let me know if you think there's a better way.

[jdenny]

• Rebase, and extend for the new CHECK-COUNT-<num> directive.
• Convert DiagList and AnnotationList to std::vector, as probinson recommended.

[jdenny]

Ping.

[probinson]

Apologies for the delay. I haven't been ignoring this series; I was having internal qualms about the amount of effort to produce extensive annotations, and the value they might provide. But I've come down in favor of doing it.

llvm/docs/CommandGuide/FileCheck.rst

92

in favor of --dump-input=fail.

llvm/utils/FileCheck/FileCheck.cpp

102

There's a way to make the argument be an enum, which has a variety of advantages. Please do it that way.

159

I'd omit the S part. 6: is clearly a line number, you don't need to document that the colon has a space after it.

498

DumpInput == "never" ? nullptr : &Diags
so we don't bother collecting diags that we will never print. Saves a small bit of time and memory, but this tool is used a *lot* in the default "never" mode and it's worth doing that small optimization.

499

So, I can say -dump-input-on-failure -dump-input=fail and it will dump the input twice? I think -dump-input-on-failure should just set -dump-input=fail (if -dump-input didn't appear separately, i.e. the new option takes precedence) and you only get one dump.

501

The detailed description of the annotations becomes long enough that I think including it with the dumped input starts to get in the way. Maybe have a -dump-input=help that will print the description and quit, or something along those lines.

[jdenny]

In D52999#1315065, @probinson wrote:

Apologies for the delay. I haven't been ignoring this series; I was having internal qualms about the amount of effort to produce extensive annotations, and the value they might provide.

No problem. Thanks for the careful consideration.

But I've come down in favor of doing it.

Great! I'll work on your suggestions soon.

llvm/utils/FileCheck/FileCheck.cpp

499

So, I can say -dump-input-on-failure -dump-input=fail and it will dump the input twice?

I was thinking -dump-input-on-failure would be removed eventually, so I decided to be lazy. I'll change it.

I think -dump-input-on-failure should just set -dump-input=fail (if -dump-input didn't appear separately, i.e. the new option takes precedence) and you only get one dump.

Do you want -dump-input=never -dump-input-on-failure to be the same as -dump-input=never or -dump-input=fail?

501

The detailed description of the annotations becomes long enough that I think including it with the dumped input starts to get in the way. Maybe have a -dump-input=help that will print the description and quit, or something along those lines.

If the input dump is short, I usually don't need the dump. I usually need the dump when it's long, and then the description is relatively tiny and doesn't feel like it's in the way. Moreover, I don't want to force users to remember how to obtain that description (FileCheck isn't even normally in my PATH, so that's another barrier), so I think it's more convenient just to print it with the dump.

However, you're the second person with your opinion, so I'm outvoted, and I'm fine with that. Any objection to always dumping a reminder that -dump-input=help exists?

[probinson]

llvm/utils/FileCheck/FileCheck.cpp

499

cl::opt doesn't support command-line-order checks, so what I'd do is have the -dump-input enum have a Default. Then if -dump-input is Default, you set it to Never or Fail depending on -dump-input-on-failure. If/when we get rid of -dump-input-on-failure, we can set the -dump-input default to Never and get rid of the Default enum too.

501

A reminder that -dump-input=help exists would be totally appropriate.

[jdenny]

llvm/utils/FileCheck/FileCheck.cpp

501

A reminder that -dump-input=help exists would be totally appropriate.

I'm assuming that -dump-input=fail might be used by bots, and I'm thinking about what happens when someone is reading without a terminal handy to run FileCheck -dump-input=help. Should we assume such people will quickly become familiar enough with these annotations that they don't need the description, or should we offer something more?

Perhaps the description should also appear in rst/html documentation, and perhaps the reminder should be a pointer to that instead of -dump-input=help because the former is more universally accessible.

What do you think?

[probinson]

llvm/utils/FileCheck/FileCheck.cpp

501

If a test failure is so involved that the annotations would be helpful, I think people would be running the test locally to try to debug it. So, getting the help from the tool should be fine.

[jdenny]

Made changes suggested by probinson:

• In docs, say -dump-input-on-failure is deprecated in favor of -dump-input=fail not -dump-input.
• Use cl::values (enum) for -dump-input value.
• In annotation description, don't document space after line number.
• Don't bother collecting diagnostics if -dump-input=never.
• Instead of leaving -dump-input-on-failure as a separate feature, make it an alias for -dump-input=fail but only when -dump-input is not otherwise specified.
• Move annotation description to a -dump-input=help option.

I also removed the detailed description of the currently enabled
markers. In place of that, I added a few notes to the brief
description that preceded it. Here's why:

• The reviews so far have led me to believe that having two descriptions was more confusing than helpful. George questioned whether the marker descriptions should be repeated (D52999#1260736). George didn't understand the short version of the !~~ description (D52999#1260736). Paul questioned why !~~ should represent both a discarded match and actual errors (D53898#1317412). (Perhaps we really need an additional marker, but my assumption is that the docs just aren't clear.)
• The detailed description added complexity to multiple parts of the existing implementation (MatchType, MatchTypeStyle, and of course DumpInputAnnotationHelp).
• Previously the detailed description changed based on whether color was enabled and based on the verbosity level. That seemed to make sense when the description printed with the dump. Now it requires the user to be sure to specify certain options the same when requesting help as when requesting the dump. The options in the latter case might be buried in a FILECHECK_OPTS in a bot, IDE, or script, so getting the options right for help could be error-prone.

If you feel that part of this change makes things worse, I can revert
it. Hopefully it's a helpful simplification and can be tweaked to
address any further problems.

[jdenny]

llvm/docs/CommandGuide/FileCheck.rst

83–85

Seeing no followup for a while, I'm assuming my response here was satisfactory, and I'm marking this done.

llvm/lib/Support/FileCheck.cpp

1062

Seeing no followup for a while, I'm assuming my response here was satisfactory, and I'm marking this done.

llvm/utils/FileCheck/FileCheck.cpp

245

Seeing no followup for a while, I'm assuming my response here was satisfactory, and I'm marking this done.

[jdenny]

Why did I have to set the repository again? I already did that on 10/30.

[jdenny]

Another way to improve the clarity of markers (to avoid the confusion discussed in, for example, D53898#1317412) occurred to me: Except for markers indicating successful completion of a directive (green ^~~ for most directives, and green X~~ for CHECK-NOT), I could add a note at the end of the marker. For example:

$ FileCheck -vv -dump-input=always check4a < input4a |& sed -n '/^<<<</,$p'
<<<<<<
         1: 01234
check:1     ^~
not:2         X~~
         2: 56789
not:2       ~~~~~
         3: abcdef
dag:3       ^~~~
dag:4'0       !~~~ discard: overlaps earlier match
         4: cdefgh
dag:4'1     ^~~~
next:5          !~ error: same line as previous match
>>>>>>

$ cat check4a
CHECK: 01
CHECK-NOT: foobar
CHECK-DAG: abcd
CHECK-DAG: cdef
CHECK-NEXT: gh

$ cat input4a 
01234
56789
abcdef
cdefgh

This shouldn't add much complexity to the implementation. Unless someone is opposed, I'll try to work on it soon.

[jdenny]

Add explanatory note to any marker not indicating successful completion of a directive (that is, anything not green).

[probinson]

I like having the extra explanations at the end of the tilde lines, that's a great idea.
One unnecessary include left, and LGTM.

llvm/include/llvm/Support/FileCheck.h

21

I believe you don't need <list> anymore.

[jdenny]

In D52999#1326125, @probinson wrote:

I like having the extra explanations at the end of the tilde lines, that's a great idea.
One unnecessary include left, and LGTM.

Thanks! I'll fix the include soon.

I won't commit this immediately as I'm thinking it's best to commit the entire patch series at the same time. Let me know if you feel differently.

[jdenny]

Remove unnecessary include.

[probinson]

A couple of nits I didn't notice until looking at the next patch.

llvm/include/llvm/Support/FileCheck.h

162

Comments should be proper sentences.

167

The style guide doesn't actually say this, but it's pretty much universal in the code base to declare each member separately.

[jdenny]

llvm/include/llvm/Support/FileCheck.h

162

Thanks. Before I adjust all the patches, does the following work, or was this more than a style issue?

/// Indicates no match for an expected pattern.

[probinson]

llvm/include/llvm/Support/FileCheck.h

162

Just style. What you propose is fine.

[jdenny]

Adjust comment, as suggested by probinson.

Change GetMarker's parameter type from unsigned to FileCheckDiag::MatchType.

[jdenny]

llvm/include/llvm/Support/FileCheck.h

167

Ah, I missed that. I'll get it before committing.

[jdenny]

llvm/include/llvm/Support/FileCheck.h

167

Done in D55738.

llvm/docs/CommandGuide/FileCheck.rst

.. option:: --implicit-check-not check-pattern
   checks. The option allows writing stricter tests without stuffing them with
   ``CHECK-NOT``\ s.
 
   For example, "``--implicit-check-not warning:``" can be useful when testing
   diagnostic messages from tools that don't have an option similar to ``clang
   -verify``. With this option FileCheck will verify that input does not contain
   warnings not covered by any ``CHECK:`` patterns.
 
+.. option:: --dump-input <mode>
+
+  Dump input to stderr, adding annotations representing currently enabled
+  diagnostics.  Do this either 'always', on 'fail', or 'never'.  Specify 'help'
+  to explain the dump format and quit.
+
 .. option:: --dump-input-on-failure
 
-  When the check fails, dump all of the original input.
+  When the check fails, dump all of the original input.  This option is
+  deprecated in favor of `--dump-input=fail`.
 
 .. option:: --enable-var-scope
 
   Enables scope for regex variables.
 
   Variables with names that start with ``$`` are considered global and
   remain set throughout the file.
 

llvm/include/llvm/Support/FileCheck.h

 
 #ifndef LLVM_SUPPORT_FILECHECK_H
 #define LLVM_SUPPORT_FILECHECK_H
 
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SourceMgr.h"
 #include <vector>
 #include <map>
 
 namespace llvm {
 
 /// Contains info about various FileCheck options.
 struct FileCheckRequest {
   std::vector<std::string> CheckPrefixes;
   bool NoCanonicalizeWhiteSpace = false;
public:
 
   int getCount() const { return Count; }
   FileCheckType &setCount(int C);
 
   std::string getDescription(StringRef Prefix) const;
 };
 }
 
+struct FileCheckDiag;
+
 class FileCheckPattern {
   SMLoc PatternLoc;
 
   /// A fixed string to match as the pattern or empty if this pattern requires
   /// a regex match.
   StringRef FixedStr;
 
   /// A regex string to match as the pattern or empty if this pattern requires
bool ParsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
                     unsigned LineNumber, const FileCheckRequest &Req);
   size_t Match(StringRef Buffer, size_t &MatchLen,
                StringMap<StringRef> &VariableTable) const;
   void PrintVariableUses(const SourceMgr &SM, StringRef Buffer,
                          const StringMap<StringRef> &VariableTable,
                          SMRange MatchRange = None) const;
   void PrintFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        const StringMap<StringRef> &VariableTable) const;
 
   bool hasVariable() const {
     return !(VariableUses.empty() && VariableDefs.empty());
   }
 
   Check::FileCheckType getCheckTy() const { return CheckTy; }
 
   int getCount() const { return CheckTy.getCount(); }
 
 private:
   bool AddRegExToRegEx(StringRef RS, unsigned &CurParen, SourceMgr &SM);
   void AddBackrefToRegEx(unsigned BackrefNum);
   unsigned
   ComputeMatchDistance(StringRef Buffer,
                        const StringMap<StringRef> &VariableTable) const;
   bool EvaluateExpression(StringRef Expr, std::string &Value) const;
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
 };
 
 //===----------------------------------------------------------------------===//
+/// Summary of a FileCheck diagnostic.
+//===----------------------------------------------------------------------===//
+
+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?
+  enum MatchType {
+    // TODO: More members will appear with later patches in this series.
+    /// Indicates no match for an expected pattern.
+    MatchNoneButExpected,
+    MatchTypeCount,
+  } MatchTy;
+  /// The search range.
+  unsigned InputStartLine, InputStartCol, InputEndLine, InputEndCol;
+  FileCheckDiag(const SourceMgr &SM, const Check::FileCheckType &CheckTy,
+                SMLoc CheckLoc, MatchType MatchTy, SMRange InputRange);
+};
+
+//===----------------------------------------------------------------------===//
 // Check Strings.
 //===----------------------------------------------------------------------===//
 
 /// A check that we found in the input file.
 struct FileCheckString {
   /// The pattern to match.
   FileCheckPattern Pat;
 
   /// Which prefix name this check matched.
   StringRef Prefix;
 
   /// The location in the match file that the check string was specified.
   SMLoc Loc;
 
   /// All of the strings that are disallowed from occurring between this match
   /// string and the previous one (or start of file).
   std::vector<FileCheckPattern> DagNotStrings;
 
   FileCheckString(const FileCheckPattern &P, StringRef S, SMLoc L)
       : Pat(P), Prefix(S), Loc(L) {}
 
   size_t Check(const SourceMgr &SM, StringRef Buffer, bool IsLabelScanMode,
                size_t &MatchLen, StringMap<StringRef> &VariableTable,
-               FileCheckRequest &Req) const;
+               FileCheckRequest &Req, std::vector<FileCheckDiag> *Diags) const;
 
   bool CheckNext(const SourceMgr &SM, StringRef Buffer) const;
   bool CheckSame(const SourceMgr &SM, StringRef Buffer) const;
   bool CheckNot(const SourceMgr &SM, StringRef Buffer,
                 const std::vector<const FileCheckPattern *> &NotStrings,
                 StringMap<StringRef> &VariableTable,
                 const FileCheckRequest &Req) const;
   size_t CheckDag(const SourceMgr &SM, StringRef Buffer,
                   std::vector<const FileCheckPattern *> &NotStrings,
                   StringMap<StringRef> &VariableTable,
-                  const FileCheckRequest &Req) const;
+                  const FileCheckRequest &Req,
+                  std::vector<FileCheckDiag> *Diags) const;
 };
 
 /// FileCheck class takes the request and exposes various methods that
 /// use information from the request.
 class FileCheck {
   FileCheckRequest Req;
 
 public:
public:
   StringRef CanonicalizeFile(MemoryBuffer &MB,
                              SmallVectorImpl<char> &OutputBuffer);
 
   /// Check the input to FileCheck provided in the \p Buffer against the \p
   /// CheckStrings read from the check file.
   ///
   /// Returns false if the input fails to satisfy the checks.
   bool CheckInput(SourceMgr &SM, StringRef Buffer,
-                  ArrayRef<FileCheckString> CheckStrings);
+                  ArrayRef<FileCheckString> CheckStrings,
+                  std::vector<FileCheckDiag> *Diags = nullptr);
 };
 } // namespace llvm
 #endif

llvm/lib/Support/FileCheck.cpp

for (const auto &VariableUse : VariableUses) {
                         {MatchRange});
       else
         SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()),
                         SourceMgr::DK_Note, OS.str());
     }
   }
 }
 
+static SMRange ProcessMatchResult(FileCheckDiag::MatchType MatchTy,
+                                  const SourceMgr &SM, SMLoc Loc,
+                                  Check::FileCheckType CheckTy,
+                                  StringRef Buffer, size_t Pos, size_t Len,
+                                  std::vector<FileCheckDiag> *Diags) {
+  SMLoc Start = SMLoc::getFromPointer(Buffer.data() + Pos);
+  SMLoc End = SMLoc::getFromPointer(Buffer.data() + Pos + Len);
+  SMRange Range(Start, End);
+  // TODO: The second condition will disappear when we extend this to handle
+  // more match types.
+  if (Diags && MatchTy != FileCheckDiag::MatchTypeCount)
+    Diags->emplace_back(SM, CheckTy, Loc, MatchTy, Range);
+  return Range;
+}
+
 void FileCheckPattern::PrintFuzzyMatch(
     const SourceMgr &SM, StringRef Buffer,
     const StringMap<StringRef> &VariableTable) const {
   // Attempt to find the closest/best fuzzy match.  Usually an error happens
   // because some string in the output didn't exactly match. In these cases, we
   // would like to show the user a best guess at what "should have" matched, to
   // save them having to actually check the input manually.
   size_t NumLinesForward = 0;
while (Ptr + 1 != End && (Ptr[1] == ' ' || Ptr[1] == '\t'))
       ++Ptr;
   }
 
   // Add a null byte and then return all but that byte.
   OutputBuffer.push_back('\0');
   return StringRef(OutputBuffer.data(), OutputBuffer.size() - 1);
 }
 
+FileCheckDiag::FileCheckDiag(const SourceMgr &SM,
+                             const Check::FileCheckType &CheckTy,
+                             SMLoc CheckLoc, MatchType MatchTy,
+                             SMRange InputRange)
+    : CheckTy(CheckTy), MatchTy(MatchTy) {
+  auto Start = SM.getLineAndColumn(InputRange.Start);
+  auto End = SM.getLineAndColumn(InputRange.End);
+  InputStartLine = Start.first;
+  InputStartCol = Start.second;
+  InputEndLine = End.first;
+  InputEndCol = End.second;
+  Start = SM.getLineAndColumn(CheckLoc);
+  CheckLine = Start.first;
+  CheckCol = Start.second;
+}
+
 static bool IsPartOfWord(char c) {
   return (isalnum(c) || c == '-' || c == '_');
 }
 
 Check::FileCheckType &Check::FileCheckType::setCount(int C) {
   assert(Count > 0 && "zero and negative counts are not supported");
   assert((C == 1 || Kind == CheckPlain) &&
          "count supported only for plain CHECK directives");
static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
   PrintMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
              MatchedCount, Buffer, VariableTable, MatchPos, MatchLen, Req);
 }
 
 static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          StringRef Prefix, SMLoc Loc,
                          const FileCheckPattern &Pat, int MatchedCount,
                          StringRef Buffer, StringMap<StringRef> &VariableTable,
-                         bool VerboseVerbose) {
+                         bool VerboseVerbose,
+                         std::vector<FileCheckDiag> *Diags) {
   if (!ExpectedMatch && !VerboseVerbose)
     return;
 
   // Otherwise, we have an error, emit an error message.
   std::string Message = formatv("{0}: {1} string not found in input",
                                 Pat.getCheckTy().getDescription(Prefix),
                                 (ExpectedMatch ? "expected" : "excluded"))
                             .str();
   if (Pat.getCount() > 1)
     Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
 
   SM.PrintMessage(
       Loc, ExpectedMatch ? SourceMgr::DK_Error : SourceMgr::DK_Remark, Message);
 
   // Print the "scanning from here" line.  If the current position is at the
   // end of a line, advance to the start of the next line.
   Buffer = Buffer.substr(Buffer.find_first_not_of(" \t\n\r"));
-
-  SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()), SourceMgr::DK_Note,
-                  "scanning from here");
+  SMRange SearchRange = ProcessMatchResult(
+      ExpectedMatch ? FileCheckDiag::MatchNoneButExpected
+                    : FileCheckDiag::MatchTypeCount,
+      SM, Loc, Pat.getCheckTy(), Buffer, 0, Buffer.size(), Diags);
+  SM.PrintMessage(SearchRange.Start, SourceMgr::DK_Note, "scanning from here");
 
   // Allow the pattern to print additional information if desired.
   Pat.PrintVariableUses(SM, Buffer, VariableTable);
   if (ExpectedMatch)
     Pat.PrintFuzzyMatch(SM, Buffer, VariableTable);
 }
 
 static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          const FileCheckString &CheckStr, int MatchedCount,
                          StringRef Buffer, StringMap<StringRef> &VariableTable,
-                         bool VerboseVerbose) {
+                         bool VerboseVerbose,
+                         std::vector<FileCheckDiag> *Diags) {
   PrintNoMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
-               MatchedCount, Buffer, VariableTable, VerboseVerbose);
+               MatchedCount, Buffer, VariableTable, VerboseVerbose, Diags);
 }
 
 /// Count the number of newlines in the specified range.
 static unsigned CountNumNewlinesBetween(StringRef Range,
                                         const char *&FirstNewLine) {
   unsigned NumNewLines = 0;
   while (1) {
     // Scan for newline.
while (1) {
 
     if (NumNewLines == 1)
       FirstNewLine = Range.begin();
   }
 }
 
 /// Match check string and its "not strings" and/or "dag strings".
 size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
                               bool IsLabelScanMode, size_t &MatchLen,
                               StringMap<StringRef> &VariableTable,
-                          FileCheckRequest &Req) const {
+                              FileCheckRequest &Req,
+                              std::vector<FileCheckDiag> *Diags) const {
   size_t LastPos = 0;
   std::vector<const FileCheckPattern *> NotStrings;
 
   // IsLabelScanMode is true when we are scanning forward to find CHECK-LABEL
   // bounds; we have not processed variable definitions within the bounded block
   // yet so cannot handle any final CHECK-DAG yet; this is handled when going
   // over the block again (including the last CHECK-LABEL) in normal mode.
   if (!IsLabelScanMode) {
     // Match "dag strings" (with mixed "not strings" if any).
-    LastPos = CheckDag(SM, Buffer, NotStrings, VariableTable, Req);
+    LastPos = CheckDag(SM, Buffer, NotStrings, VariableTable, Req, Diags);
     if (LastPos == StringRef::npos)
       return StringRef::npos;
   }
 
   // Match itself from the last position after matching CHECK-DAG.
   size_t LastMatchEnd = LastPos;
   size_t FirstMatchPos = 0;
   // Go match the pattern Count times. Majority of patterns only match with
   // count 1 though.
   assert(Pat.getCount() != 0 && "pattern count can not be zero");
   for (int i = 1; i <= Pat.getCount(); i++) {
     StringRef MatchBuffer = Buffer.substr(LastMatchEnd);
     size_t CurrentMatchLen;
     // get a match at current start point
     size_t MatchPos = Pat.Match(MatchBuffer, CurrentMatchLen, VariableTable);
     if (i == 1)
       FirstMatchPos = LastPos + MatchPos;
 
     // report
     if (MatchPos == StringRef::npos) {
       PrintNoMatch(true, SM, *this, i, MatchBuffer, VariableTable,
-                   Req.VerboseVerbose);
+                   Req.VerboseVerbose, Diags);
       return StringRef::npos;
     }
     PrintMatch(true, SM, *this, i, MatchBuffer, VariableTable, MatchPos,
                CurrentMatchLen, Req);
 
     // move start point after the match
     LastMatchEnd += MatchPos + CurrentMatchLen;
   }
if (!IsLabelScanMode) {
     // the same line (i.e. that there is no newline between them).
     if (CheckSame(SM, SkippedRegion))
       return StringRef::npos;
 
     // If this match had "not strings", verify that they don't exist in the
     // skipped region.
     if (CheckNot(SM, SkippedRegion, NotStrings, VariableTable, Req))
       return StringRef::npos;
   }
 
   return FirstMatchPos;
 }
 
 /// Verify there is a single line in the given buffer.
 bool FileCheckString::CheckNext(const SourceMgr &SM, StringRef Buffer) const {
   if (Pat.getCheckTy() != Check::CheckNext &&
       Pat.getCheckTy() != Check::CheckEmpty)
bool FileCheckString::CheckNot(const SourceMgr &SM, StringRef Buffer,
   for (const FileCheckPattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
 
     size_t MatchLen = 0;
     size_t Pos = Pat->Match(Buffer, MatchLen, VariableTable);
 
     if (Pos == StringRef::npos) {
       PrintNoMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer,
-                   VariableTable, Req.VerboseVerbose);
+                   VariableTable, Req.VerboseVerbose, nullptr);
       continue;
     }
 
     PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, VariableTable,
                Pos, MatchLen, Req);
 
     return true;
   }
 
   return false;
 }
 
 /// Match "dag strings" and their mixed "not strings".
-size_t FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
-                             std::vector<const FileCheckPattern *> &NotStrings,
-                             StringMap<StringRef> &VariableTable,
-                             const FileCheckRequest &Req) const {
+size_t
+FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
+                          const FileCheckRequest &Req,
+                          std::vector<FileCheckDiag> *Diags) const {
   if (DagNotStrings.empty())
     return 0;
 
   // The start of the search range.
   size_t StartPos = 0;
 
   struct MatchRange {
     size_t Pos;
for (auto PatItr = DagNotStrings.begin(), PatEnd = DagNotStrings.end();
     // CHECK-DAG group.
     for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
       StringRef MatchBuffer = Buffer.substr(MatchPos);
       size_t MatchPosBuf = Pat.Match(MatchBuffer, MatchLen, VariableTable);
       // With a group of CHECK-DAGs, a single mismatching means the match on
       // that group of CHECK-DAGs fails immediately.
       if (MatchPosBuf == StringRef::npos) {
         PrintNoMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, MatchBuffer,
-                     VariableTable, Req.VerboseVerbose);
+                     VariableTable, Req.VerboseVerbose, Diags);
         return StringRef::npos;
       }
       // Re-calc it as the offset relative to the start of the original string.
       MatchPos += MatchPosBuf;
       if (Req.VerboseVerbose)
         PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer,
                    VariableTable, MatchPos, MatchLen, Req);
       MatchRange M{MatchPos, MatchPos + MatchLen};
for (const auto &Var : LocalVars)
     VariableTable.erase(Var);
 }
 
 /// Check the input to FileCheck provided in the \p Buffer against the \p
 /// CheckStrings read from the check file.
 ///
 /// Returns false if the input fails to satisfy the checks.
 bool llvm::FileCheck::CheckInput(SourceMgr &SM, StringRef Buffer,
-                ArrayRef<FileCheckString> CheckStrings) {
+                                 ArrayRef<FileCheckString> CheckStrings,
+                                 std::vector<FileCheckDiag> *Diags) {
   bool ChecksFailed = false;
 
   /// VariableTable - This holds all the current filecheck variables.
   StringMap<StringRef> VariableTable;
 
   for (const auto& Def : Req.GlobalDefines)
     VariableTable.insert(StringRef(Def).split('='));
 
   unsigned i = 0, j = 0, e = CheckStrings.size();
   while (true) {
     StringRef CheckRegion;
     if (j == e) {
       CheckRegion = Buffer;
     } else {
       const FileCheckString &CheckLabelStr = CheckStrings[j];
       if (CheckLabelStr.Pat.getCheckTy() != Check::CheckLabel) {
         ++j;
         continue;
       }
 
       // Scan to next CHECK-LABEL match, ignoring CHECK-NOT and CHECK-DAG
       size_t MatchLabelLen = 0;
-      size_t MatchLabelPos =
-          CheckLabelStr.Check(SM, Buffer, true, MatchLabelLen, VariableTable,
-                              Req);
+      size_t MatchLabelPos = CheckLabelStr.Check(
+          SM, Buffer, true, MatchLabelLen, VariableTable, Req, Diags);
       if (MatchLabelPos == StringRef::npos)
         // Immediately bail of CHECK-LABEL fails, nothing else we can do.
         return false;
 
       CheckRegion = Buffer.substr(0, MatchLabelPos + MatchLabelLen);
       Buffer = Buffer.substr(MatchLabelPos + MatchLabelLen);
       ++j;
     }
 
     if (Req.EnableVarScope)
       ClearLocalVars(VariableTable);
 
     for (; i != j; ++i) {
       const FileCheckString &CheckStr = CheckStrings[i];
 
       // Check each string within the scanned region, including a second check
       // of any final CHECK-LABEL (to verify CHECK-NOT and CHECK-DAG)
       size_t MatchLen = 0;
-      size_t MatchPos =
-          CheckStr.Check(SM, CheckRegion, false, MatchLen, VariableTable, Req);
+      size_t MatchPos = CheckStr.Check(SM, CheckRegion, false, MatchLen,
+                                       VariableTable, Req, Diags);
 
       if (MatchPos == StringRef::npos) {
         ChecksFailed = true;
         i = j;
         break;
       }
 
       CheckRegion = CheckRegion.substr(MatchPos + MatchLen);

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

+;--------------------------------------------------
+; Use -strict-whitespace to check marker alignment here.
+; (Also check multiline marker where start/end columns vary across lines.)
+;
+; In the remaining checks, don't use -strict-whitespace and thus check just the
+; presence, order, and lengths of markers.  That way, if we ever change padding
+; within line labels, we don't have to adjust so many tests.
+;--------------------------------------------------
+
+; RUN: echo 'hello world' > %t.in
+; RUN: echo 'goodbye' >> %t.in
+; RUN: echo 'world' >> %t.in
+
+; RUN: echo 'CHECK: hello' > %t.chk
+; RUN: echo 'CHECK: universe' >> %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -strict-whitespace -match-full-lines -check-prefix=ALIGN %s
+
+; ALIGN:Full input was:
+; ALIGN-NEXT:<<<<<<
+; ALIGN-NEXT:         1: hello world
+; ALIGN-NEXT:check:2           X~~~~
+; ALIGN-NEXT:         2: goodbye
+; ALIGN-NEXT:check:2     ~~~~~~~
+; ALIGN-NEXT:         3: world
+; ALIGN-NEXT:check:2     ~~~~~ error: no match found
+; ALIGN-NEXT:>>>>>>
+; ALIGN-NOT:{{.}}
+
+;--------------------------------------------------
+; CHECK (also: multi-line search range)
+;--------------------------------------------------
+
+; Good match and no match.
+
+; RUN: echo 'hello'   > %t.in
+; RUN: echo 'again'   >> %t.in
+; RUN: echo 'whirled' >> %t.in
+
+; RUN: echo 'CHECK: hello' > %t.chk
+; RUN: echo 'CHECK: world' >> %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefix=CHK
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=CHK,CHK-V
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=CHK,CHK-V
+
+; CHK:        <<<<<<
+; CHK-NEXT:            1: hello
+; CHK-NEXT:            2: again
+; CHK-NEXT:   check:2     X~~~~
+; CHK-NEXT:            3: whirled
+; CHK-NEXT:   check:2     ~~~~~~~ error: no match found
+; CHK-NEXT:   >>>>>>
+; CHK-NOT:    {{.}}
+
+;--------------------------------------------------
+; CHECK-COUNT-<num>
+;--------------------------------------------------
+
+; Good match and no match.
+
+; RUN: echo 'pete'   > %t.in
+; RUN: echo 'repete' >> %t.in
+; RUN: echo 'repeat' >> %t.in
+
+; RUN: echo 'CHECK-COUNT-3: pete' > %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefix=CNT
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=CNT,CNT-V
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=CNT,CNT-V
+
+; CNT:        <<<<<<
+; CNT-NEXT:            1: pete
+; CNT-NEXT:            2: repete
+; CNT-NEXT:            3: repeat
+; CNT-NEXT:   count:1     X~~~~~ error: no match found
+; CNT-NEXT:   >>>>>>
+; CNT-NOT:    {{.}}
+
+;--------------------------------------------------
+; CHECK-NEXT (also: EOF search-range)
+;--------------------------------------------------
+
+; Good match and no match.
+
+; RUN: echo 'hello' > %t.in
+; RUN: echo 'again' >> %t.in
+
+; RUN: echo 'CHECK: hello' > %t.chk
+; RUN: echo 'CHECK-NEXT: again' >> %t.chk
+; RUN: echo 'CHECK-NEXT: world' >> %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefix=NXT
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=NXT,NXT-V
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=NXT,NXT-V,NXT-VV
+
+; NXT:        <<<<<<
+; NXT-NEXT:           1: hello
+; NXT-NEXT:           2: again
+; NXT-NEXT:           3:
+; NXT-NEXT:   next:3     X error: no match found
+; NXT-NEXT:   >>>>>>
+; NXT-NOT:    {{.}}
+
+;--------------------------------------------------
+; CHECK-SAME (also: single-char search range)
+;--------------------------------------------------
+
+; Good match and no match.
+
+; RUN: echo 'hello world!' > %t.in
+
+; RUN: echo 'CHECK: hello' > %t.chk
+; RUN: echo 'CHECK-SAME: world' >> %t.chk
+; RUN: echo 'CHECK-SAME: again' >> %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefix=SAM
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=SAM,SAM-V
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=SAM,SAM-V,SAM-VV
+
+; SAM:        <<<<<<
+; SAM-NEXT:           1: hello world!
+; SAM-NEXT:   same:3                X error: no match found
+; SAM-NEXT:   >>>>>>
+; SAM-NOT:    {{.}}
+
+;--------------------------------------------------
+; CHECK-EMPTY (also: search range ends at label)
+;--------------------------------------------------
+
+; Good match and no match.
+;
+; CHECK-EMPTY always seems to match an empty line at EOF (illegally when it's
+; not the next line) unless either (1) the last line is non-empty and has no
+; newline or (2) there's a CHECK-LABEL to end the search range before EOF.  We
+; choose scenario 2 to check the case of no match.
+
+; RUN: echo 'hello' > %t.in
+; RUN: echo '' >> %t.in
+; RUN: echo 'world' >> %t.in
+; RUN: echo 'label' >> %t.in
+
+; RUN: echo 'CHECK: hello' > %t.chk
+; RUN: echo 'CHECK-EMPTY:' >> %t.chk
+; RUN: echo 'CHECK-EMPTY:' >> %t.chk
+; RUN: echo 'CHECK-LABEL: label' >> %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefix=EMP
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=EMP,EMP-V
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=EMP,EMP-V,EMP-VV
+
+; EMP:        <<<<<<
+; EMP-NEXT:            1: hello
+; EMP-NEXT:            2:
+; EMP-NEXT:            3: world
+; EMP-NEXT:   empty:3     X~~~~
+; EMP-NEXT:            4: label
+; EMP-NEXT:   empty:3     ~~~~~ error: no match found
+; EMP-NEXT:   >>>>>>
+; EMP-NOT:    {{.}}
+
+;--------------------------------------------------
+; CHECK-DAG
+;--------------------------------------------------
+
+; 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
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG
+; 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:         <<<<<<
+; DAG-NEXT:           1: abc
+; DAG-NEXT:           2: def
+; DAG-NEXT:           3: abc
+; DAG-NEXT:    dag:4     X~~ error: no match found
+; DAG-NEXT:    >>>>>>
+; DAG-NOT:     {{.}}
+
+;--------------------------------------------------
+; CHECK-LABEL
+;--------------------------------------------------
+
+; Good match and no match.
+
+; RUN: echo 'lab0' > %t.in
+; RUN: echo 'foo' >> %t.in
+; RUN: echo 'lab1' >> %t.in
+; RUN: echo 'bar' >> %t.in
+
+; RUN: echo 'CHECK-LABEL: lab0' > %t.chk
+; RUN: echo 'CHECK: foo' >> %t.chk
+; RUN: echo 'CHECK-LABEL: lab2' >> %t.chk
+
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=LAB
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=LAB,LAB-V
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=LAB,LAB-V,LAB-VV
+
+; LAB:         <<<<<<
+; LAB-NEXT:             1: lab0
+; LAB-NEXT:             2: foo
+; LAB-NEXT:    label:3     X~~
+; LAB-NEXT:             3: lab1
+; LAB-NEXT:    label:3     ~~~~
+; LAB-NEXT:             4: bar
+; LAB-NEXT:    label:3     ~~~ error: no match found
+; LAB-NEXT:    >>>>>>
+; LAB-NOT:     {{.}}
+
+

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

+; RUN: echo ciao > %t.good
+; RUN: echo world >> %t.good
+
+; RUN: echo hello > %t.err
+; RUN: echo world >> %t.err
+
+; RUN: echo 'CHECK: ciao' > %t.check
+; RUN: echo 'CHECK-NEXT: world' >> %t.check
+
+;--------------------------------------------------
+; unknown value
+;--------------------------------------------------
+
+; RUN: not FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines -dump-input=foobar 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL
+
+; No positional arg.
+; RUN: not FileCheck -dump-input=foobar 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL
+
+BADVAL: FileCheck: for the -dump-input option: Cannot find option named 'foobar'!
+
+;--------------------------------------------------
+; help
+;--------------------------------------------------
+
+; Appended to normal command line.
+; RUN: FileCheck -input-file %t.err -color %t.check -dump-input=help \
+; RUN: | FileCheck %s -check-prefix=HELP
+
+; No positional arg.
+; RUN: FileCheck -dump-input=help | FileCheck %s -check-prefix=HELP
+
+HELP-NOT: {{.}}
+HELP: The following description was requested by -dump-input=help
+HELP: try{{.*}}-color
+HELP-NOT: {{.}}
+
+;--------------------------------------------------
+; never
+;--------------------------------------------------
+
+; RUN: FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:           -match-full-lines -dump-input=never 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP -allow-empty
+
+; RUN: not FileCheck -input-file %t.err %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines -dump-input=never 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP
+
+;--------------------------------------------------
+; default: never
+;--------------------------------------------------
+
+; RUN: FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:           -match-full-lines 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP -allow-empty
+
+; RUN: not FileCheck -input-file %t.err %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP
+
+;--------------------------------------------------
+; fail
+;--------------------------------------------------
+
+; RUN: FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:           -match-full-lines -dump-input=fail 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP -allow-empty
+
+; RUN: not FileCheck -input-file %t.err %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines -dump-input=fail 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ERR
+
+;--------------------------------------------------
+; -dump-input-on-failure
+;--------------------------------------------------
+
+; RUN: FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:           -match-full-lines -dump-input-on-failure 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP -allow-empty
+
+; RUN: not FileCheck -input-file %t.err %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines -dump-input-on-failure 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ERR
+
+; RUN: env FILECHECK_DUMP_INPUT_ON_FAILURE=1 \
+; RUN: FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:           -match-full-lines 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-NODUMP -allow-empty
+
+; RUN: env FILECHECK_DUMP_INPUT_ON_FAILURE=1 \
+; RUN: not FileCheck -input-file %t.err %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ERR
+
+;--------------------------------------------------
+; always
+;--------------------------------------------------
+
+; RUN: FileCheck -input-file %t.good %t.check -check-prefix=CHECK \
+; RUN:           -match-full-lines -dump-input=always -v 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-GOOD
+
+; RUN: not FileCheck -input-file %t.err %t.check -check-prefix=CHECK \
+; RUN:               -match-full-lines -dump-input=always 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ERR
+
+; END.
+
+; CHECK-GOOD: Full input was:
+; CHECK-GOOD-NEXT: <<<<<<
+; CHECK-GOOD-NEXT:          1: ciao
+; CHECK-GOOD-NEXT:          2: world
+; CHECK-GOOD-NEXT: >>>>>>
+
+; CHECK-ERR: Full input was:
+; CHECK-ERR-NEXT: <<<<<<
+; CHECK-ERR-NEXT:          1: hello
+; CHECK-ERR-NEXT: check:1     X~~~~
+; CHECK-ERR-NEXT:          2: world
+; CHECK-ERR-NEXT: check:1     ~~~~~ error: no match found
+; CHECK-ERR-NEXT: >>>>>>
+
+; CHECK-NODUMP-NOT: <<<<<<

llvm/test/FileCheck/no-check-file.txt

+; RUN: not FileCheck 2>&1 | FileCheck %s
+
+CHECK: <check-file> not specified

llvm/test/FileCheck/verbose_mode.txt

-; RUN: not FileCheck -input-file %s %s --check-prefix=CHECK1 --match-full-lines --dump-input-on-failure 2>&1 | FileCheck %s --check-prefix=CHECKERROR --match-full-lines
-; RUN: env FILECHECK_DUMP_INPUT_ON_FAILURE=1 not FileCheck -input-file %s %s --check-prefix=CHECK1 --match-full-lines 2>&1 |  FileCheck %s --check-prefix=CHECKERROR --match-full-lines
-; RUN: env FILECHECK_DUMP_INPUT_ON_FAILURE=1 not FileCheck -input-file %s %s --check-prefix=CHECK1 --match-full-lines --dump-input-on-failure=0 2>&1 |  FileCheck %s --check-prefix=CHECKERRORNOVERBOSE --match-full-lines
-
-hello
-world
-
-; CHECK1: ciao
-; CHECK1-NEXT: world
-
-; CHECKERROR: Full input was:
-; CHECKERROR-NEXT: <<<<<<
-; CHECKERROR: hello
-; CHECKERROR: world
-; CHECKERROR: >>>>>>
-
-; CHECKERRORNOVERBOSE-NOT: <<<<<<

llvm/utils/FileCheck/FileCheck.cpp

 // the file matched the expected contents, and exit status of 1 if it did not
 // contain the expected contents.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/FileCheck.h"
 using namespace llvm;
 
 static cl::opt<std::string>
-    CheckFilename(cl::Positional, cl::desc("<check-file>"), cl::Required);
+    CheckFilename(cl::Positional, cl::desc("<check-file>"), cl::Optional);
 
 static cl::opt<std::string>
     InputFilename("input-file", cl::desc("File to check (defaults to stdin)"),
                   cl::init("-"), cl::value_desc("filename"));
 
 static cl::list<std::string> CheckPrefixes(
     "check-prefix",
     cl::desc("Prefix to use from check file (defaults to 'CHECK')"));
static cl::opt<bool> VerboseVerbose(
     cl::desc("Print information helpful in diagnosing internal FileCheck\n"
              "issues.  Implies -v.\n"));
 static const char * DumpInputEnv = "FILECHECK_DUMP_INPUT_ON_FAILURE";
 
 static cl::opt<bool> DumpInputOnFailure(
     "dump-input-on-failure", cl::init(std::getenv(DumpInputEnv)),
     cl::desc("Dump original input to stderr before failing.\n"
              "The value can be also controlled using\n"
-             "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n"));
+             "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n"
+             "This option is deprecated in favor of -dump-input=fail.\n"));
+
+enum DumpInputValue {
+  DumpInputDefault,
+  DumpInputHelp,
+  DumpInputNever,
+  DumpInputFail,
+  DumpInputAlways
+};
+
+static cl::opt<DumpInputValue> DumpInput(
+    "dump-input", cl::init(DumpInputDefault),
+    cl::desc("Dump input to stderr, adding annotations representing\n"
+             " currently enabled diagnostics\n"),
+    cl::value_desc("mode"),
+    cl::values(clEnumValN(DumpInputHelp, "help",
+                          "Explain dump format and quit"),
+               clEnumValN(DumpInputNever, "never", "Never dump input"),
+               clEnumValN(DumpInputFail, "fail", "Dump input on failure"),
+               clEnumValN(DumpInputAlways, "always", "Always dump input")));
 
 typedef cl::list<std::string>::const_iterator prefix_iterator;
 
 
 
 
 
 
 
 static void DumpCommandLine(int argc, char **argv) {
   errs() << "FileCheck command line: ";
   for (int I = 0; I < argc; I++)
     errs() << " " << argv[I];
   errs() << "\n";
 }
 
+struct MarkerStyle {
+  /// The starting char (before tildes) for marking the line.
+  char Lead;
+  /// What color to use for this annotation.
+  raw_ostream::Colors Color;
+  /// A note to follow the marker, or empty string if none.
+  std::string Note;
+  MarkerStyle() {}
+  MarkerStyle(char Lead, raw_ostream::Colors Color, const std::string &Note)
+      : Lead(Lead), Color(Color), Note(Note) {}
+};
+
+static MarkerStyle GetMarker(FileCheckDiag::MatchType MatchTy) {
+  switch (MatchTy) {
+  case FileCheckDiag::MatchNoneButExpected:
+    return MarkerStyle('X', raw_ostream::RED, "error: no match found");
+  case FileCheckDiag::MatchTypeCount:
+    llvm_unreachable_internal("unexpected match type");
+  }
+  llvm_unreachable_internal("unexpected match type");
+}
+
+static void DumpInputAnnotationHelp(raw_ostream &OS) {
+  OS << "The following description was requested by -dump-input=help to\n"
+     << "explain the input annotations printed by -dump-input=always and\n"
+     << "-dump-input=fail:\n\n";
+
+  // Labels for input lines.
+  OS << "  - ";
+  WithColor(OS, raw_ostream::SAVEDCOLOR, true) << "L:";
+  OS << "     labels line number L of the input file\n";
+
+  // Labels for annotation lines.
+  OS << "  - ";
+  WithColor(OS, raw_ostream::SAVEDCOLOR, true) << "T:L";
+  OS << "    labels the match result for a pattern of type T from "
+     << "line L of\n"
+     << "           the check file\n";
+
+  // Markers on annotation lines.
+  OS << "  - ";
+  WithColor(OS, raw_ostream::SAVEDCOLOR, true) << "X~~";
+  OS << "    marks search range when no match is found\n";
+
+  // Colors.
+  OS << "  - colors ";
+  WithColor(OS, raw_ostream::RED, true) << "error";
+  OS << "\n\n"
+     << "If you are not seeing color above or in input dumps, try: -color\n";
+}
+
+/// An annotation for a single input line.
+struct InputAnnotation {
+  /// The check file line (one-origin indexing) where the directive that
+  /// produced this annotation is located.
+  unsigned CheckLine;
+  /// The label for this annotation.
+  std::string Label;
+  /// What input line (one-origin indexing) this annotation marks.  This might
+  /// be different from the starting line of the original diagnostic if this is
+  /// a non-initial fragment of a diagnostic that has been broken across
+  /// multiple lines.
+  unsigned InputLine;
+  /// The column range (one-origin indexing, open end) in which to to mark the
+  /// input line.  If InputEndCol is UINT_MAX, treat it as the last column
+  /// before the newline.
+  unsigned InputStartCol, InputEndCol;
+  /// The marker to use.
+  MarkerStyle Marker;
+};
+
+/// Get an abbreviation for the check type.
+std::string GetCheckTypeAbbreviation(Check::FileCheckType Ty) {
+  switch (Ty) {
+  case Check::CheckPlain:
+    if (Ty.getCount() > 1)
+      return "count";
+    return "check";
+  case Check::CheckNext:
+    return "next";
+  case Check::CheckSame:
+    return "same";
+  case Check::CheckNot:
+    return "not";
+  case Check::CheckDAG:
+    return "dag";
+  case Check::CheckLabel:
+    return "label";
+  case Check::CheckEmpty:
+    return "empty";
+  case Check::CheckEOF:
+    return "eof";
+  case Check::CheckBadNot:
+    return "bad-not";
+  case Check::CheckBadCount:
+    return "bad-count";
+  case Check::CheckNone:
+    llvm_unreachable("invalid FileCheckType");
+  }
+  llvm_unreachable("unknown FileCheckType");
+}
+
+static void BuildInputAnnotations(const std::vector<FileCheckDiag> &Diags,
+                                  std::vector<InputAnnotation> &Annotations,
+                                  unsigned &LabelWidth) {
+  // What's the widest label?
+  LabelWidth = 0;
+  for (auto DiagItr = Diags.begin(), DiagEnd = Diags.end(); DiagItr != DiagEnd;
+       ++DiagItr) {
+    InputAnnotation A;
+
+    // Build label, which uniquely identifies this check result.
+    A.CheckLine = DiagItr->CheckLine;
+    llvm::raw_string_ostream Label(A.Label);
+    Label << GetCheckTypeAbbreviation(DiagItr->CheckTy) << ":"
+          << DiagItr->CheckLine;
+    Label.flush();
+    LabelWidth = std::max((std::string::size_type)LabelWidth, A.Label.size());
+
+    MarkerStyle Marker = GetMarker(DiagItr->MatchTy);
+    A.Marker = Marker;
+
+    // Compute the mark location, and break annotation into multiple
+    // annotations if it spans multiple lines.
+    A.InputLine = DiagItr->InputStartLine;
+    A.InputStartCol = DiagItr->InputStartCol;
+    if (DiagItr->InputStartLine == DiagItr->InputEndLine) {
+      // Sometimes ranges are empty in order to indicate a specific point, but
+      // that would mean nothing would be marked, so adjust the range to
+      // include the following character.
+      A.InputEndCol =
+          std::max(DiagItr->InputStartCol + 1, DiagItr->InputEndCol);
+      Annotations.push_back(A);
+    } else {
+      assert(DiagItr->InputStartLine < DiagItr->InputEndLine &&
+             "expected input range not to be inverted");
+      A.InputEndCol = UINT_MAX;
+      A.Marker.Note = "";
+      Annotations.push_back(A);
+      for (unsigned L = DiagItr->InputStartLine + 1, E = DiagItr->InputEndLine;
+           L <= E; ++L) {
+        // If a range ends before the first column on a line, then it has no
+        // characters on that line, so there's nothing to render.
+        if (DiagItr->InputEndCol == 1 && L == E) {
+          Annotations.back().Marker.Note = Marker.Note;
+          break;
+        }
+        InputAnnotation B;
+        B.CheckLine = A.CheckLine;
+        B.Label = A.Label;
+        B.InputLine = L;
+        B.Marker = Marker;
+        B.Marker.Lead = '~';
+        B.InputStartCol = 1;
+        if (L != E) {
+          B.InputEndCol = UINT_MAX;
+          B.Marker.Note = "";
+        } else
+          B.InputEndCol = DiagItr->InputEndCol;
+        Annotations.push_back(B);
+      }
+    }
+  }
+}
+
+static void DumpAnnotatedInput(
+    raw_ostream &OS, StringRef InputFileText,
+    std::vector<InputAnnotation> &Annotations, unsigned LabelWidth) {
+  OS << "Full input was:\n<<<<<<\n";
+
+  // Sort annotations.
+  //
+  // First, sort in the order of input lines to make it easier to find relevant
+  // annotations while iterating input lines in the implementation below.
+  // FileCheck diagnostics are not always reported and recorded in the order of
+  // input lines due to, for example, CHECK-DAG and CHECK-NOT.
+  //
+  // Second, for annotations for the same input line, sort in the order of the
+  // FileCheck directive's line in the check file (where there's at most one
+  // directive per line).  The rationale of this choice is that, for any input
+  // line, this sort establishes a total order of annotations that, with
+  // respect to match results, is consistent across multiple lines, thus
+  // making match results easier to track from one line to the next when they
+  // span multiple lines.
+  std::sort(Annotations.begin(), Annotations.end(),
+            [](const InputAnnotation &A, const InputAnnotation &B) {
+              if (A.InputLine != B.InputLine)
+                return A.InputLine < B.InputLine;
+              return A.CheckLine < B.CheckLine;
+            });
+
+  // Compute the width of the label column.
+  const unsigned char *InputFilePtr = InputFileText.bytes_begin(),
+                      *InputFileEnd = InputFileText.bytes_end();
+  unsigned LineCount = InputFileText.count('\n');
+  if (InputFileEnd[-1] != '\n')
+    ++LineCount;
+  unsigned LineNoWidth = log10(LineCount) + 1;
+  // +3 below adds spaces (1) to the left of the (right-aligned) line numbers
+  // on input lines and (2) to the right of the (left-aligned) labels on
+  // annotation lines so that input lines and annotation lines are more
+  // visually distinct.  For example, the spaces on the annotation lines ensure
+  // that input line numbers and check directive line numbers never align
+  // horizontally.  Those line numbers might not even be for the same file.
+  // One space would be enough to achieve that, but more makes it even easier
+  // to see.
+  LabelWidth = std::max(LabelWidth, LineNoWidth) + 3;
+
+  // Print annotated input lines.
+  auto AnnotationItr = Annotations.begin(), AnnotationEnd = Annotations.end();
+  for (unsigned Line = 1;
+       InputFilePtr != InputFileEnd || AnnotationItr != AnnotationEnd;
+       ++Line) {
+    const unsigned char *InputFileLine = InputFilePtr;
+
+    // Print right-aligned line number.
+    WithColor(OS, raw_ostream::BLACK, true)
+        << format_decimal(Line, LabelWidth) << ": ";
+
+    // Print numbered line.
+    bool Newline = false;
+    while (InputFilePtr != InputFileEnd && !Newline) {
+      if (*InputFilePtr == '\n')
+        Newline = true;
+      else
+        OS << *InputFilePtr;
+      ++InputFilePtr;
+    }
+    OS << '\n';
+    unsigned InputLineWidth = InputFilePtr - InputFileLine - Newline;
+
+    // Print any annotations.
+    while (AnnotationItr != AnnotationEnd &&
+           AnnotationItr->InputLine == Line) {
+      WithColor COS(OS, AnnotationItr->Marker.Color, true);
+      // The two spaces below are where the ": " appears on input lines.
+      COS << left_justify(AnnotationItr->Label, LabelWidth) << "  ";
+      unsigned Col;
+      for (Col = 1; Col < AnnotationItr->InputStartCol; ++Col)
+        COS << ' ';
+      COS << AnnotationItr->Marker.Lead;
+      // If InputEndCol=UINT_MAX, stop at InputLineWidth.
+      for (++Col; Col < AnnotationItr->InputEndCol && Col <= InputLineWidth;
+           ++Col)
+        COS << '~';
+      const std::string &Note = AnnotationItr->Marker.Note;
+      if (!Note.empty()) {
+        // Put the note at the end of the input line.  If we were to instead
+        // put the note right after the marker, subsequent annotations for the
+        // same input line might appear to mark this note instead of the input
+        // line.
+        for (; Col <= InputLineWidth; ++Col)
+          COS << ' ';
+        COS << ' ' << Note;
+      }
+      COS << '\n';
+      ++AnnotationItr;
+    }
+  }
+
+  OS << ">>>>>>\n";
+}
+
 int main(int argc, char **argv) {
   // Enable use of ANSI color codes because FileCheck is using them to
   // highlight text.
   llvm::sys::Process::UseANSIEscapeCodes(true);
 
   InitLLVM X(argc, argv);
   cl::ParseCommandLineOptions(argc, argv, /*Overview*/ "", /*Errs*/ nullptr,
                               "FILECHECK_OPTS");
+  if (DumpInput == DumpInputHelp) {
+    DumpInputAnnotationHelp(outs());
+    return 0;
+  }
+  if (CheckFilename.empty()) {
+    errs() << "<check-file> not specified\n";
+    return 2;
+  }
 
   FileCheckRequest Req;
   for (auto Prefix : CheckPrefixes)
     Req.CheckPrefixes.push_back(Prefix);
 
   for (auto CheckNot : ImplicitCheckNot)
     Req.ImplicitCheckNot.push_back(CheckNot);
 
if (!PrefixRE.isValid(REError)) {
     errs() << "Unable to combine check-prefix strings into a prefix regular "
               "expression! This is likely a bug in FileCheck's verification of "
               "the check-prefix strings. Regular expression parsing failed "
               "with the following error: "
            << REError << "\n";
     return 2;
   }
 
-
   SourceMgr SM;
 
   // Read the expected strings from the check file.
   ErrorOr<std::unique_ptr<MemoryBuffer>> CheckFileOrErr =
       MemoryBuffer::getFileOrSTDIN(CheckFilename);
   if (std::error_code EC = CheckFileOrErr.getError()) {
     errs() << "Could not open check file '" << CheckFilename
            << "': " << EC.message() << '\n';
int main(int argc, char **argv) {
 
   SmallString<4096> InputFileBuffer;
   StringRef InputFileText = FC.CanonicalizeFile(InputFile, InputFileBuffer);
 
   SM.AddNewSourceBuffer(MemoryBuffer::getMemBuffer(
                             InputFileText, InputFile.getBufferIdentifier()),
                         SMLoc());
 
-  int ExitCode =
-      FC.CheckInput(SM, InputFileText, CheckStrings) ? EXIT_SUCCESS : 1;
-  if (ExitCode == 1 && DumpInputOnFailure)
-    errs() << "Full input was:\n<<<<<<\n" << InputFileText << "\n>>>>>>\n";
+  if (DumpInput == DumpInputDefault)
+    DumpInput = DumpInputOnFailure ? DumpInputFail : DumpInputNever;
+
+  std::vector<FileCheckDiag> Diags;
+  int ExitCode = FC.CheckInput(SM, InputFileText, CheckStrings,
+                               DumpInput == DumpInputNever ? nullptr : &Diags)
+                     ? EXIT_SUCCESS
+                     : 1;
+  if (DumpInput == DumpInputAlways ||
+      (ExitCode == 1 && DumpInput == DumpInputFail)) {
+    errs() << "\n"
+           << "Input file: "
+           << (InputFilename == "-" ? "<stdin>" : InputFilename.getValue())
+           << "\n"
+           << "Check file: " << CheckFilename << "\n"
+           << "\n"
+           << "-dump-input=help describes the format of the following dump.\n"
+           << "\n";
+    std::vector<InputAnnotation> Annotations;
+    unsigned LabelWidth;
+    BuildInputAnnotations(Diags, Annotations, LabelWidth);
+    DumpAnnotatedInput(errs(), InputFileText, Annotations, LabelWidth);
+  }
 
   return ExitCode;
 }