[PATCH] D79963: [llvm][test] Add COM: directives before colon-less non-CHECKs in comments. NFC

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs added a comment.

In D79963#2054177 <https://reviews.llvm.org/D79963#2054177>, @jhenderson wrote:

> The error message improvement will help somewhat, but it doesn't solve the initial problem of "I wrote my comment in the same style as all the surrounding tests, it is easy to read in its current form, but not FileCheck wants me to change it to something that doesn't read as well (because it starts looking like a FileCheck/lit directive which has some meaning)."


Yeah, also pretty bad is getting caught myself with a `COM;` in this very patch. Gosh this is hard ;)

> I'm similar in thinking to @MaskRay, I think. Whilst I absolutely agree that missing colons are an issue and that something should be done, I think we just need to be a bit more refined with the approach. I think the absolutely vast majority of FileCheck CHECKs are either at the start of the line (with possibly some whitespace before), or preceded by some form of comment marker. How about we make the check only pick up cases where one of these two criteria are met? To make things even simpler, I'd define "comment marker" as any non-ASCII alphanumeric, non-whitespace character. The following would be the result:
> 
>   CHECK (would be diagnosed)
>   # CHECK (would be diagnosed)
>   # a CHECK (would not be diagnosed)
>   ## CHECK (would be diagnosed - allowed to make life simpler for other comment sequences like '//')
>   # COM: CHECK (would not be diagnosed)
>   some text # CHECK (would be diagnosed)
>       @#/*&%        CHECK (would be diagnosed)
> 
> 
> This will still cause a small number of false positives (e.g. the three llvm-readobj tests would all generate them, I believe), but it would be easier to reorganise the comment to not trip over it without making it look "ugly" and inconsistent with other comments elsewhere. I guess there might be some false negatives, but I doubt there'd be all that many at all?

I like that too. I'll prototype this, and see if I can get some concrete numbers on the change in diagnosed cases. Also promise to summarize on llvm-dev (haven't sparked that discussion back up yet since I've been quite busy, but I definitely see the importance / value of it).


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

https://reviews.llvm.org/D79963