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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D54769#1317043 <https://reviews.llvm.org/D54769#1317043>, @jdenny wrote:

> In D54769#1316998 <https://reviews.llvm.org/D54769#1316998>, @dblaikie wrote:
>
> > In D54769#1316992 <https://reviews.llvm.org/D54769#1316992>, @jdenny wrote:
> >
> > > Conclusion: A distinct general syntax for FileCheck directives would permit us to implement //precise// detection of //all// unused directives and thus more cleanly combat directive typos.  There's plenty to be discussed, and the migration to a new syntax would be a long-term investment (but it can be done incrementally).  If the problem with typos is as bad as evidence seems to suggest so far,
> >
> >
> > What's the evidence that suggests typos are a cause of a significant cost to the project? I see them come up from time to time, but something on the sub 1% of test cases/false negatives, I'd have thought, if I had to guess.
>
>
> I  probably worded that too strongly.  I'm basing that assessment on the discussions for these patches, including the results Sjoerd has presented so far.  If people's opinions aren't where I thought they were, and if the diagnostic counts Sjoerd reported are mostly false positives, then perhaps you're right that any diagnostics for this problem are not worthwhile and we should move on.


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)

> 
> 
>> 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? 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))


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

https://reviews.llvm.org/D54769