[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman edited reviewers, added: hokein, whisperity, njames93; removed: aaron.ballman.
aaron.ballman added a comment.
Herald added a subscriber: rnkovacs.

We run into this problem quite frequently -- the C++ Core Guidelines put very little effort into thinking about enforcement, and so tool vendors like us are left holding the bag. We can either do what the rule says for enforcement (which is what users often expect to happen, because the rules are supposed to be the source of truth), or we can do something actually useful. For example, this rule is documented (https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-macro-usage.html) to cover ES.30 and ES.31 whose enforcement is `Scream when you see a macro that isn't just used for source control (e.g., #ifdef)`. When the rule and the check disagree, we usually ask check authors to work with the guideline authors to come to an agreement on what the rule should be changed to say and I think we need to do that dance again here. (Aside: our docs also claim this checks for ES.33 whose enforcement is `Warn against short macro names.` and I don't see anything in the check that actually does that; I think the docs meant ES.32 about defining macro names in all caps, and the option for this part of the check is not enabled by default.)

However, my personal experience on engaging with the C++ Core Guidelines authors on enforcement issues in the past has not been positive, which is why I made a personal decision to not review C++ Core Guidelines for clang-tidy once they started hitting enforcement issues where the rule is of too low quality (I don't have the time to do that amount of work on behalf of the C++ Core Guidelines). I think that's the case here, so I'm resigning from the review. However, if the C++ Core Guidelines authors engage in the topic and improve their enforcement to be usefully enforceable, I'm happy to be added back as a reviewer.

I'm adding a few new reviewers who might have more ability to engage on the review in the near term.


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

https://reviews.llvm.org/D116386