[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

27

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

27–36

Please implement constructor in the .cpp file.

39–41

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

43–51

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

2

I believe this can be removed.

20

Missing check name:

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

45

Missing line and column info

[carlosgalvezp]

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

8

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

Remove

3

Remove

4

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)
+      : ClangTidyCheck(Name, Context),
+        RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+            "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+    if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                    HeaderFileExtensions,
+                                    utils::defaultFileExtensionDelimiters())) {
+      this->configurationDiag("Invalid header file extension: '%0'")
+          << RawStringHeaderFileExtensions;
+    }
+  }
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
+    Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+    if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                    HeaderFileExtensions,
+                                    utils::defaultFileExtensionDelimiters())) {
+      this->configurationDiag("Invalid header file extension: '%0'")
+          << RawStringHeaderFileExtensions;
+    }
+  }
+
+  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 {
+
+void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
+}
+
+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->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-check");
     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 `llvmlibc-inline-function-decl-check <http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc/inline-function-decl-check.html>`_
+check.
+
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Removed checks
 ^^^^^^^^^^^^^^

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

+.. title:: clang-tidy - llvmlibc-inline-funciton-decl-check
+
+llvmlibc-inline-function-decl-check
+====================================
+
+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.

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

+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN:   -- -header-filter=.* -- -I %S
+
+#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
+  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
+  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
+
+  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
+
+  LIBC_INLINE bool operator==(const MyClass &RHS) {
+    return RHS.A == A;
+  }
+
+  static int getVal(const MyClass &V) {
+  // CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE macro
+    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
+    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
+    return V.A *= A;
+  }
+};
+
+} // namespace __llvm_libc
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H