[PATCH] D29692: [clang-tidy] add check modernize-use-const-instead-of-define

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin marked 9 inline comments as done.
AlexanderLanin added a comment.

Not sure about CppCoreGuidelines as several guidelines have the same rule and I only used CppCoreGuidelines as it's convenient to link a specific rule. But I can move it if you like?!



================
Comment at: clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp:41
+/// others like ~ are not so obvious and depend on usage
+bool isReasonableNumberPrefix(const Token &T) {
+  return T.isOneOf(tok::plus, tok::minus);
----------------
Eugene.Zelenko wrote:
> In LLVM Functions should be static, not inside anonymous namespace. Same below.
mhh copied from MacroParentheses check


================
Comment at: docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst:11
+
+voif defineSeven() {
+  #define X 7
----------------
malcolm.parsons wrote:
> s/voif/void/
> Why is this in a function anyway?
that's why I wrote strange example ;-)
Hope the new one is better ?!


================
Comment at: test/clang-tidy/modernize-use-const-instead-of-define.cpp:6
+#define BAD1              -1
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD2              2
----------------
malcolm.parsons wrote:
> Check the message?
I didn't check the exact message since I changed the wording several times and it's currently the same message for everything that's detected anyway. Shall I check the exact message anyway?


Repository:
  rL LLVM

https://reviews.llvm.org/D29692