[clang-tools-extra] r345610 - [clang-tidy] cppcoreguidelines-macro-usage: print macro names

Roman Lebedev via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 30 08:52:36 PDT 2018


Author: lebedevri
Date: Tue Oct 30 08:52:36 2018
New Revision: 345610

URL: http://llvm.org/viewvc/llvm-project?rev=345610&view=rev
Log:
[clang-tidy] cppcoreguidelines-macro-usage: print macro names

Summary:
The macro may not have location (or more generally, the location may not exist),
e.g. if it originates from compiler's command-line.

The check complains on all the macros, even those without the location info.
Which means, it only says it does not like it. What is 'it'? I have no idea.
If we don't print the name, then there is no way to deal with that situation.

And in general, not printing name here forces the user to try to understand,
given, the macro definition location, what is the macro name?
This isn't fun.

Also, ignores-by-default the macros originating from command-line,
with an option to not ignore those.

I suspect some more issues may crop up later.

Reviewers: JonasToth, aaron.ballman, hokein, xazax.hun, alexfh

Reviewed By: JonasToth, aaron.ballman

Subscribers: nemanjai, kbarton, rnkovacs, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D53817

Added:
    clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
    clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
    clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
    clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
    clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp?rev=345610&r1=345609&r2=345610&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp Tue Oct 30 08:52:36 2018
@@ -31,32 +31,41 @@ bool isCapsOnly(StringRef Name) {
 
 class MacroUsageCallbacks : public PPCallbacks {
 public:
-  MacroUsageCallbacks(MacroUsageCheck *Check, StringRef RegExp, bool CapsOnly)
-      : Check(Check), RegExp(RegExp), CheckCapsOnly(CapsOnly) {}
+  MacroUsageCallbacks(MacroUsageCheck *Check, const SourceManager &SM,
+                      StringRef RegExp, bool CapsOnly, bool IgnoreCommandLine)
+      : Check(Check), SM(SM), RegExp(RegExp), CheckCapsOnly(CapsOnly),
+        IgnoreCommandLineMacros(IgnoreCommandLine) {}
   void MacroDefined(const Token &MacroNameTok,
                     const MacroDirective *MD) override {
     if (MD->getMacroInfo()->isUsedForHeaderGuard() ||
         MD->getMacroInfo()->getNumTokens() == 0)
       return;
 
+    if (IgnoreCommandLineMacros &&
+        SM.isWrittenInCommandLineFile(MD->getLocation()))
+      return;
+
     StringRef MacroName = MacroNameTok.getIdentifierInfo()->getName();
     if (!CheckCapsOnly && !llvm::Regex(RegExp).match(MacroName))
-      Check->warnMacro(MD);
+      Check->warnMacro(MD, MacroName);
 
     if (CheckCapsOnly && !isCapsOnly(MacroName))
-      Check->warnNaming(MD);
+      Check->warnNaming(MD, MacroName);
   }
 
 private:
   MacroUsageCheck *Check;
+  const SourceManager &SM;
   StringRef RegExp;
   bool CheckCapsOnly;
+  bool IgnoreCommandLineMacros;
 };
 } // namespace
 
 void MacroUsageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AllowedRegexp", AllowedRegexp);
   Options.store(Opts, "CheckCapsOnly", CheckCapsOnly);
+  Options.store(Opts, "IgnoreCommandLineMacros", IgnoreCommandLineMacros);
 }
 
 void MacroUsageCheck::registerPPCallbacks(CompilerInstance &Compiler) {
@@ -64,31 +73,34 @@ void MacroUsageCheck::registerPPCallback
     return;
 
   Compiler.getPreprocessor().addPPCallbacks(
-      llvm::make_unique<MacroUsageCallbacks>(this, AllowedRegexp,
-                                             CheckCapsOnly));
+      llvm::make_unique<MacroUsageCallbacks>(this, Compiler.getSourceManager(),
+                                             AllowedRegexp, CheckCapsOnly,
+                                             IgnoreCommandLineMacros));
 }
 
-void MacroUsageCheck::warnMacro(const MacroDirective *MD) {
+void MacroUsageCheck::warnMacro(const MacroDirective *MD, StringRef MacroName) {
   StringRef Message =
-      "macro used to declare a constant; consider using a 'constexpr' "
+      "macro '%0' used to declare a constant; consider using a 'constexpr' "
       "constant";
 
   /// A variadic macro is function-like at the same time. Therefore variadic
   /// macros are checked first and will be excluded for the function-like
   /// diagnostic.
   if (MD->getMacroInfo()->isVariadic())
-    Message = "variadic macro used; consider using a 'constexpr' "
+    Message = "variadic macro '%0' used; consider using a 'constexpr' "
               "variadic template function";
   else if (MD->getMacroInfo()->isFunctionLike())
-    Message = "function-like macro used; consider a 'constexpr' template "
+    Message = "function-like macro '%0' used; consider a 'constexpr' template "
               "function";
 
-  diag(MD->getLocation(), Message);
+  diag(MD->getLocation(), Message) << MacroName;
 }
 
-void MacroUsageCheck::warnNaming(const MacroDirective *MD) {
+void MacroUsageCheck::warnNaming(const MacroDirective *MD,
+                                 StringRef MacroName) {
   diag(MD->getLocation(), "macro definition does not define the macro name "
-                          "using all uppercase characters");
+                          "'%0' using all uppercase characters")
+      << MacroName;
 }
 
 } // namespace cppcoreguidelines

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.h?rev=345610&r1=345609&r2=345610&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/MacroUsageCheck.h Tue Oct 30 08:52:36 2018
@@ -28,17 +28,20 @@ public:
   MacroUsageCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context),
         AllowedRegexp(Options.get("AllowedRegexp", "^DEBUG_*")),
