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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a reviewer: jhenderson.
jhenderson added a comment.

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?

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.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77227