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

Thu Nov 21 02:30:30 PST 2019

thopre added a comment.

Sorry to react only now but for some reason Herald only subscribed me today. First of all, thanks Sjoerd for working on this. I agree with the current consensus, that seems the right approach to me. Below are some wishlist items this discussion made me think about in case any of you find them of interest.

In D53710#1291939 <https://reviews.llvm.org/D53710#1291939>, @probinson wrote:

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

While I agree we cannot diagnose all typos I do think we ought to be able to catch simple cases and I think 1 letter swap, 1 missing letter and 1 extra letter all ought to be diagnosed. That can be combined with Sjoerd's patch by adding an extra message in case the condition for the warning does occur: carets pointing to the suspected typoed CHECK followed by "did you mean <correct CHECK>?".

I would also like in a separate patch trying to diagnose things that look like a directive and are close to one of the --check-prefixes, e.g.:

FileCheck --check-prefix FOO

FOO: correct check
FO: typoed check

This example would not trigger under the follow-up patch proposed by Paul since there's one correct occurence of the directive FOO yet probably suggest an error. So to summarize:

- I would like to see Sjoerd's new warning give suggestion for places which might just be a typo. Just to be clear, this can be done as a follow-up patch, and I am not asking you Sjoerd to do it, just advertising it in case some brave soul feel like taking on the challenge
- I would like to see a new warning to find what looks like a typoed directive of one of the CHECK prefix

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

https://reviews.llvm.org/D53710