-        CheckCapsOnly(Options.get("CheckCapsOnly", 0)) {}
+        CheckCapsOnly(Options.get("CheckCapsOnly", 0)),
+        IgnoreCommandLineMacros(Options.get("IgnoreCommandLineMacros", 1)) {}
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerPPCallbacks(CompilerInstance &Compiler) override;
-  void warnMacro(const MacroDirective *MD);
-  void warnNaming(const MacroDirective *MD);
+  void warnMacro(const MacroDirective *MD, StringRef MacroName);
+  void warnNaming(const MacroDirective *MD, StringRef MacroName);
 
 private:
   /// A regular expression that defines how allowed macros must look like.
   std::string AllowedRegexp;
   /// Control if only the check shall only test on CAPS_ONLY macros.
   bool CheckCapsOnly;
+  /// Should the macros without a valid location be diagnosed?
+  bool IgnoreCommandLineMacros;
 };
 
 } // namespace cppcoreguidelines

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst?rev=345610&r1=345609&r2=345610&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst Tue Oct 30 08:52:36 2018
@@ -26,3 +26,8 @@ Options
     Boolean flag to warn on all macros except those with CAPS_ONLY names.
     This option is intended to ease introduction of this check into older
     code bases. Default value is `0`/`false`.
+
+.. option:: IgnoreCommandLineMacros
+
+    Boolean flag to toggle ignoring command-line-defined macros.
+    Default value is `1`/`true`.

Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp?rev=345610&r1=345609&r2=345610&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp Tue Oct 30 08:52:36 2018
@@ -6,16 +6,16 @@
 #define INCLUDE_GUARD
 
 #define problematic_constant 0
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name 'problematic_constant' using all uppercase characters
 
 #define problematic_function(x, y) ((a) > (b) ? (a) : (b))
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name 'problematic_function' using all uppercase characters
 
 #define problematic_variadic(...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name 'problematic_variadic' using all uppercase characters
 //
 #define problematic_variadic2(x, ...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name 'problematic_variadic2' using all uppercase characters
 
 #define OKISH_CONSTANT 42
 #define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))

Added: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp?rev=345610&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-command-line-macros.cpp Tue Oct 30 08:52:36 2018
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy -check-suffixes=NORMAL %s cppcoreguidelines-macro-usage %t -- -- -D_ZZZ_IM_A_MACRO
+// RUN: %check_clang_tidy -check-suffixes=NORMAL %s cppcoreguidelines-macro-usage %t -- -config='{CheckOptions: [{key: cppcoreguidelines-macro-usage.IgnoreCommandLineMacros, value: 1}]}' -- -D_ZZZ_IM_A_MACRO
+// RUN: %check_clang_tidy -check-suffixes=NORMAL,CL %s cppcoreguidelines-macro-usage %t -- -config='{CheckOptions: [{key: cppcoreguidelines-macro-usage.IgnoreCommandLineMacros, value: 0}]}' -- -D_ZZZ_IM_A_MACRO
+
+// CHECK-MESSAGES-CL: warning: macro '_ZZZ_IM_A_MACRO' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES-NORMAL: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant

Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp?rev=345610&r1=345609&r2=345610&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp Tue Oct 30 08:52:36 2018
@@ -6,16 +6,16 @@
 #define INCLUDE_GUARD
 
 #define PROBLEMATIC_CONSTANT 0
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
 
 #define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro 'PROBLEMATIC_FUNCTION' used; consider a 'constexpr' template function
 
 #define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC' used; consider using a 'constexpr' variadic template function
 
 #define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
 #define DEBUG_CONSTANT 0
 #define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))

Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage.cpp?rev=345610&r1=345609&r2=345610&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-macro-usage.cpp Tue Oct 30 08:52:36 2018
@@ -4,15 +4,15 @@
 #define INCLUDE_GUARD
 
 #define PROBLEMATIC_CONSTANT 0
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
 
 #define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro 'PROBLEMATIC_FUNCTION' used; consider a 'constexpr' template function
 
 #define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC' used; consider using a 'constexpr' variadic template function
 
 #define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
 #endif




More information about the cfe-commits mailing list