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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 15 01:35:45 PDT 2020


jhenderson added a comment.

I saw the benefit initially when the proposal was made, but now seeing the consequence in practice in the tools tests at least makes me realise I'm not a fan of the change to make FileCheck error if it sees the string somewhere else in the line. In particular, GNU and LLVM are common prefixes used to distinguish checks for different output modes, but also will often appear in comments to explain the test cases. Similar comments apply to the machine names etc. Needing to add COM is unobvious, and makes the comments messier to read, I found, as soon as I started reading. I think people will repeatedly trip over this when they are writing tests, both new-comers and existing users, because it's not a requirement to use COM in most situations. I think this will mostly lead to annoyance and frustration, and consequently, less of a desire to do any of 1) write comments, 2) write tests, or 3) contribute to LLVM (due to too many hurdles to understand), which in turn will impact the quality of our codebase.


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