[clang-tidy] Add check to ensure llvm-libc implementations are defined in correct namespace.

[sivachandra]

Instead of this narrow check, what we really want is a check to ensure that all implementation detail resides in the namespace __llvm_libc. Anything outside would be special and requiring a NOLINT... for them is reasonable. Have you considered such an approach?

[njames93]

This check should only worry about the first declaration of the function, any redecls or the definition could appear outside the namespace like this:

namespace blah{
    void foo();
}
void blah::foo();
void blah::foo(){}

There are some missing test cases I'd like to see like
wrapping in an enclosing namespace (including anonymous)

namespace __llvm_libc {
namespace <optional name> {
// impl
}
}

namespace <optional name> {
namesapce __llvm_libc {
// impl
}
}

As I don't work on libc I'm not sure how these should be handled, maybe its fine if there is a corresponding inline namespace.

clang-tools-extra/clang-tidy/llvmlibc/EntrypointNamespaceCheck.cpp

67

Is there a rule that all libc implementation headers must have the extension .h. If not there is utils::FileExtensionSet that could be used.
Alternatively you could just check to see if the SourceLocation is in the main file
if (!Result.SourceManager->isInMainFile(MatchedDecl->getLocation())

71

To save fetching the option each time, why not just store the required namespace in the class and initialize it in the constructor

[PaulkaToast]

Abandoning for a more generalized check. D76818

[PaulkaToast]

clang-tools-extra/clang-tidy/llvmlibc/EntrypointNamespaceCheck.cpp

67

Thanks for this! I incorporated your recommendations in the new patch. (:

clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyLLVMLibcModule
+  EntrypointNamespaceCheck.cpp
   LLVMLibcTidyModule.cpp
   RestrictSystemLibcHeadersCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyPortabilityModule
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/clang-tidy/llvmlibc/EntrypointNamespaceCheck.h

+//===--- EntrypointNamespaceCheck.h - clang-tidy ----------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_ENTRYPOINTNAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_ENTRYPOINTNAMESPACECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm_libc {
+
+/// Checks that llvmlibc implentations are within the correct namespace.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc-entrypoint-namespace.html
+class EntrypointNamespaceCheck : public ClangTidyCheck {
+public:
+  EntrypointNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  ast_matchers::MatchFinder *Finder;
+};
+
+} // namespace llvm_libc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_ENTRYPOINTNAMESPACECHECK_H

clang-tools-extra/clang-tidy/llvmlibc/EntrypointNamespaceCheck.cpp

+//===--- EntrypointNamespaceCheck.cpp - clang-tidy ------------------------===//
+//
+// 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 "EntrypointNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/MacroArgs.h"
+#include "clang/Lex/PPCallbacks.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_libc {
+
+namespace {
+class EntrypointPPCallbacks : public PPCallbacks {
+public:
+  EntrypointPPCallbacks(EntrypointNamespaceCheck &Check, MatchFinder *Finder,
+                        std::string EntrypointMacro)
+      : Check(Check), Finder(Finder), EntrypointMacro(EntrypointMacro) {}
+
+  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
+                    SourceRange Range, const MacroArgs *Args) override;
+
+private:
+  EntrypointNamespaceCheck &Check;
+  MatchFinder *Finder;
+  std::string EntrypointMacro;
+};
+
+void EntrypointPPCallbacks::MacroExpands(const Token &MacroNameTok,
+                                         const MacroDefinition &MD,
+                                         SourceRange Range,
+                                         const MacroArgs *Args) {
+  if (MacroNameTok.getIdentifierInfo()->getName() != EntrypointMacro)
+    return;
+
+  // Get the name of the function passed to macro and register a matcher.
+  StringRef Name = Args->getUnexpArgument(0)->getIdentifierInfo()->getName();
+  Finder->addMatcher(functionDecl(hasName(Name)).bind("libc_entry"), &Check);
+}
+} // End of anonymous namespace.
+
+void EntrypointNamespaceCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  std::string EntrypointMacro =
+      Options.get("EntrypointMacro", "LLVM_LIBC_ENTRYPOINT");
+  PP->addPPCallbacks(
+      std::make_unique<EntrypointPPCallbacks>(*this, Finder, EntrypointMacro));
+}
+
+void EntrypointNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+  this->Finder = Finder;
+}
+
+void EntrypointNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("libc_entry");
+
+  // Ignore declarations from header files.
+  if (Result.SourceManager->getFilename(MatchedDecl->getLocation())
+          .endswith(".h"))
+    return;
+
+  std::string RequiredNamespace =
+      Options.get("RequiredNamespace", "__llvm_libc");
+  const DeclContext *EnclosingDecl =
+      MatchedDecl->getEnclosingNamespaceContext();
+  // If there is no namespace, translation unit is returned.
+  if (!EnclosingDecl->isNamespace()) {
+    diag(MatchedDecl->getLocation(),
+         "function %0 is not defined in a namespace, please wrap implentation "
+         "in '%1' namespace.")
+        << MatchedDecl << RequiredNamespace;
+    return;
+  }
+
+  const NamespaceDecl *Namespace = cast<NamespaceDecl>(EnclosingDecl);
+  if (Namespace->getName() != RequiredNamespace) {
+    diag(MatchedDecl->getLocation(),
+         "function %0 is defined in namespace %1, should be in '%2' namespace.")
+        << MatchedDecl << Namespace << RequiredNamespace;
+  }
+}
+
+} // namespace llvm_libc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp

 //===--- LLVMLibcTidyModule.cpp - clang-tidy ------------------------------===//
 //
 // 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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "EntrypointNamespaceCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace llvm_libc {
 
 class LLVMLibcModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<EntrypointNamespaceCheck>(
+        "llvmlibc-entrypoint-namespace");
     CheckFactories.registerCheck<RestrictSystemLibcHeadersCheck>(
         "llvmlibc-restrict-system-libc-headers");
   }
 };
 
 // Register the LLVMLibcTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<LLVMLibcModule>
     X("llvmlibc-module", "Adds LLVM libc standards checks.");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
   result of a memory allocation function (``malloc()``, ``calloc()``,
   ``realloc()``, ``alloca()``) instead of its argument.
 
 - New :doc:`bugprone-spuriously-wake-up-functions
   <clang-tidy/checks/bugprone-spuriously-wake-up-functions>` check.
 
   Finds ``cnd_wait``, ``cnd_timedwait``, ``wait``, ``wait_for``, or
   ``wait_until`` function calls when the function is not invoked from a loop
-  that checks whether a condition predicate holds or the function has a 
+  that checks whether a condition predicate holds or the function has a
   condition parameter.
 
 - New :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` check.
 
   Checks for usages of identifiers reserved for use by the implementation.
 
 - New :doc:`bugprone-suspicious-include
   <clang-tidy/checks/bugprone-suspicious-include>` check.
 
   Finds cases where an include refers to what appears to be an implementation
   file, which often leads to hard-to-track-down ODR violations, and diagnoses
   them.
 
 - New :doc:`cert-oop57-cpp
   <clang-tidy/checks/cert-oop57-cpp>` check.
 
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
+- New :doc:`llvmlibc-entrypoint-namespace
+  <clang-tidy/checks/llvmlibc-entrypoint-namespace>` check.
+
+  Finds where llvm-libc entrypoint macro is called and checks that it is wrapped
+  in the correct namespace.
+
 - New :doc:`llvmlibc-restrict-system-libc-headers
   <clang-tidy/checks/llvmlibc-restrict-system-libc-headers>` check.
 
   Finds includes of system libc headers not provided by the compiler within
   llvm-libc implementations.
 
 - New :doc:`objc-dealloc-in-category
   <clang-tidy/checks/objc-dealloc-in-category>` check.
- New alias :doc:`cert-dcl51-cpp
   <clang-tidy/checks/cert-dcl51-cpp>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`readability-qualified-auto
-  <clang-tidy/checks/readability-qualified-auto>` check now supports a 
+  <clang-tidy/checks/readability-qualified-auto>` check now supports a
   `AddConstToQualified` to enable adding ``const`` qualifiers to variables
   typed with ``auto *`` and ``auto &``.
 
 - Improved :doc:`readability-redundant-string-init
   <clang-tidy/checks/readability-redundant-string-init>` check now supports a
   `StringNames` option enabling its application to custom string classes. The
   check now detects in class initializers and constructor initializers which
   are deemed to be redundant.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `hicpp-signed-bitwise <hicpp-signed-bitwise.html>`_,
    `linuxkernel-must-use-errs <linuxkernel-must-use-errs.html>`_,
    `llvm-header-guard <llvm-header-guard.html>`_,
    `llvm-include-order <llvm-include-order.html>`_, "Yes"
    `llvm-namespace-comment <llvm-namespace-comment.html>`_,
    `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes"
    `llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes"
    `llvm-twine-local <llvm-twine-local.html>`_, "Yes"
+   `llvmlibc-entrypoint-namespace <llvmlibc-entrypoint-namespace.html>`_,
    `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, "Yes"
    `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
    `misc-misplaced-const <misc-misplaced-const.html>`_,
    `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `misc-no-recursion <misc-no-recursion.html>`_,
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/llvmlibc-entrypoint-namespace.rst

+.. title:: clang-tidy - llvmlibc-entrypoint-namespace
+
+llvmlibc-entrypoint-namespace
+=============================
+
+Finds where llvm-libc entrypoint macro is called and checks that it is wrapped
+in the correct namespace.
+
+.. code-block:: c++
+
+    // Correct: entrypoint inside correct namespace.
+    namespace __llvm_libc {
+    void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+    }
+
+    // Incorrect: entrypoint not in a namespace.
+    void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+
+    // Incorrect: entrypoint inside incorrect namespace.
+    namespace something_else {
+    void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+    }
+
+Options
+-------
+
+.. option:: EntrypointMacro
+
+    The name of the macro used when defining llvm-libc implementations. The
+    default is `LLVM_LIBC_ENTRYPOINT`.
+
+.. option:: RequiredNamespace
+
+    The namespace that llvm-libc implementations must be wrapped in. The default
+    is `__llvm_libc`.

clang-tools-extra/test/clang-tidy/checkers/llvmlibc-entrypoint-namespace.cpp

+// RUN: %check_clang_tidy %s llvmlibc-entrypoint-namespace %t
+
+#define LLVM_LIBC_ENTRYPOINT(name) name
+#define SOMETHING_ELSE(name) name
+
+// use of entrypoint macro with correct namespace.
+namespace __llvm_libc {
+void LLVM_LIBC_ENTRYPOINT(correct_entrypoint)(char *param) {}
+} // namespace __llvm_libc
+
+// different macros are ignored.
+namespace something_else {
+void SOMETHING_ELSE(different_macro)(char *param) {}
+} // namespace something_else
+
+// Not in a namespace.
+void LLVM_LIBC_ENTRYPOINT(missing_namespace)(char *param) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: function 'missing_namespace' is not defined in a namespace, please wrap implentation in '__llvm_libc' namespace.
+
+// Inside incorrect namespace.
+namespace something_else {
+void LLVM_LIBC_ENTRYPOINT(incorrect_namespace)(char *param) {}
+} // namespace something_else
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: function 'incorrect_namespace' is defined in namespace 'something_else', should be in '__llvm_libc' namespace.