[clang-tools-extra] [clang-tidy][libc] Fix namespace check with macro (PR #68134)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 5 12:57:33 PDT 2023
https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/68134
>From baf9d8e19d2b064c05527757c6173f875b59b286 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 3 Oct 2023 10:39:02 -0700
Subject: [PATCH 1/3] [clang-tidy][libc] Fix namespace check with macro
The name of the namespace for LLVM's libc is now provided by a macro.
The ImplementationNamespaceCheck was updated to handle this, but the
CalleeNamespaceCheck was missed. This patch updates the
CalleeNamespaceCheck to handle the macro.
---
.../clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
index 98ae857b589fd64..7ad4b5fb7f043ab 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -45,9 +45,10 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
if (FuncDecl->getBuiltinID() != 0)
return;
- // If the outermost namespace of the function is __llvm_libc, we're good.
+ // If the outermost namespace of the function starts with __llvm_libc, we're
+ // good.
const auto *NS = dyn_cast<NamespaceDecl>(getOutermostNamespace(FuncDecl));
- if (NS && NS->getName() == "__llvm_libc")
+ if (NS && NS->getName().starts_with("__llvm_libc"))
return;
const DeclarationName &Name = FuncDecl->getDeclName();
@@ -55,8 +56,9 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
IgnoredFunctions.contains(Name.getAsIdentifierInfo()->getName()))
return;
- diag(UsageSiteExpr->getBeginLoc(), "%0 must resolve to a function declared "
- "within the '__llvm_libc' namespace")
+ diag(UsageSiteExpr->getBeginLoc(),
+ "%0 must resolve to a function declared "
+ "within the '__llvm_libc' namespace (use macro `LIBC_NAMESPACE`)")
<< FuncDecl;
diag(FuncDecl->getLocation(), "resolves to this declaration",
>From b3c710beda5b45dc18a5cbd74e141c1169971450 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 3 Oct 2023 15:35:32 -0700
Subject: [PATCH 2/3] update release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8fc28c090341802..36cc58f4ab91b21 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -321,6 +321,11 @@ Changes in existing checks
<clang-tidy/checks/readability/static-accessed-through-instance>` check to
identify calls to static member functions with out-of-class inline definitions.
+- Improved :doc:`llvmlibc-callee-namespace
+ <clang-tidy/checks/llvmlibc/callee-namespace>` to support
+ customizable namespace. This matches the change made to implementation in
+ namespace.
+
Removed checks
^^^^^^^^^^^^^^
>From 8258d9861e3e0ed63adabed9fc584bd2a90739e2 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Thu, 5 Oct 2023 12:56:28 -0700
Subject: [PATCH 3/3] Correct docs, adjust checks, and add tests
---
.../llvmlibc/CalleeNamespaceCheck.cpp | 10 +++--
.../ImplementationInNamespaceCheck.cpp | 4 +-
.../clang-tidy/llvmlibc/NamespaceConstants.h | 16 +++++++
clang-tools-extra/docs/ReleaseNotes.rst | 9 ++--
.../checks/llvmlibc/callee-namespace.rst | 12 +++--
.../checkers/llvmlibc/callee-namespace.cpp | 44 ++++++++++++++-----
6 files changed, 68 insertions(+), 27 deletions(-)
create mode 100644 clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
index 7ad4b5fb7f043ab..bfd0abd3fe59ae1 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "CalleeNamespaceCheck.h"
+#include "NamespaceConstants.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -45,10 +46,11 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
if (FuncDecl->getBuiltinID() != 0)
return;
- // If the outermost namespace of the function starts with __llvm_libc, we're
- // good.
+ // If the outermost namespace of the function is a macro that starts with
+ // __llvm_libc, we're good.
const auto *NS = dyn_cast<NamespaceDecl>(getOutermostNamespace(FuncDecl));
- if (NS && NS->getName().starts_with("__llvm_libc"))
+ if (NS && Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) &&
+ NS->getName().starts_with(RequiredNamespaceStart))
return;
const DeclarationName &Name = FuncDecl->getDeclName();
@@ -58,7 +60,7 @@ void CalleeNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
diag(UsageSiteExpr->getBeginLoc(),
"%0 must resolve to a function declared "
- "within the '__llvm_libc' namespace (use macro `LIBC_NAMESPACE`)")
+ "within the namespace defined by the 'LIBC_NAMESPACE' macro")
<< FuncDecl;
diag(FuncDecl->getLocation(), "resolves to this declaration",
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
index bbc60217cdb1d5b..ae9819ed97502e8 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ImplementationInNamespaceCheck.h"
+#include "NamespaceConstants.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -14,9 +15,6 @@ using namespace clang::ast_matchers;
namespace clang::tidy::llvm_libc {
-const static StringRef RequiredNamespaceStart = "__llvm_libc";
-const static StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE";
-
void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
translationUnitDecl(
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
new file mode 100644
index 000000000000000..31fcdaa7b273b0b
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
@@ -0,0 +1,16 @@
+//===--- NamespaceConstants.h -----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/StringRef.h"
+
+namespace clang::tidy::llvm_libc {
+
+const static StringRef RequiredNamespaceStart = "__llvm_libc";
+const static StringRef RequiredNamespaceMacroName = "LIBC_NAMESPACE";
+
+} // namespace clang::tidy::llvm_libc
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 36cc58f4ab91b21..14f9e19c501025e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -241,6 +241,11 @@ Changes in existing checks
<clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
``inline`` namespaces in the same format as :program:`clang-format`.
+- Improved :doc:`llvmlibc-callee-namespace
+ <clang-tidy/checks/llvmlibc/callee-namespace>` to support
+ customizable namespace. This matches the change made to implementation in
+ namespace.
+
- Improved :doc:`llvmlibc-implementation-in-namespace
<clang-tidy/checks/llvmlibc/implementation-in-namespace>` to support
customizable namespace. This further allows for testing the libc when the
@@ -321,10 +326,6 @@ Changes in existing checks
<clang-tidy/checks/readability/static-accessed-through-instance>` check to
identify calls to static member functions with out-of-class inline definitions.
-- Improved :doc:`llvmlibc-callee-namespace
- <clang-tidy/checks/llvmlibc/callee-namespace>` to support
- customizable namespace. This matches the change made to implementation in
- namespace.
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/callee-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/callee-namespace.rst
index c072dd069e74795..ba742b5e1cab58a 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/callee-namespace.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/callee-namespace.rst
@@ -3,14 +3,18 @@
llvmlibc-callee-namespace
====================================
-Checks all calls resolve to functions within ``__llvm_libc`` namespace.
+Checks all calls resolve to functions within correct namespace.
.. code-block:: c++
- 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 {
// Allow calls with the fully qualified name.
- __llvm_libc::strlen("hello");
+ LIBC_NAMESPACE::strlen("hello");
// Allow calls to compiler provided functions.
(void)__builtin_abs(-1);
@@ -21,4 +25,4 @@ Checks all calls resolve to functions within ``__llvm_libc`` namespace.
// Disallow calling into functions in the global namespace.
::strlen("!");
- } // namespace __llvm_libc
+ } // namespace LIBC_NAMESPACE
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/callee-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/callee-namespace.cpp
index f18ab2b89e7861b..c867b92d5881c60 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/callee-namespace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/callee-namespace.cpp
@@ -1,6 +1,16 @@
// RUN: %check_clang_tidy %s llvmlibc-callee-namespace %t
+#define OTHER_MACRO_NAMESPACE custom_namespace
+namespace OTHER_MACRO_NAMESPACE {
+ void wrong_name_macro_func() {}
+}
+
namespace __llvm_libc {
+ void right_name_no_macro_func() {}
+}
+
+#define LIBC_NAMESPACE __llvm_libc_xyz
+namespace LIBC_NAMESPACE {
namespace nested {
void nested_func() {}
} // namespace nested
@@ -22,12 +32,12 @@ struct global_struct {
int operator()() const { return 0; }
};
-namespace __llvm_libc {
+namespace LIBC_NAMESPACE {
void Test() {
// Allow calls with the fully qualified name.
- __llvm_libc::libc_api_func();
- __llvm_libc::nested::nested_func();
- void (*qualifiedPtr)(void) = __llvm_libc::libc_api_func;
+ LIBC_NAMESPACE::libc_api_func();
+ LIBC_NAMESPACE::nested::nested_func();
+ void (*qualifiedPtr)(void) = LIBC_NAMESPACE::libc_api_func;
qualifiedPtr();
// Should not trigger on compiler provided function calls.
@@ -36,22 +46,22 @@ void Test() {
// Bare calls are allowed as long as they resolve to the correct namespace.
libc_api_func();
nested::nested_func();
- void (*barePtr)(void) = __llvm_libc::libc_api_func;
+ void (*barePtr)(void) = LIBC_NAMESPACE::libc_api_func;
barePtr();
// Allow calling entities defined in the namespace.
- __llvm_libc::libc_api_struct{}();
+ LIBC_NAMESPACE::libc_api_struct{}();
// Disallow calling into global namespace for implemented entrypoints.
::libc_api_func();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
- // CHECK-MESSAGES: :15:6: note: resolves to this declaration
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'libc_api_func' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro
+ // CHECK-MESSAGES: :25:6: note: resolves to this declaration
// Disallow indirect references to functions in global namespace.
void (*badPtr)(void) = ::libc_api_func;
badPtr();
- // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the '__llvm_libc' namespace
- // CHECK-MESSAGES: :15:6: note: resolves to this declaration
+ // CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 'libc_api_func' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro
+ // CHECK-MESSAGES: :25:6: note: resolves to this declaration
// Allow calling into global namespace for specific functions.
::malloc();
@@ -59,8 +69,18 @@ void Test() {
// Disallow calling on entities that are not in the namespace, but make sure
// no crashes happen.
global_struct{}();
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'operator()' must resolve to a function declared within the '__llvm_libc' namespace
- // CHECK-MESSAGES: :22:7: note: resolves to this declaration
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'operator()' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro
+ // CHECK-MESSAGES: :32:7: note: resolves to this declaration
+
+ OTHER_MACRO_NAMESPACE::wrong_name_macro_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wrong_name_macro_func' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro
+ // CHECK-MESSAGES: :3:31: note: expanded from macro 'OTHER_MACRO_NAMESPACE'
+ // CHECK-MESSAGES: :5:8: note: resolves to this declaration
+
+ __llvm_libc::right_name_no_macro_func();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'right_name_no_macro_func' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro
+ // CHECK-MESSAGES: :9:8: note: resolves to this declaration
+
}
} // namespace __llvm_libc
More information about the cfe-commits
mailing list