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

Jon Roelofs via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 14 16:54:10 PDT 2020


jroelofs added a comment.

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!

> 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.

> 
> 
> 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, and I could go either way on `;; COM:` vs `; COM:`. Happy to update those too.


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