WIP: [FileCheck] Add undefined prefix report tool

[jdenny]

Summary

This patch is not ready for a detailed review. It has not been
rebased in a long time, it needs better documentation, it needs more
testing, and the code may not be great. I'm posting it early so
@probinson might use it as a starting point for a similar project he
is planning.

[jdenny]

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_OR_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.

[probinson]

Thanks, Joel! It will likely take a few weeks for me to finish up the unittests part of my project, at which point this seems like a nice starting point for the FileCheck part. The basic infrastructure is definitely there.

llvm/utils/FileCheck/FileCheck.cpp

while (AnnotationItr != AnnotationEnd &&
       ++AnnotationItr;
     }
   }
   DumpEllipsisOrElidedLines(OS, ElidedLinesOS.str(), LabelWidth);
 
   OS << ">>>>>>\n";
 }
 
+static bool WritePrefixReport() {
+  // This is a separate environment variable not a command-line option to be
+  // passed via FILECHECK_OPTS because the latter is blocked in some test
+  // suites for some FileCheck uses, but we need to see all FileCheck uses
+  // or we might falsely report as undefined prefixes that are only defined by
+  // those uses.
+  if (const char *PrefixReport = getenv("FILECHECK_PREFIX_REPORT")) {
+    SmallString<200> CheckFilenameAbs = StringRef(CheckFilename);
+    std::error_code EC = sys::fs::make_absolute(CheckFilenameAbs);
+    if (EC) {
+      errs() << "Failed computing absolute version of check file name "
+             << "\"" << CheckFilename << "\": " << EC.message() << '\n';
+      return true;
+    }
+    raw_fd_ostream ReportFile(PrefixReport, EC,
+                              sys::fs::OF_Text | sys::fs::OF_Append);
+    if (EC) {
+      errs() << "Failed opening prefix report file \"" << PrefixReport
+             << "\": " << EC.message() << '\n';
+      return true;
+    }
+    ReportFile << CheckFilenameAbs << ":";
+    if (CheckPrefixes.empty())
+      ReportFile << "CHECK";
+    else {
+      for (auto Prefix : CheckPrefixes) {
+        if (Prefix != CheckPrefixes.front())
+          ReportFile << ",";
+        ReportFile << Prefix;
+      }
+    }
+    ReportFile << "\n";
+    ReportFile.close();
+    if (ReportFile.has_error()) {
+      errs() << "Failed writing prefix report file \"" << PrefixReport
+             << "\": " << EC.message() << '\n';
+      return true;
+    }
+  }
+  return false;
+}
+
 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;
   }
 
+  if (WritePrefixReport())
+    return 2;
+
   FileCheckRequest Req;
   for (StringRef Prefix : CheckPrefixes)
     Req.CheckPrefixes.push_back(Prefix);
 
   for (StringRef Prefix : CommentPrefixes)
     Req.CommentPrefixes.push_back(Prefix);
 
   for (StringRef CheckNot : ImplicitCheckNot)

llvm/utils/FileCheck/utils/process-prefix-report.py

