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

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 14 17:26:37 PDT 2020


jdenny added a comment.

In D79963#2037615 <https://reviews.llvm.org/D79963#2037615>, @jroelofs wrote:

> In D79963#2037408 <https://reviews.llvm.org/D79963#2037408>, @jdenny wrote:
>
> > This looks like a move in the right direction to me, but I'm biased.
>
>
> Thanks for having a look!


Thanks for working on all this.

> 
> 
>> Hopefully others will chime in, especially individual test suite maintainers.
>> 
>> Here are some minor style suggestions, but feel free to outright ignore them if I'm the only one who cares:
>> 
>> 1. Sometimes you add `COM:` only to lines your diagnostic might catch and not to other lines in the same comment block.  I'd prefer to see it on every line in a comment block.
> 
> I was aiming for the latter, though it looks like the style I ended up with for all of the "here's what each of the CHECKs mean" descriptions having their first comment without the `COM:` directive. Happy to make those consistent with the rest.

Thanks.

> 
> 
>> 
>> 
>> 2. Where `;;` was used to differentiate true comments from FileCheck check directives, you now have `;; COM:`.  I think `; COM:` is sufficient.
> 
> I waffled about that, and even considered adding `;;` as a comment directive in those files. Likewise for `##`. Seems not worth it to add more flavors of `COM:` just for that,

I strongly agree that we should not go down that path.  I feel like it's just asking for new dimensions of silent FileCheck prefix mistakes.  Let's not deviate from `COM:` except as a last resort.

> and I could go either way on `;; COM:` vs `; COM:`. Happy to update those too.

That's my vote, but I'll leave it up to you and anyone else who might comment.  It's a minor point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79963





More information about the llvm-commits mailing list