[PATCH] D77227: [RFC][FileCheck] Require colon immediately after CHECK directives

Jonathan Roelofs via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 1 12:04:50 PDT 2020


jroelofs added a comment.

In D77227#1955284 <https://reviews.llvm.org/D77227#1955284>, @jdenny wrote:

> Thanks for working on this.
>
>   CHECK           gotcha 1
>
>
> This case seems like it would trigger too many false positives.  Do you have numbers on how many true positives that this case alone catches and that are not caught by the other cases?


I don't yet, but I'd be happy to take some measurements/estimates.

Subjectively, it was pretty bad. There are a lot of CHECK's without colons in comments (rightfully so).

One solution to that might be to add a FileCheck-specific comment sequence, though I don't imagine that being very ergonomic to use, and wouldn't help with all the existing tests that have these false positive comments.

> 
> 
>   CHECK :         gotcha 2
> 
> 
> As long as only whitespace separates `CHECK` and `:`, this one seems ok.

Ok in what sense? If there is any whitespace between the check directive and the colon, FileCheck does not trigger on it. That, to me, makes it look like a valid test that doesn't actually doing anything. Whether this is intentional or not isn't particularly clear to me.

> 
> 
>   CHECK-SAME-DAG: gotcha 3
>   CHECKNEXT:      gotcha 4
>   CHECKDAG:       gotcha 5
> 
> 
> These seem much less likely to trigger false positives.

Agreed.

> @probinson and @SjoerdMeijer were also involved in previous discussions in this area and might want to take a look.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77227





More information about the llvm-commits mailing list