+#!/usr/bin/env python
+
+import getopt
+import re
+import os
+import sys
+
+# For each line:
+# - match = the first match to directivePattern.
+# - If there is no match, continue to the next line.
+# - directive = the match without the trailing ':'.
+# - Unless --starts-with is specified, if directive is in directivesIgnored,
+#   continue to the next line.
+# - prefix = directive minus any suffix from suffixPatterns.
+# - If prefix is listed as defined in the prefix report, continue to the next
+#   line.
+# - If --starts-with is specified, and if prefix does not start with one of the
+#   strings specified there, continue to the next line.
+# - Report the prefix as undefined.
+directivePattern = re.compile('([a-zA-Z0-9_-]+):')
+directivesIgnored = frozenset(('RUN', 'FIXME', 'TODO', 'REQUIRES',
+                               'UNSUPPORTED', 'XFAIL'))
+suffixPatterns = ('COUNT-(?:[0-9]*)', 'NEXT', 'SAME', 'NOT', 'DAG', 'LABEL',
+                  'EMPTY')
+suffixPattern = re.compile('\A(.*)-(?:{})\Z'.format('|'.join(suffixPatterns)))
+
+def readReportFile(cmd, reportFileName):
+    try:
+      reportFile = open(reportFileName, "r")
+    except IOError as e:
+        sys.stderr.write("{cmd}: error: {file}: failed to open report file: "
+                         "{err}\n".format(cmd=cmd, file=reportFileName,
+                                          err=e.strerror))
+        sys.exit(1)
+    lineNum = 0
+    checkFiles = {}
+    for line in reportFile:
+        lineNum += 1
+        colon = line.rfind(":")
+        if colon == -1:
+            sys.stderr.write("{cmd}: error: {file} at line {line}: missing "
+                             "':'\n".format(cmd=cmd, file=reportFileName,
+                                            line=lineNum))
+            sys.exit(1)
+        checkFileName = line[:colon]
+        if len(checkFileName) == 0:
+            sys.stderr.write("{cmd}: error: {file} at line {line}: check file "
+                             " name is empty\n".format(
+                                 cmd=cmd, file=reportFileName, line=lineNum))
+            sys.exit(1)
+        prefixesField = line[colon + 1:].strip()
+        if len(prefixesField) == 0:
+            sys.stderr.write("{cmd}: error: {file} at line {line}: prefix "
+                             "list is blank\n".format(
+                                 cmd=cmd, file=reportFileName, line=lineNum))
+            sys.exit(1)
+        prefixes = prefixesField.split(',')
+        if not checkFiles.get(checkFileName, None):
+            checkFiles[checkFileName] = set()
+        checkFiles[checkFileName].update(prefixes)
+    if lineNum == 0:
+        sys.stderr.write("{cmd}: error: {file}: empty file\n"
+                         .format(cmd=cmd, file=reportFileName))
+        sys.exit(1)
+    return checkFiles
+
+def readCheckFile(checkFileName, prefixesDefined, suppressUndefined,
+                  suppressUnused, startsWith):
+    try:
+      checkFile = open(checkFileName, "r")
+    except IOError as e:
+      sys.stderr.write("error: {file}: failed to open check file: {err}\n"
+                       .format(file=checkFileName, err=e.strerror))
+      return True
+
+    errHeader = "{}:\n".format(checkFileName)
+    err = False
+
+    if startsWith:
+        for prefix in prefixesDefined:
+            found = False
+            for s in startsWith:
+                if prefix.startswith(s):
+                    found = True
+                    break
+            if not found:
+                if not err:
+                    sys.stderr.write(errHeader)
+                    sys.stderr.write("  prefixes defined but not matching"
+                                     " starts-with strings:\n")
+                sys.stderr.write("    {}\n".format(prefix))
+                err = True
+
+    lineNum = 0
+    prefixesUsed = set()
+    undefinedErr = False
+    for line in checkFile:
+        lineNum += 1
+        directiveColonMatch = directivePattern.search(line)
+        if not directiveColonMatch:
+            continue
+        directive = directiveColonMatch.group(1)
+        if not startsWith and directive in directivesIgnored:
+            continue
+        suffixMatch = suffixPattern.match(directive)
+        if suffixMatch:
+            prefix = suffixMatch.group(1)
+        else:
+            prefix = directive
+        if prefix in prefixesDefined:
+            prefixesUsed.add(prefix)
+            continue
+        if suppressUndefined:
+            continue
+        if startsWith:
+            found = False
+            for s in startsWith:
+                if prefix.startswith(s):
+                    found = True
+                    break
+            if not found:
+                # TODO: Should we try to match again on the same line?
+                continue
+        if not err:
+            sys.stderr.write(errHeader)
+            err = True
+        if not undefinedErr:
+            sys.stderr.write("  prefixes used but never defined:\n")
+            undefinedErr = True
+        sys.stderr.write("    used on line {line}: {prefix}\n"
+                         .format(line=lineNum, prefix=prefix))
+
+    if not suppressUnused:
+        prefixesUnused = prefixesDefined - prefixesUsed
+        if prefixesUnused:
+            if not err:
+                sys.stderr.write(errHeader)
+                err = True
+            sys.stderr.write("  prefixes defined but never used:\n")
+            for prefix in prefixesUnused:
+                sys.stderr.write("    {}\n".format(prefix))
+    return err
+
+def usage(cmd):
+    return '''\
+Usage: {cmd} [opt]... prefix-report-file"
+
+Where opt is one of
+
+  * --suppress-undefined:
+
+    Don't report used but undefined prefixes.
+
+  * --suppress-unused:
+
+    Don't report defined but unused prefixes.
+
+  * --starts-with=<LIST>
+
+    A comma-delimited list of strings one of which each prefix must start with.
+    This helps to avoid false positives when reporting used but undefined
+    prefixes because it ignores any used prefix that doesn't start with one of
+    these strings.  To help avoid omissions from this list, it also complains
+    about any defined prefix that doesn't start with one of strings, and it
+    disables ignoring the usual list of false positives ("TODO:", "FIXME:",
+    etc.) in cases where they start with one of these strings.
+'''.format(cmd=cmd)
+
+def main(argv):
+    cmd = os.path.basename(argv[0])
+    args = argv[1:]
+
+    # Process command-line options.
+    suppressUndefined = False
+    suppressUnused = False
+    startsWith = None
+    try:
+        opts, args = getopt.gnu_getopt(
+            args, "", ["suppress-undefined", "suppress-unused",
+                       "starts-with="])
+    except getopt.GetoptError as err:
+        sys.stderr.write("{cmd}: error: {err}\n\n".format(
+                             cmd=cmd, err=str(err)))
+        sys.stderr.write(usage(cmd))
+        sys.exit(1)
+    for o, a in opts:
+        if o == "--suppress-undefined":
+            suppressUndefined = True
+        elif o == "--suppress-unused":
+            suppressUnused = True
+        elif o == "--starts-with":
+            # TODO: Complain for invalid prefixes.
+            startsWith = a.split(',')
+        else:
+            assert False, "unexpected command-line option"
+
+    # Process command-line argument.
+    if len(args) != 1:
+        sys.stderr.write(usage(cmd))
+        sys.exit(1)
+    reportFileName = args[0]
+
+    # Read FileCheck's prefix report for a list of prefixes defined for each
+    # check file, and then check which prefixes are actually used in those
+    # check files.
+    checkFiles = readReportFile(cmd, reportFileName)
+    err = False
+    for checkFileName, prefixesDefined in checkFiles.items():
+        if readCheckFile(checkFileName, prefixesDefined, suppressUndefined,
+                         suppressUnused, startsWith):
+            err = True
+    if err:
+        sys.exit(1)
+
+if __name__=='__main__':
+    main(sys.argv)

