[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
Fri May 15 10:18:29 PDT 2020


jdenny added a comment.

IMHO, any comment style will become obvious once you're used to it, and I don't see the readability issue being raised here.  Of course, those issues are highly subjective.

I saw the `;;` and `##` suggestion before I prototyped `COM:`, and here are some (perhaps more objective) reasons why I prototyped `COM:` instead:

1. For those new to FileCheck or maybe just to these issues, I doubt `;;` and `##` look like they should have any relevance to FileCheck (or lit in the future).  `COM:` looks like a FileCheck directive.  As for any FileCheck directive, if you've never seen it, check the docs to find out what it does.

2. `;;` and `##` varies with file type.  Must we configure something different for every file type, or will we enable all of them for all files?  Will there be a `///` and `@@`?  Any other styles to learn?  `COM:` shouldn't need to vary.

3. If you don't know that `;;`, `##`, `///`, `@@`, etc. are special, you might happen to like such a comment style for some purpose and then accidentally suppress a FileCheck directive.  Quietly ignored FileCheck directives are exactly what this proposed diagnostic and several others in the past have been trying to combat.  I have trouble imagining anyone accidentally inserting `COM:` in front of a directive and thinking the latter directive will continue to function.  It then looks like `COM:` becomes the directive.

4. Someone in a previous thread pointed out that `##` is already a comment style in some assemblers.  I don't know if we'll need that for LLVM testing, but will there every be another comment style we cannot support in this manner because such an expansion of an existing comment style collides with it?

If people are really opposed to `COM:`, perhaps we should take this discussion to the mailing list again.


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

https://reviews.llvm.org/D79963





More information about the llvm-commits mailing list