[PATCH] D54769: [FileCheck] New option -warn

Mon Dec 3 11:06:05 PST 2018

jdenny added a comment.

In D54769#1317073 <https://reviews.llvm.org/D54769#1317073>, @dblaikie wrote:

>

> Ah, OK - Went back to review those. Yeah, unused prefixes are perhaps mostly harmless due to refactoring. Unused directives in the file (things that look like FileCheck directives but aren't used by any FileCheck invocation for that file) would be interesting to know about, but hard to get good numbers I'd imagine, without a bunch more work - having a warning in FileCheck itself would be insufficient (because each FileCheck only sees potentially a subset of the prefixes used in a given file), so probably a bit of a hacky scripting solution could be done to catch most of the cases & get some numbers to see if a more robust/long-term solution could be used (& what the actual bug rate is - how often are they really mistyped V being used only in a subset of checks)

Agreed.

>>> How would having a fixed prefix (like the FC example given) help diagnose misspelled directives in one FileCheck run, when another FileCheck run on the same file may use different check prefixes? (usually named prefixes are used for this situation, where a single file contains checks for multiple different things (different tests/using different flags, or multiple tests using different tools (compile this file, then run dwarfdump & FileCheck that output, then run objdump and FileCheck that output with a different prefix))? The file contains, say, a collection of checks with two different prefixes (DWARF: and OBJ:, for instance) - and each FileCheck run is run with only one of those prefixes, so all the others would look like typos, right? (even if they were prefixed with "FC-")
>> 
>> You'd certainly have to check all FileCheck calls.  For many simple test files, I think a script could parse the test file and report prefixes that are used in directives but never defined in FileCheck commands in RUN lines (interestingly, the diagnostic in the current patch reverses this, and I've already explained how that could produce false positives).  Dynamic approaches could also be explored for more complex tests.  Again, any approach necessarily involves checking all FileCheck calls.
> 
> How're you thinking you'd check all the FileCheck calls?

For simple tests, an external script could parse (static) test files as I described.  That script could run as a separate test case.

I'm not sure which tests would be "simple", but any test that generates the FileCheck check file dynamically would certainly not work with such a script.

For a (hopefully) complete solution, here's a possible approach.  Each FileCheck call builds (1) a list of all prefixes defined (by `--check-prefix[es]`) for that one call and (2) a list of all prefixes used in directives but not defined in the first list, and then it merges those lists into a database.  At the end of a test suite run, lit calls a special FileCheck incantation that compiles that database into a list of unused directives.

> Each FileCheck run would itself look for other FileCheck commands in RUN lines to compute the total set of prefixes used & validate them all? (that'd mean redundant work (O(N^2) in the number of FileCheck commands/prefixes - but probably scale OK) & redundant error messages (since each FileCheck command would report all the unused check names itself))

That seems possible too.  We could experiment with different approaches.

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

https://reviews.llvm.org/D54769