llvm/utils/lit/lit/TestingConfig.py

def fromdefaults(litConfig):
             'LLVM_DISABLE_CRASH_REPORT' : '1',
             }
 
         pass_vars = ['LIBRARY_PATH', 'LD_LIBRARY_PATH', 'SYSTEMROOT', 'TERM',
                      'CLANG', 'LD_PRELOAD', 'ASAN_OPTIONS', 'UBSAN_OPTIONS',
                      'LSAN_OPTIONS', 'ADB', 'ANDROID_SERIAL', 'SSH_AUTH_SOCK',
                      'SANITIZER_IGNORE_CVE_2016_2143', 'TMPDIR', 'TMP', 'TEMP',
                      'TEMPDIR', 'AVRLIT_BOARD', 'AVRLIT_PORT',
-                     'FILECHECK_OPTS', 'VCINSTALLDIR', 'VCToolsinstallDir',
+                     'FILECHECK_OPTS', 'FILECHECK_PREFIX_REPORT',
+                     'VCINSTALLDIR', 'VCToolsinstallDir',
                      'VSINSTALLDIR', 'WindowsSdkDir', 'WindowsSDKLibVersion']
 
         if sys.platform == 'win32':
             pass_vars.append('INCLUDE')
             pass_vars.append('LIB')
             pass_vars.append('PATHEXT')
             environment['PYTHONBUFFERED'] = '1'