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 <michae...@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 <michae...@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 <michae...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits