[PATCH] D53710: [FileCheck] Warn if a prefix is only used in LABEL checks

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 8 11:16:31 PST 2018


probinson added a comment.

Because there is nothing to indicate that a line contains a FileCheck directive, other than that it pattern-matches a user-specified string with an optional suffix, we can't reliably report all typos.  So for example:

  CHECK-LABEL: A
  CHECK: B
  CHEKC: C

the third line can't be diagnosed by FileCheck under any reasonable heuristic. So, anything we do is a best-effort, and ideally produces no false positives by default.

Staying out of the 'dodgy' and 'broken' terminology debate, it seems that a heuristic of "prefix occurs only with LABEL suffix" will have too many false positives (or at least, marginal positives, for tests that do behave correctly even if they don't use LABEL the way it was intended).  So, if we proceed with this heuristic, it needs to be under an off-by-default command line option.  (You can set the option using Joel's new environment variable, and look at particular tests or sets of tests, at your leisure.  If we get to a point where the option provides a clean test run for all components and all targets, then we can make it on-by-default.)

Another useful diagnostic (probably best done separately) would be when a given prefix triggers no directives at all.  That's obviously an incorrect test; either it has typos or has a dangling `-check-prefix` whose directives have all been removed.  Right now, this diagnostic occurs only when there are no matching directives at all.  But if you specify two prefixes, and only one of them matches a directive, FileCheck is silent.  I think that's bad.

Renaming LABEL to BOUND is probably a pipe dream, let's take that off the table at least for now.  Improved understanding of the suffix would help in general, but it's a bit tangential to the current goal.

To summarize, then:

1. Put the current diagnostic under an off-by-default option.
2. In a follow-up patch, emit a diagnostic for any prefix that has no matches (even if other prefixes do have matches). This might require fixing some tests, or it will need to be off-by-default while we do test cleanup.
3. Ignore the RFC and rename LABEL to BOUND ideas.


https://reviews.llvm.org/D53710





More information about the llvm-commits mailing list