Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] ignoring macro with hash preprocessing token in cppcoreguidelines-macro-usage #95265

Merged

Conversation

HerrCai0907
Copy link
Contributor

# and ## preprocessing tokens cannot be replaced by constexpr function. It should be ignored in check.

@llvmbot
Copy link
Member

llvmbot commented Jun 12, 2024

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

# and ## preprocessing tokens cannot be replaced by constexpr function. It should be ignored in check.


Full diff: https://github.com/llvm/llvm-project/pull/95265.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp (+5-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst (+1)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp (+4)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
index 0cd4bf6fdfd87..11eb056e916d3 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -7,12 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "MacroUsageCheck.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Regex.h"
-#include <algorithm>
 #include <cctype>
 #include <functional>
 
@@ -37,7 +37,10 @@ class MacroUsageCallbacks : public PPCallbacks {
                     const MacroDirective *MD) override {
     if (SM.isWrittenInBuiltinFile(MD->getLocation()) ||
         MD->getMacroInfo()->isUsedForHeaderGuard() ||
-        MD->getMacroInfo()->getNumTokens() == 0)
+        MD->getMacroInfo()->tokens_empty() ||
+        llvm::any_of(MD->getMacroInfo()->tokens(), [](const Token &T) {
+          return T.isOneOf(tok::TokenKind::hash, tok::TokenKind::hashhash);
+        }))
       return;
 
     if (IgnoreCommandLineMacros &&
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6bf70c5cf4f8a..5d5aecd67b2d7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -266,6 +266,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/use-after-move>` check to also handle
   calls to ``std::forward``.
 
+- Improved :doc:`cppcoreguidelines-macro-usage
+  <clang-tidy/checks/cppcoreguidelines/macro-usage>` check by ignoring macro with
+  hash preprocessing token.
+
 - Improved :doc:`cppcoreguidelines-missing-std-forward
   <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by no longer
   giving false positives for deleted functions, by fixing false negatives when only
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst
index 8b763c28479e6..49417effbb6ff 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst
@@ -17,6 +17,7 @@ Examples:
   #define C 0
   #define F1(x, y) ((a) > (b) ? (a) : (b))
   #define F2(...) (__VA_ARGS__)
+  #define F3(x, y) x##y
   #define COMMA ,
   #define NORETURN [[noreturn]]
   #define DEPRECATED attribute((deprecated))
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp
index 404aafb6b1c45..865ef9df1182e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp
@@ -31,6 +31,10 @@
 // CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
 // These are all examples of common macros that shouldn't have constexpr suggestions.
+#define CONCAT_NAME(a, b) a##b
+
+#define CONCAT_STR(a, b) #a #b
+
 #define COMMA ,
 
 #define NORETURN [[noreturn]]

@llvmbot
Copy link
Member

llvmbot commented Jun 12, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

Changes

# and ## preprocessing tokens cannot be replaced by constexpr function. It should be ignored in check.


Full diff: https://github.com/llvm/llvm-project/pull/95265.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp (+5-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst (+1)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp (+4)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
index 0cd4bf6fdfd87..11eb056e916d3 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -7,12 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "MacroUsageCheck.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Regex.h"
-#include <algorithm>
 #include <cctype>
 #include <functional>
 
@@ -37,7 +37,10 @@ class MacroUsageCallbacks : public PPCallbacks {
                     const MacroDirective *MD) override {
     if (SM.isWrittenInBuiltinFile(MD->getLocation()) ||
         MD->getMacroInfo()->isUsedForHeaderGuard() ||
-        MD->getMacroInfo()->getNumTokens() == 0)
+        MD->getMacroInfo()->tokens_empty() ||
+        llvm::any_of(MD->getMacroInfo()->tokens(), [](const Token &T) {
+          return T.isOneOf(tok::TokenKind::hash, tok::TokenKind::hashhash);
+        }))
       return;
 
     if (IgnoreCommandLineMacros &&
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6bf70c5cf4f8a..5d5aecd67b2d7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -266,6 +266,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/use-after-move>` check to also handle
   calls to ``std::forward``.
 
+- Improved :doc:`cppcoreguidelines-macro-usage
+  <clang-tidy/checks/cppcoreguidelines/macro-usage>` check by ignoring macro with
+  hash preprocessing token.
+
 - Improved :doc:`cppcoreguidelines-missing-std-forward
   <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by no longer
   giving false positives for deleted functions, by fixing false negatives when only
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst
index 8b763c28479e6..49417effbb6ff 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/macro-usage.rst
@@ -17,6 +17,7 @@ Examples:
   #define C 0
   #define F1(x, y) ((a) > (b) ? (a) : (b))
   #define F2(...) (__VA_ARGS__)
+  #define F3(x, y) x##y
   #define COMMA ,
   #define NORETURN [[noreturn]]
   #define DEPRECATED attribute((deprecated))
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp
index 404aafb6b1c45..865ef9df1182e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/macro-usage.cpp
@@ -31,6 +31,10 @@
 // CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
 // These are all examples of common macros that shouldn't have constexpr suggestions.
+#define CONCAT_NAME(a, b) a##b
+
+#define CONCAT_STR(a, b) #a #b
+
 #define COMMA ,
 
 #define NORETURN [[noreturn]]

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HerrCai0907 HerrCai0907 merged commit b06da39 into llvm:main Jun 13, 2024
11 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/cppcoreguidelines-macro-usage branch June 13, 2024 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants