[clang-tools-extra] daca972 - [clang-tidy][libc] Fix namespace check with macro (#68134)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 6 14:06:25 PDT 2023
Author: michaelrj-google
Date: 2023-10-06T14:06:22-07:00
New Revision: daca97216cf132d733513f992d49e3c722aabf40
URL: https://github.com/llvm/llvm-project/commit/daca97216cf132d733513f992d49e3c722aabf40
DIFF: https://github.com/llvm/llvm-project/commit/daca97216cf132d733513f992d49e3c722aabf40.diff
LOG: [clang-tidy][libc] Fix namespace check with macro (#68134)
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.
Added:
clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h
Modified:
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/llvmlibc/callee-namespace.rst
clang-tools-extra/test/clang-tidy/checkers/llvmlibc/callee-namespace.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
index 98ae857b589fd64..af977bff70f7c2a 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -7,9 +7,11 @@
//===----------------------------------------------------------------------===//
#include "CalleeNamespaceCheck.h"
+#include "NamespaceConstants.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/StringSet.h"
using namespace clang::ast_matchers;
@@ -45,9 +47,11 @@ 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 is a macro that starts with
+ // __llvm_libc, we're good.
const auto *NS = dyn_cast<NamespaceDecl>(getOutermostNamespace(FuncDecl));
- if (NS && NS->getName() == "__llvm_libc")
+ if (NS && Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) &&
+ NS->getName().starts_with(RequiredNamespaceStart))
return;
const DeclarationName &Name = FuncDecl->getDeclName();
@@ -55,9 +59,10 @@ 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")
- << FuncDecl;
+ diag(UsageSiteExpr->getBeginLoc(),
+ "%0 must resolve to a function declared "
+ "within the namespace defined by the '%1' macro")
+ << FuncDecl << RequiredNamespaceMacroName;
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 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 0ecf786e73bc413..c1b926b296b055a 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
@@ -319,6 +324,7 @@ 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.
+
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