[clang-tidy] Fix `llvmlibc-inline-function-decl` false positives for templated function definitions

[AMS21]

Summary

For a declaration the FunctionDecl begin location does not include the
template parameter lists, but for some reason if you have a separate
definitions to the declaration the begin location does include them.
With this patch we now correctly handle that case.

This fixes llvm#62746

[PiotrZSL]

I got one concern, if you write this:

template <typename... Ts>LIBC_INLINE void VariadicTemplate<Ts...>::goodVariadicTemplate() {}
template <typename... Ts>inline void VariadicTemplate<Ts...>::badVariadicTemplate() {}

Aka, no space after >, warning will be emitted for both.
That's because of TemplateParams->getRAngleLoc().getLocWithOffset(1);
So we find > character, then we skip it, now we pointing into inline or whitespace location, and when we skip to a next token.
If you remove this getLocWithOffset(1) then findNextTokenSkippingComments should work correctly.

Additionally you missing release notes.
There is also other way to deal with this issue, simply use FunctionDecl::getReturnTypeSourceRange() and work with tidy::utils::lexer::getPreviousToken.
But in theory both solutions should work fine. Question is also how it will work with attributes, like [[nodiscard]] that could be put before inline.

Requesting change mainly due to false-positive with space.

[AMS21]

Implemented suggested fix and added a test case
Added entry to ReleaseNotes

[AMS21]

Regarding the problems with attributes like nodiscard. I'm honestly not quite sure, but from the way the check is implemented and the warning is worded, it sounds to me like the attributes should come after LIBC_INLINE. Although looking at the Styleguide for libc, I didn't find anything the placement of the macro there.

I thinks its best for now to leave the behavior as it was.

[PiotrZSL]

LGTM

[AMS21]

If there are no more problems, I would kindly as for someone to push this on my behalf :)

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 "../utils/FileExtensionsUtils.h"
+#include "../utils/LexerUtils.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 {
 
+namespace {
+
+const TemplateParameterList *
+getLastTemplateParameterList(const FunctionDecl *FuncDecl) {
+  const TemplateParameterList *ReturnList =
+      FuncDecl->getDescribedTemplateParams();
+
+  if (!ReturnList) {
+    const unsigned NumberOfTemplateParameterLists =
+        FuncDecl->getNumTemplateParameterLists();
+
+    if (NumberOfTemplateParameterLists > 0)
+      ReturnList = FuncDecl->getTemplateParameterList(
+          NumberOfTemplateParameterLists - 1);
+  }
+
+  return ReturnList;
+}
+
+} // namespace
+
 InlineFunctionDeclCheck::InlineFunctionDeclCheck(StringRef Name,
                                                  ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       HeaderFileExtensions(Context->getHeaderFileExtensions()) {}
 
 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 == nullptr || !FuncDecl->isInlined())
     return;
 
   SourceLocation SrcBegin = FuncDecl->getBeginLoc();
+
+  // If we have a template parameter list, we need to skip that because the
+  // LIBC_INLINE macro must be placed after that.
+  if (const TemplateParameterList *TemplateParams =
+          getLastTemplateParameterList(FuncDecl)) {
+    SrcBegin = TemplateParams->getRAngleLoc();
+    std::optional<Token> NextToken =
+        utils::lexer::findNextTokenSkippingComments(
+            SrcBegin, *Result.SourceManager, Result.Context->getLangOpts());
+    if (NextToken)
+      SrcBegin = NextToken->getLocation();
+  }
+
   // Consider functions only in header files.
   if (!utils::isSpellingLocInHeaderFile(SrcBegin, *Result.SourceManager,
                                         HeaderFileExtensions))
     return;
 
   // Ignore lambda functions as they are internal and implicit.
-  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl)) {
+  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
     if (MethodDecl->getParent()->isLambda())
       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/docs/ReleaseNotes.rst

- Fixed an issue in :doc:`google-readability-avoid-underscore-in-googletest-name
   <clang-tidy/checks/google/readability-avoid-underscore-in-googletest-name>` when using
   ``DISABLED_`` in the test suite name.
 
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`llvm-header-guard
   <clang-tidy/checks/llvm/header-guard>` check.
   Global options of the same name should be used instead.
 
+- Fix false positive in :doc:`llvmlibc-inline-function-decl
+  <clang-tidy/checks/llvmlibc/inline-function-decl>` when using templated
+  function with separate declarations and definitions.
+
 - Improved the performance of the :doc:`misc-confusable-identifiers
   <clang-tidy/checks/misc/confusable-identifiers>` check through optimizations.
 
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`misc-definitions-in-headers
   <clang-tidy/checks/misc/definitions-in-headers>` check.
   Global options of the same name should be used instead.
 

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

 LIBC_INLINE void lambda() {
 // CHECK-MESSAGES-NOT: :[[@LINE+4]]:3: warning: '__invoke' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
 // CHECK-MESSAGES-NOT: :[[@LINE+3]]:3: warning: 'operator void (*)()' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
 // CHECK-MESSAGES-NOT: :[[@LINE+2]]:3: warning: '~(lambda at [[FILENAME:.+]])' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
 // CHECK-MESSAGES-NOT: :[[@LINE+1]]:6: warning: 'operator()' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
   [](){};
 }
 
+namespace issue_62746 {
+
+void goodSimpleFunction();
+void badSimpleFunction();
+void badSimpleFunctionWrongLocation();
+
+LIBC_INLINE void goodSimpleFunction() {}
+
+inline void badSimpleFunction() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badSimpleFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+void LIBC_INLINE badSimpleFunctionWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badSimpleFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T>
+void goodTemplateFunction();
+template <typename T>
+void badTemplateFunction();
+template <typename T>
+void badTemplateFunctionWrongLocation();
+
+template <typename T> LIBC_INLINE void goodTemplateFunction() {}
+
+template <typename T> inline void badTemplateFunction() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badTemplateFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> void LIBC_INLINE badTemplateFunctionWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badTemplateFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts>
+void goodVariadicFunction();
+template <typename... Ts>
+void badVariadicFunction();
+template <typename... Ts>
+void badVariadicFunctionWrongLocation();
+
+template <typename... Ts> LIBC_INLINE void goodVariadicFunction() {}
+
+template <typename... Ts> inline void badVariadicFunction() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> void LIBC_INLINE badVariadicFunctionWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+struct NoTemplate {
+  void goodNoTemplate();
+  void badNoTemplate();
+  void badNoTemplateWrongLocation();
+
+  template <typename T>
+  void goodNestedTemplate();
+  template <typename T>
+  void badNestedTemplate();
+  template <typename T>
+  void badNestedTemplateWrongLocation();
+
+  template <typename... Ts>
+  void goodVariadicTemplate();
+  template <typename... Ts>
+  void badVariadicTemplate();
+  template <typename... Ts>
+  void badVariadicTemplateWrongLocation();
+
+};
+
+LIBC_INLINE void NoTemplate::goodNoTemplate() {}
+
+inline void NoTemplate::badNoTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badNoTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+void LIBC_INLINE NoTemplate::badNoTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badNoTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> LIBC_INLINE void NoTemplate::goodNestedTemplate() {}
+
+template <typename T> inline void NoTemplate::badNestedTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badNestedTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> void LIBC_INLINE NoTemplate::badNestedTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badNestedTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> LIBC_INLINE void NoTemplate::goodVariadicTemplate() {}
+
+template <typename... Ts> void inline NoTemplate::badVariadicTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> void LIBC_INLINE NoTemplate::badVariadicTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T>
+struct SimpleTemplate {
+  void goodSimpleTemplate();
+  void badSimpleTemplate();
+  void badSimpleTemplateWrongLocation();
+
+  template <typename U>
+  void goodNestedTemplate();
+  template <typename U>
+  void badNestedTemplate();
+  template <typename U>
+  void badNestedTemplateWrongLocation();
+
+  template <typename... Ts>
+  void goodNestedVariadicTemplate();
+  template <typename... Ts>
+  void badNestedVariadicTemplate();
+  template <typename... Ts>
+  void badNestedVariadicTemplateWrongLocation();
+};
+
+template <typename T> LIBC_INLINE void SimpleTemplate<T>::goodSimpleTemplate() {}
+
+template <typename T> inline void SimpleTemplate<T>::badSimpleTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badSimpleTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> void LIBC_INLINE SimpleTemplate<T>::badSimpleTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badSimpleTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> template <typename U> LIBC_INLINE void SimpleTemplate<T>::goodNestedTemplate() {}
+
+template <typename T> template <typename U> inline void SimpleTemplate<T>::badNestedTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'badNestedTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> template <typename U> void LIBC_INLINE SimpleTemplate<T>::badNestedTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'badNestedTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> template <typename... Ts> LIBC_INLINE void SimpleTemplate<T>::goodNestedVariadicTemplate() {}
+
+template <typename T> template <typename... Ts> inline void SimpleTemplate<T>::badNestedVariadicTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: 'badNestedVariadicTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T> template <typename... Ts> void LIBC_INLINE SimpleTemplate<T>::badNestedVariadicTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: 'badNestedVariadicTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts>
+struct VariadicTemplate {
+  void goodVariadicTemplate();
+  void badVariadicTemplate();
+  void badVariadicTemplateWrongLocation();
+
+  template <typename U>
+  void goodNestedTemplate();
+  template <typename U>
+  void badNestedTemplate();
+  template <typename U>
+  void badNestedTemplateWrongLocation();
+
+  template <typename... Us>
+  void goodNestedVariadicTemplate();
+  template <typename... Us>
+  void badNestedVariadicTemplate();
+  template <typename... Us>
+  void badNestedVariadicTemplateWrongLocation();
+};
+
+template <typename... Ts> LIBC_INLINE void VariadicTemplate<Ts...>::goodVariadicTemplate() {}
+
+template <typename... Ts> inline void VariadicTemplate<Ts...>::badVariadicTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> void LIBC_INLINE VariadicTemplate<Ts...>::badVariadicTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> template <typename U> LIBC_INLINE void VariadicTemplate<Ts...>::goodNestedTemplate() {}
+
+template <typename... Ts> template <typename U> inline void VariadicTemplate<Ts...>::badNestedTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: 'badNestedTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> template <typename U> void LIBC_INLINE VariadicTemplate<Ts...>::badNestedTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: 'badNestedTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> template <typename... Us> LIBC_INLINE void VariadicTemplate<Ts...>::goodNestedVariadicTemplate() {}
+
+template <typename... Ts> template <typename... Us> inline void VariadicTemplate<Ts...>::badNestedVariadicTemplate() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:53: warning: 'badNestedVariadicTemplate' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename... Ts> template <typename... Us> void LIBC_INLINE VariadicTemplate<Ts...>::badNestedVariadicTemplateWrongLocation() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:53: warning: 'badNestedVariadicTemplateWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+template <typename T>
+void goodWeirdFormatting();
+template <typename T>
+void badWeirdFormatting();
+
+template <typename T>LIBC_INLINE void goodWeirdFormatting() {}
+
+template <typename T>void LIBC_INLINE badWeirdFormatting() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'badWeirdFormatting' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+
+} // namespace issue_62746
+
 } // namespace __llvm_libc
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H