https://github.com/ilovepi created
https://github.com/llvm/llvm-project/pull/98424
This patch updates the clang-tidy checks for llvm-libc to ensure that the
namespace macro used to declare the libc namespace is updated from
LIBC_NAMESPACE to LIBC_NAMESPACE_DECL which by default has hidden visibility.
Co-authored-by: Prabhu Rajesakeran <prabh...@google.com>
>From 2270c6ac1a775727bcb9766dd75646e20d120b0a Mon Sep 17 00:00:00 2001
From: prabhukr <prabh...@google.com>
Date: Mon, 8 Jul 2024 15:18:10 -0700
Subject: [PATCH] [libc] Update libc namespace clang-tidy checks
Namespace macro that should be used to declare a new namespace is
updated from LIBC_NAMESPACE to LIBC_NAMESPACE_DECL which by default has
hidden visibility (#97109). This commit updates the clang-tidy checks to
match the new policy.
---
.../llvmlibc/CalleeNamespaceCheck.cpp | 4 +-
.../ImplementationInNamespaceCheck.cpp | 23 +++++++++--
.../clang-tidy/llvmlibc/NamespaceConstants.h | 8 +++-
.../llvmlibc/implementation-in-namespace.rst | 20 ++++-----
.../llvmlibc/implementation-in-namespace.cpp | 41 ++++++++++---------
5 files changed, 58 insertions(+), 38 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
index af977bff70f7c..caa19c595823f 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -51,7 +51,7 @@ void CalleeNamespaceCheck::check(const
MatchFinder::MatchResult &Result) {
// __llvm_libc, we're good.
const auto *NS = dyn_cast<NamespaceDecl>(getOutermostNamespace(FuncDecl));
if (NS && Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) &&
- NS->getName().starts_with(RequiredNamespaceStart))
+ NS->getName().starts_with(RequiredNamespaceRefStart))
return;
const DeclarationName &Name = FuncDecl->getDeclName();
@@ -62,7 +62,7 @@ void CalleeNamespaceCheck::check(const
MatchFinder::MatchResult &Result) {
diag(UsageSiteExpr->getBeginLoc(),
"%0 must resolve to a function declared "
"within the namespace defined by the '%1' macro")
- << FuncDecl << RequiredNamespaceMacroName;
+ << FuncDecl << RequiredNamespaceRefMacroName;
diag(FuncDecl->getLocation(), "resolves to this declaration",
clang::DiagnosticIDs::Note);
diff --git
a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
index ae9819ed97502..bb436e4d12a30 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
@@ -30,20 +30,35 @@ void ImplementationInNamespaceCheck::check(
const auto *MatchedDecl =
Result.Nodes.getNodeAs<Decl>("child_of_translation_unit");
const auto *NS = dyn_cast<NamespaceDecl>(MatchedDecl);
+
+ // LLVM libc declarations should be inside of a non-anonymous namespace.
if (NS == nullptr || NS->isAnonymousNamespace()) {
diag(MatchedDecl->getLocation(),
"declaration must be enclosed within the '%0' namespace")
- << RequiredNamespaceMacroName;
+ << RequiredNamespaceDeclMacroName;
return;
}
+
+ // Enforce that the namespace is the result of macro expansion
if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
- << RequiredNamespaceMacroName;
+ << RequiredNamespaceDeclMacroName;
return;
}
- if (NS->getName().starts_with(RequiredNamespaceStart) == false) {
+
+ // We want the macro to have [[gnu::visibility("hidden")]] as a prefix, but
+ // visibility is just an attribute in the AST construct, so we check that
+ // instead.
+ if (NS->getVisibility() != Visibility::HiddenVisibility) {
diag(NS->getLocation(), "the '%0' macro should start with '%1'")
- << RequiredNamespaceMacroName << RequiredNamespaceStart;
+ << RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart;
+ return;
+ }
+
+ // Lastly, make sure the namespace name actually has the __llvm_libc prefix
+ if (NS->getName().starts_with(RequiredNamespaceRefStart) == false) {
+ diag(NS->getLocation(), "the '%0' macro expansion should start with '%1'")
+ << RequiredNamespaceDeclMacroName << RequiredNamespaceRefStart;
return;
}
}
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
index 7d4120085b866..83908a7875d03 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
+++ b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
@@ -10,7 +10,11 @@
namespace clang::tidy::llvm_libc {
-const static llvm::StringRef RequiredNamespaceStart = "__llvm_libc";
-const static llvm::StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE";
+const static llvm::StringRef RequiredNamespaceRefStart = "__llvm_libc";
+const static llvm::StringRef RequiredNamespaceRefMacroName = "LIBC_NAMESPACE";
+const static llvm::StringRef RequiredNamespaceDeclStart =
+ "[[gnu::visibility(\"hidden\")]] __llvm_libc";
+const static llvm::StringRef RequiredNamespaceDeclMacroName =
+ "LIBC_NAMESPACE_DECL";
} // 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 47ea2b866a934..ec52b9f73a3f3 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,13 +8,13 @@ correct namespace.
.. code-block:: c++
- // Implementation inside the LIBC_NAMESPACE namespace.
+ // Implementation inside the LIBC_NAMESPACE_DECL namespace.
// Correct if:
- // - LIBC_NAMESPACE is a macro
- // - LIBC_NAMESPACE expansion starts with `__llvm_libc`
- namespace LIBC_NAMESPACE {
+ // - LIBC_NAMESPACE_DECL is a macro
+ // - LIBC_NAMESPACE_DECL expansion starts with
`[[gnu::visibility("hidden")]] __llvm_libc`
+ namespace LIBC_NAMESPACE_DECL {
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
- // Namespaces within LIBC_NAMESPACE namespace are allowed.
+ // Namespaces within LIBC_NAMESPACE_DECL namespace are allowed.
namespace inner {
int localVar = 0;
}
@@ -22,16 +22,16 @@ correct namespace.
extern "C" void str_fuzz() {}
}
- // Incorrect: implementation not in the LIBC_NAMESPACE namespace.
+ // Incorrect: implementation not in the LIBC_NAMESPACE_DECL namespace.
void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
- // Incorrect: outer most namespace is not the LIBC_NAMESPACE macro.
+ // Incorrect: outer most namespace is not the LIBC_NAMESPACE_DECL 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 {
+ // Incorrect: outer most namespace expansion does not start with
`[[gnu::visibility("hidden")]] __llvm_libc`.
+ #define LIBC_NAMESPACE_DECL custom_namespace
+ namespace LIBC_NAMESPACE_DECL {
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 2246a430dd1f5..4a5576f2c5d62 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,60 +3,61 @@
#define MACRO_A "defining macros outside namespace is valid"
class ClassB;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
struct StructC {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
-char *VarD = MACRO_A;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
+const char *VarD = MACRO_A;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
typedef int typeE;
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
void funcF() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
namespace outer_most {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be
the 'LIBC_NAMESPACE' macro
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be
the 'LIBC_NAMESPACE_DECL' macro
class A {};
}
// Wrapped in anonymous namespace.
namespace {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE' namespace
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration must be enclosed
within the 'LIBC_NAMESPACE_DECL' namespace
class A {};
}
namespace namespaceG {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be
the 'LIBC_NAMESPACE' macro
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be
the 'LIBC_NAMESPACE_DECL' macro
namespace __llvm_libc {
namespace namespaceH {
class ClassB;
} // namespace namespaceH
struct StructC {};
} // namespace __llvm_libc
-char *VarD = MACRO_A;
+const char *VarD = MACRO_A;
typedef int typeE;
void funcF() {}
} // namespace namespaceG
// Wrapped in macro namespace but with an incorrect name
-#define LIBC_NAMESPACE custom_namespace
-namespace LIBC_NAMESPACE {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'LIBC_NAMESPACE' macro should
start with '__llvm_libc'
+#define LIBC_NAMESPACE_DECL [[gnu::visibility("hidden")]] custom_namespace
+namespace LIBC_NAMESPACE_DECL {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'LIBC_NAMESPACE_DECL' macro
expansion should start with '__llvm_libc'
+
namespace namespaceH {
class ClassB;
} // namespace namespaceH
-} // namespace LIBC_NAMESPACE
+} // namespace LIBC_NAMESPACE_DECL
-// 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 {
+// Wrapped in macro namespace with a valid name, LIBC_NAMESPACE_DECL starts
with '__llvm_libc'
+#undef LIBC_NAMESPACE_DECL
+#define LIBC_NAMESPACE_DECL [[gnu::visibility("hidden")]] __llvm_libc_xyz
+namespace LIBC_NAMESPACE_DECL {
namespace namespaceI {
class ClassB;
} // namespace namespaceI
struct StructC {};
-char *VarD = MACRO_A;
+const char *VarD = MACRO_A;
typedef int typeE;
void funcF() {}
extern "C" void extern_funcJ() {}
-} // namespace LIBC_NAMESPACE
+} // namespace LIBC_NAMESPACE_DECL
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
