[PATCH] D91191: WIP: [FileCheck] Add undefined prefix report tool

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added a comment.

Here are some examples of using this patch:

  # Remove any existing report or you'll append to it.
  $ rm -f report.txt
  
  # Specify the report location absolutely so we don't end up with many
  # different files relative to the various FileCheck invocations in the
  # test suite.
  $ FILECHECK_PREFIX_REPORT=`pwd`/report.txt ninja check-lit
  
  # Process the report, assuming llvm/utils/FileCheck/utils is in PATH.
  $ process-prefix-report.py report.txt
  /home/jdenny/ornl/llvm/llvm-mono-git2-build/utils/lit/tests/Inputs/shtest-env/env-calls-env.txt:
    prefixes used but never defined:
      used on line 6: CHECK-2
      used on line 7: CHECK-2
    prefixes defined but never used:
      CHECK-2-EMPTY

Ah!  That's my bug.  The problem is I defined a prefix named
`CHECK-2-EMPTY`, but directives using that prefix are parsed as having
a `CHECK-2` prefix.  That's subtle.  It would be nice if FileCheck
complained about such prefixes.

It would also be nice if "defined but never used" errors included line
numbers.  I suppose lit could set line numbers in env vars so
FileCheck could read those when writing out the report.

Anyway, let's try another example:

  $ rm -f report.txt
  $ FILECHECK_PREFIX_REPORT=`pwd`/report.txt ninja check-llvm-feature
  $ process-prefix-report.py report.txt
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/md_on_instruction.ll:
    prefixes used but never defined:
      used on line 4: entry
      used on line 12: label
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/OperandBundles/special-state.ll:
    prefixes used but never defined:
      used on line 11: entry
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/strip_names.ll:
    prefixes used but never defined:
      used on line 22: somelabel
  # and so on with 110 lines of output

There are too many false "used but never defined" errors this time
because the test code syntax collides with FileCheck directive syntax.
In other suites I've looked at, this often happens in comments as
well.

So, let's try to make the output more meaningful by making the
assumption that most FileCheck prefixes start with one of a small set
of strings that are unlikely to be mistyped:

  $ process-prefix-report.py --starts-with=CHECK report.txt
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/strip_names.ll:
    prefixes defined but not matching starts-with strings:
      NONAME
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/optnone-opt.ll:
    prefixes defined but not matching starts-with strings:
      O2O3
      NPM-LOOP
      NPM-O2O3
      NPM-O1
      NPM-MORE
      O1
      O0
      NPM-O0
      LOOP
      MORE
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/optnone-llc.ll:
    prefixes defined but not matching starts-with strings:
      LLC-MORE
      NOFAST
      FAST
      LLC-Ox

OK, we need more starts-with strings as we might be ignoring many used
prefixes.  Try to specify the shortest versions possible so that
directives aren't overlooked.  For example, specify "NPM" not
"NPM-LOOP".

  $ process-prefix-report.py --starts-with=CHECK,O,NPM,LOOP,MORE,LLC,NO,FAST report.txt
  /home/jdenny/ornl/llvm/llvm-mono-git2/llvm/test/Feature/optnone-llc.ll:
    prefixes used but never defined:
      used on line 37: LLC-O0

That's better.

The starts-with assumption here does hold true in some of my
downstream test suites.  However, so far, the majority of test suites
I've encountered upstream don't obey that assumption.  So, this
approach can prove tedious and can potentially miss many used but
never defined prefixes.

But maybe one day we'll have a better way....

I've been thinking for a while it would be nice to have a
`CHECK-IF(expr): pattern` directive.  `expr` would likely be composed
of FileCheck numeric variables and operators.  You could do things
like `CHECK-IF(GCN|NVPTX): pattern` instead of having to define a
GCN_AND_NVPTX prefix or write the same directive multiple times with
different prefixes.  Directives suffixes would look like
`CHECK-NEXT-IF(expr): pattern`.

Now imagine if test suites gradually start using FileCheck variables
inside of `-IF` directives instead of using FileCheck prefixes, and
imagine if FileCheck invocations set them with `-D`.  `git grep -- '-IF *('`
currently has no matches in the LLVM repo, so FileCheck
could assume all matches to that pattern are intended as FileCheck
directives even if otherwise mistyped, and I'm assuming it's
relatively unlikely that people will mistype the `-IF *(` part.  Thus,
finding variables used in such `-IF` directives will prove much easier
than finding used prefixes.  No `--starts-with` needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91191/new/

https://reviews.llvm.org/D91191