[clang-tools-extra] [clang-tidy] Update llvmlibc-implementation-in-namespace to new rules (PR #66504)
Guillaume Chatelet via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 18 00:41:19 PDT 2023
https://github.com/gchatelet updated https://github.com/llvm/llvm-project/pull/66504
>From f1427a81c4a3425c1a574316fc26d2c74297b34b Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Fri, 15 Sep 2023 12:34:17 +0000
Subject: [PATCH 1/2] [clang-tidy] Update llvmlibc-implementation-in-namespace
to new rules
This is the implementation of step 3 of
https://discourse.llvm.org/t/rfc-customizable-namespace-to-allow-testing-the-libc-when-the-system-libc-is-also-llvms-libc/73079.
---
.../ImplementationInNamespaceCheck.cpp | 21 ++++++++-----
.../llvmlibc/implementation-in-namespace.rst | 23 +++++++++-----
.../llvmlibc/implementation-in-namespace.cpp | 31 +++++++++++++------
3 files changed, 50 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
index d05310f09ef773a..69a385f5be9807f 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
@@ -14,7 +14,9 @@ using namespace clang::ast_matchers;
namespace clang::tidy::llvm_libc {
-const static StringRef RequiredNamespace = "__llvm_libc";
+const static StringRef RequiredNamespaceStart = "__llvm_libc";
+const static StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE";
+
void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
decl(hasParent(translationUnitDecl()), unless(linkageSpecDecl()))
@@ -29,16 +31,19 @@ void ImplementationInNamespaceCheck::check(
if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation()))
return;
- if (const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) {
- if (NS->getName() != RequiredNamespace) {
- diag(NS->getLocation(), "'%0' needs to be the outermost namespace")
- << RequiredNamespace;
- }
+ if (auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) {
+ if (!Result.SourceManager->isMacroBodyExpansion(NS->getLocation()))
+ diag(NS->getLocation(),
+ "the outermost namespace should be the '%0' macro")
+ << RequiredNamespaceMacroName;
+ else if (!NS->getName().starts_with(RequiredNamespaceStart))
+ diag(NS->getLocation(), "the outermost namespace should start with '%0'")
+ << RequiredNamespaceStart;
return;
}
diag(MatchedDecl->getLocation(),
- "declaration must be declared within the '%0' namespace")
- << RequiredNamespace;
+ "declaration must be declared within a namespace starting with '%0'")
+ << RequiredNamespaceStart;
}
} // namespace clang::tidy::llvm_libc
diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst
index 33d6dc8ff125c84..47ea2b866a93404 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/implementation-in-namespace.rst
@@ -8,21 +8,30 @@ correct namespace.
.. code-block:: c++
- // Correct: implementation inside the correct namespace.
- namespace __llvm_libc {
+ // Implementation inside the LIBC_NAMESPACE namespace.
+ // Correct if:
+ // - LIBC_NAMESPACE is a macro
+ // - LIBC_NAMESPACE expansion starts with `__llvm_libc`
+ namespace LIBC_NAMESPACE {
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
- // Namespaces within __llvm_libc namespace are allowed.
- namespace inner{
+ // Namespaces within LIBC_NAMESPACE namespace are allowed.
+ namespace inner {
int localVar = 0;
}
// Functions with C linkage are allowed.
- extern "C" void str_fuzz(){}
+ extern "C" void str_fuzz() {}
}
- // Incorrect: implementation not in a namespace.
+ // Incorrect: implementation not in the LIBC_NAMESPACE namespace.
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
- // Incorrect: outer most namespace is not correct.
+ // Incorrect: outer most namespace is not the LIBC_NAMESPACE macro.
namespace something_else {
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
}
+
+ // Incorrect: outer most namespace expansion does not start with `__llvm_libc`.
+ #define LIBC_NAMESPACE custom_namespace
+ namespace LIBC_NAMESPACE {
+ void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
index e75556a623b655c..16c5f9ca1067ec5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp
@@ -3,18 +3,18 @@
#define MACRO_A "defining macros outside namespace is valid"
class ClassB;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the '__llvm_libc' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within a namespace starting with '__llvm_libc'
struct StructC {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within the '__llvm_libc' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within a namespace starting with '__llvm_libc'
char *VarD = MACRO_A;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the '__llvm_libc' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within a namespace starting with '__llvm_libc'
typedef int typeE;
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within the '__llvm_libc' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within a namespace starting with '__llvm_libc'
void funcF() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within the '__llvm_libc' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within a namespace starting with '__llvm_libc'
namespace namespaceG {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: '__llvm_libc' needs to be the outermost namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE' macro
namespace __llvm_libc{
namespace namespaceH {
class ClassB;
@@ -26,9 +26,20 @@ typedef int typeE;
void funcF() {}
} // namespace namespaceG
-// Wrapped in correct namespace.
-namespace __llvm_libc {
-// Namespaces within __llvim_libc namespace allowed.
+// Wrapped in macro namespace but with an incorrect name
+#define LIBC_NAMESPACE custom_namespace
+namespace LIBC_NAMESPACE {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should start with '__llvm_libc'
+namespace namespaceH {
+class ClassB;
+} // namespace namespaceH
+} // namespace LIBC_NAMESPACE
+
+
+// Wrapped in macro namespace with a valid name, LIBC_NAMESPACE starts with '__llvm_libc'
+#undef LIBC_NAMESPACE
+#define LIBC_NAMESPACE __llvm_libc_xyz
+namespace LIBC_NAMESPACE {
namespace namespaceI {
class ClassB;
} // namespace namespaceI
@@ -37,4 +48,4 @@ char *VarD = MACRO_A;
typedef int typeE;
void funcF() {}
extern "C" void extern_funcJ() {}
-} // namespace __llvm_libc
+} // namespace LIBC_NAMESPACE
>From bae9be668c284678818e957272a2906fe12c0808 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Mon, 18 Sep 2023 07:40:49 +0000
Subject: [PATCH 2/2] Address comment
---
.../clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
index 69a385f5be9807f..7ac483571b0de6c 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
@@ -31,7 +31,7 @@ void ImplementationInNamespaceCheck::check(
if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation()))
return;
- if (auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) {
+ if (const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl)) {
if (!Result.SourceManager->isMacroBodyExpansion(NS->getLocation()))
diag(NS->getLocation(),
"the outermost namespace should be the '%0' macro")
More information about the cfe-commits
mailing list