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

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

In D77227#1959041 <https://reviews.llvm.org/D77227#1959041>, @jhenderson wrote:

> Chiming in late, as I only just saw this discussion. I'll be honest, I don't fully follow the explanations. Would it be possible to write up the policy in FileCheck's documentation somewhere?


After we figure out what it should be. :-)  But there's a deeper point here, the debate should take place on llvm-dev and not in a review.

> In D77227#1955433 <https://reviews.llvm.org/D77227#1955433>, @jroelofs wrote:
> 
>> 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.
> 
> 
> This one seems like something that should be fixed in FileCheck to me. It seems reasonable to add whitespace before or after the "CHECK" part to make things line up in certain cases, particularly when --match-full-lines + --strict-whitespace are enabled.
> 
> I've written and/or reviewed several tests which successfully use something like the following pattern for readability:
> 
>   # RUN: ... | FileCheck %s --check-prefixes=ONE,ANOTHER --strict-whitespace --match-full-lines
>   
>   #          ONE:some output
>   #     ONE-NEXT:more output
>   # ANOTHER-NEXT:yet more output
> 
> 
> Beauty is in the eye of the beholder, but some people might prefer:
> 
>   # RUN: ... | FileCheck %s --check-prefixes=ONE,ANOTHER --strict-whitespace --match-full-lines
>   
>   # ONE         :some output
>   # ONE-NEXT    :more output
>   # ANOTHER-NEXT:yet more output
> 
> 
> so it seems like it should work.

I'd be okay with that.

> In D77227#1955473 <https://reviews.llvm.org/D77227#1955473>, @jdenny wrote:
> 
>> > One solution to that might be to add a FileCheck-specific comment sequence,
>>
>> I think that idea came up once before.  It might also be useful when you want to temporarily comment out a directive.  It would be better than the current practice of mangling the directive.
> 
> 
> FWIW, I'd be supportive of something like this too (I'd go with '##' personally, because that's how I've started marking my comments in tests, to distinguish from lit/FileCheck lines). Mangling the CHECK directive is just asking for forgetting to unmangle it later, and I've had to do the former on more than one occasion. FWIW, I think any comment syntax we adopted should be followed by lit too. I like the symmetry in lit directives and FileCheck prefixes as things stand.

I wouldn't use `##` as I believe there are some assembler variants that use `##` (and not just `#`) as the comment delimiter.  I have a vague memory of writing (possibly Darwin?) assembler where single `#` was insufficient.

We could use `=` which would take care of many of the false-positives from RUN: lines.  Within llvm\test,
`--check-prefix FOO` occurs 519 times while `--check-prefix=FOO` occurs 13313 times.
`--check-prefixes FOO` occurs 85 times while `--check-prefixes=FOO` occurs 5312 times
so it would take a chunk of churn before we could require `=` on the option; that part could be automated, if we went that route.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77227