[FileCheck] Implement -v and -vv for tracing matches
ClosedPublic

Authored by jdenny on May 19 2018, 4:57 PM.

Details

Summary

-v prints all directive pattern matches.

-vv additionally prints info that might be noise to users but that can
be helpful to FileCheck developers.

To maximize code reuse and to make diagnostics more consistent, this
patch also adjusts and extends some of the existing diagnostics.
CHECK-NOT failures now report variables uses. Many more diagnostics
now report the check prefix and kind of directive.

Diff Detail

Repository
rL LLVM

The code changes look pretty tidy. Let's sort out the -vv description and then we can call it good.

docs/CommandGuide/FileCheck.rst
107 ↗(On Diff #147689)

I might not get so specific with the list of what gets printed; I'd prefer a broader statement of intent, maybe with specifics as examples. Something like:

Print information helpful in diagnosing internal FileCheck issues; for example, discarded ...

utils/FileCheck/FileCheck.cpp
98 ↗(On Diff #147689)

Again, getting too specific may become a maintenance issue. The general motivation appears to be along the lines of "report things that FileCheck does or does not match, but which normally would not result in any output."

jdenny added inline comments.May 22 2018, 2:10 PM
utils/FileCheck/FileCheck.cpp
98 ↗(On Diff #147689)

Sure. Two questions:

  1. If I move each comment to the relevant VerboseVerbose check, that should eliminate the maintenance burden. That would allow us to remember the rationale behind how I divided things between -v and -vv. Then again, the rationale is now captured in phabricator, so I can just toss the comments if you prefer. What do you think?
  1. Is my understanding of the EOF pattern correct? Specifically, is there any reason why the EOF pattern is needed after a DAG group? Is that just an implementation convenience because DAGs and NOTs are stored together?

Thanks.

probinson added inline comments.May 23 2018, 8:15 AM
utils/FileCheck/FileCheck.cpp
98 ↗(On Diff #147689)
  1. Sounds great.
  1. DAG/NOT ranges are delimited by the surrounding match points; the EOF pattern is an implementation convenience to create an artificial match point at EOF, when the last directive in the file is DAG/NOT. It just simplifies handling the ranges.
jdenny updated this revision to Diff 148239.May 23 2018, 10:11 AM
jdenny marked 4 inline comments as done.

If this doesn't accomplish the changes you had in mind, please let me know.

I've also fixed a test case that was missing a newline at the end of the file, and that required adjusting some CHECK directives because of the EOF match.

This revision is now accepted and ready to land.May 23 2018, 10:45 AM

LGTM

Thanks. Of course, this patch depends on the non-overlapping DAG patches, so I'll commit later.

jdenny set the repository for this revision to rL LLVM.May 24 2018, 12:32 PM
jdenny updated this revision to Diff 153850.Mon, Jul 2, 8:02 PM

Rebased.

jdenny updated this revision to Diff 154302.Thu, Jul 5, 1:44 PM

This updates -v to make more sense in the case of the new CHECK-EMPTY directive. Without this update, -v reported the match for CHECK-EMPTY as occurring at the preceding newline. That's confusing, especially if the preceding line was also empty because it then seemed like the expected empty line was the preceding line instead. Because CHECK-EMPTY is conceptually a CHECK-NEXT for an empty line, -v now reports the match as occurring after the preceding newline, just as it does for CHECK-NEXT. That is, as for CHECK-NEXT, the match range starts at the empty line, and there is a constraint that there is one preceding newline in the search range.

Still LGTM. And of course still waiting on the other reviews, which I'm doing next.

This revision was automatically updated to reflect the committed changes.