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

Eric Christopher via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 26 10:51:47 PDT 2020


echristo added a comment.



> 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 think this will work for me. It'll make it harder to temporarily disable a particular subset of a test (at least I use ## for that), but this seems more reasonable. Probably needs to be documented in the developer manual as well around writing testcases.


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

https://reviews.llvm.org/D79963





More information about the llvm-commits mailing list