[clang-tidy][libc] Add an inline function checker for the libc project.

[sivachandra]

Summary

The new checker checks if inline functions defined in header files are
tagged with the LIBC_INLINE macro. See https://libc.llvm.org/code_style.html
for more information about this macro.

[Eugene.Zelenko]

Please add entry in Release Notes.

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

30

Please don't use auto unless type is explicitly stated in same statement or iterator.

[sivachandra]

Use explicit type instead of auto.

[PiotrZSL]

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

32

Probably better would be to check if this isn't main source Result.SourceManager->isInMainFile(SrcBegin)

36

Maybe put this LIBC_INLINE into some configuration, so check could be used by someone else also for other macros.

[PiotrZSL]

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

21

or maybe even better:
Finder->addMatcher(functionDecl(unless(isExpansionInMainFile()), isInline()).bind("func_decl"), this);
Instead of line 26 and 32.

[carlosgalvezp]

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

21

I'm not sure that works - if we pass a header directly to clang-tidy, it will consider it as the "main file", right? That's why the established pattern is based on HeaderFileExtensions, please check out other checks.

30

We have a well-established pattern for detecting code in headers, grep for HeaderFileExtensions in existing checks.

[PiotrZSL]

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

21

Yes you right, but isInline still can be checked here.
As for HeaderFileExtensions never used it from both developer and user point of view.
When running clang-tidy on headers is still better simply create file with single include.
Maybe if there would be AST_MATCHER for HeaderFileExtensions.

[carlosgalvezp]

clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp

18–19 ↗

(On Diff #492324)

These should specify which line they apply to, as done in all other checks.

In particular, to get the unit test to work, you will not be able to use a .cpp that includes a header. The test file must be a .hpp file (for which you need HeaderFileExtensions as I said above).

[sivachandra]

Use the HeaderFileExtensions pattern to match constructs in header files.

[sivachandra]

Update release notes.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

104

One sentence is enough.

[sivachandra]

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

21

Unfortunately, even isInline is not good enough because isInline only matches functions marked with inline: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L7769. It misses implicitly inline functions like constexpr functions and class methods.

30

Thanks! I have now switched over to use the HeaderFileExtensions pattern.

[sivachandra]

Limit the description in the release notes to one sentence.

[sivachandra]

Limit the description in the release notes to one sentence.

[carlosgalvezp]

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

28

Check if FuncDecl is not a nullptr before dereferencing. You can do an early return like:

if (!FuncDecl)
  return;

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

28–37

Please implement constructor in the .cpp file.

40–42

This is OK to keep in the header as it's only one line.

44–52

Ditto

clang-tools-extra/docs/ReleaseNotes.rst

100

I believe you'll need a rebase since a new check was recently added.

104

You need to also add the check to clang-tools-extra/docs/clang-tidy/checks/list.rst

clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

3

I believe this can be removed.

21

Missing check name:

... LIBC_INLINE macro [llvmlibc-inline-function-decl]

46

Missing line and column info

[carlosgalvezp]

clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst

8 ↗

(On Diff #492661)

Document the HeaderFileExtensions option, see existing checks.

[carlosgalvezp]

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

28

Remove

clang-tools-extra/docs/ReleaseNotes.rst

100

Remove

100

Remove

clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst

1 ↗

(On Diff #492661)

Remove

3 ↗

(On Diff #492661)

Remove

4 ↗

(On Diff #492661)

Nit: remove extra =

[sivachandra]

Address comments.

[carlosgalvezp]

LGTM, thanks for the contribution! Please wait a few days for other reviewers to have a look.

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

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyLLVMLibcModule
   CalleeNamespaceCheck.cpp
   ImplementationInNamespaceCheck.cpp
+  InlineFunctionDeclCheck.cpp
   LLVMLibcTidyModule.cpp
   RestrictSystemLibcHeadersCheck.cpp
 
   LINK_LIBS
   clangTidy
   clangTidyPortabilityModule
   clangTidyUtils
 

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

+//===-- InlineFunctionDeclCheck.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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_INLINEFUNCTIONDECLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_INLINEFUNCTIONDECLCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/FileExtensionsUtils.h"
+
+namespace clang::tidy::llvm_libc {
+
+/// Checks that explicitly and implicitly inline functions in headers files
+/// are tagged with the LIBC_INLINE macro.
+///
+/// For more information about the LIBC_INLINE macro, see
+/// https://libc.llvm.org/code_style.html.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc/inline-function-decl-check.html
+class InlineFunctionDeclCheck : public ClangTidyCheck {
+public:
+  InlineFunctionDeclCheck(StringRef Name, ClangTidyContext *Context);
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const StringRef RawStringHeaderFileExtensions;
+  utils::FileExtensionsSet HeaderFileExtensions;
+};
+
+} // namespace clang::tidy::llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_INLINEFUNCTIONDECLCHECK_H

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

+//===-- InlineFunctionDeclCheck.cpp ---------------------------------------===//
+//
+// 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 "InlineFunctionDeclCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include "llvm/ADT/StringSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::llvm_libc {
+
+InlineFunctionDeclCheck::InlineFunctionDeclCheck(StringRef Name,
+                                                 ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+          "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                  HeaderFileExtensions,
+                                  utils::defaultFileExtensionDelimiters())) {
+    this->configurationDiag("Invalid header file extension: '%0'")
+        << RawStringHeaderFileExtensions;
+  }
+}
+
+void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
+}
+
+void InlineFunctionDeclCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                  HeaderFileExtensions,
+                                  utils::defaultFileExtensionDelimiters())) {
+    this->configurationDiag("Invalid header file extension: '%0'")
+        << RawStringHeaderFileExtensions;
+  }
+}
+
+void InlineFunctionDeclCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");
+
+  // Consider only explicitly or implicitly inline functions.
+  if (FuncDecl == nullptr || !FuncDecl->isInlined())
+    return;
+
+  SourceLocation SrcBegin = FuncDecl->getBeginLoc();
+  // Consider functions only in header files.
+  if (!utils::isSpellingLocInHeaderFile(SrcBegin, *Result.SourceManager,
+                                        HeaderFileExtensions))
+    return;
+
+  // Check if decl starts with LIBC_INLINE
+  auto Loc = FullSourceLoc(Result.SourceManager->getFileLoc(SrcBegin),
+                           *Result.SourceManager);
+  llvm::StringRef SrcText = Loc.getBufferData().drop_front(Loc.getFileOffset());
+  if (SrcText.starts_with("LIBC_INLINE"))
+    return;
+
+  diag(SrcBegin, "%0 must be tagged with the LIBC_INLINE macro; the macro "
+                 "should be placed at the beginning of the declaration")
+      << FuncDecl;
+}
+
+} // namespace clang::tidy::llvm_libc

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 "CalleeNamespaceCheck.h"
 #include "ImplementationInNamespaceCheck.h"
+#include "InlineFunctionDeclCheck.h"
 #include "RestrictSystemLibcHeadersCheck.h"
 
 namespace clang::tidy {
 namespace llvm_libc {
 
 class LLVMLibcModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<CalleeNamespaceCheck>(
         "llvmlibc-callee-namespace");
     CheckFactories.registerCheck<ImplementationInNamespaceCheck>(
         "llvmlibc-implementation-in-namespace");
+    CheckFactories.registerCheck<InlineFunctionDeclCheck>(
+        "llvmlibc-inline-function-decl");
     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.");
 
 } // namespace llvm_libc
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the LLVMLibcModule.
 volatile int LLVMLibcModuleAnchorSource = 0;
 
 } // namespace clang::tidy

clang-tools-extra/docs/ReleaseNotes.rst

 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 New checks
 ^^^^^^^^^^
 
 - New :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` check.
 
   Checks for functions that have safer, more secure replacements available, or
   are considered deprecated due to design flaws.
   This check relies heavily on, but is not exclusive to, the functions from
   the *Annex K. "Bounds-checking interfaces"* of C11.
 
 - New :doc:`cppcoreguidelines-avoid-capture-default-when-capturing-this
   <clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this>` check.
 
   Warns when lambda specify a capture default and capture ``this``.
 
+- New :doc: `llvmlibc-inline-function-decl
+  <clang-tidy/checks/llvmlibc/inline-function-decl>` check.
+
+  Checks that all implicit and explicit inline functions in header files are
+  tagged with the ``LIBC_INLINE`` macro.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc24-c
   <clang-tidy/checks/cert/msc24-c>` to :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` was added.
 
 - New alias :doc:`cert-msc33-c

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

.. csv-table::
    `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-callee-namespace <llvmlibc/callee-namespace.html>`_,
    `llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace.html>`_,
+   `llvmlibc-inline-function-decl <llvmlibc/inline-function-decl.html>`_,
    `llvmlibc-restrict-system-libc-headers <llvmlibc/restrict-system-libc-headers.html>`_, "Yes"
    `misc-confusable-identifiers <misc/confusable-identifiers.html>`_,
    `misc-const-correctness <misc/const-correctness.html>`_, "Yes"
    `misc-definitions-in-headers <misc/definitions-in-headers.html>`_, "Yes"
    `misc-misleading-bidirectional <misc/misleading-bidirectional.html>`_,
    `misc-misleading-identifier <misc/misleading-identifier.html>`_,
    `misc-misplaced-const <misc/misplaced-const.html>`_,
    `misc-new-delete-overloads <misc/new-delete-overloads.html>`_,

clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst

+.. title:: clang-tidy - llvmlibc-inline-function-decl
+
+llvmlibc-inline-function-decl
+=============================
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro. See the `libc style guide
+<https://libc.llvm.org/code_style.html>`_ for more information about this macro.
+
+Options
+-------
+
+.. option:: HeaderFileExtensions
+
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no
+   other desired extensions) or leave an empty element in the list. E.g.,
+   "h,hh,hpp,hxx," (note the trailing comma).

clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl %t
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H
+
+#define LIBC_INLINE inline
+
+namespace __llvm_libc {
+
+LIBC_INLINE int addi(int a, int b) {
+  return a + b;
+}
+
+LIBC_INLINE constexpr long addl(long a, long b) {
+  return a + b;
+}
+
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  return a + b;
+}
+
+inline unsigned long addul(unsigned long a, unsigned long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  return a + b;
+}
+
+class  MyClass {
+  int A;
+public:
+  MyClass() : A(123) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+  LIBC_INLINE MyClass(int V) : A(V) {}
+
+  constexpr operator int() const { return A; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+  LIBC_INLINE bool operator==(const MyClass &RHS) {
+    return RHS.A == A;
+  }
+
+  static int getVal(const MyClass &V) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'getVal' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+    return V.A;
+  }
+
+  LIBC_INLINE static void setVal(MyClass &V, int A) {
+    V.A = A;
+  }
+
+  constexpr static int addInt(MyClass &V, int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+    return V.A += A;
+  }
+
+  static LIBC_INLINE int mulInt(MyClass &V, int A) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'mulInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+    return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H