[clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

[jhuber6]

Summary

The llvmlibc-inline-function-decl check is intended to be used to
allow declarations in the libc project's header to be changed per-TU.
However, it is impossible to place this macro in front of a lambda so
this is not helpful. Additionally, lambdas are always going to have
internal linkage so they will not differ accross TUs.

Fixes https://github.com/llvm/llvm-project/issues/62147

[PiotrZSL]

Missing test

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

43

auto -> const auto

[PiotrZSL]

This comment was removed by PiotrZSL.

[PiotrZSL]

Add test

[jhuber6]

Add test

[carlosgalvezp]

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

43

Since there are 2 lines under this if, please add braces for better readability.

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

64–67

Not needed, simply write a plain comment explaining why the check should not warn here.

[jhuber6]

Address nit

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

64–67

Since I've already written it I think we should be able to keep it. If really you don't like the extra check lines I can remove them.

[PiotrZSL]

From functional point of view it's looking good.

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

42

NOTE: This comment refers also to inline exclusion in line 33. I don't think is needed, or you can extract line 33 and 43 into separate function like isExternalyVisibleDeclaration, and to be honest for check that there are other ways, some checks already do that, because simply then you can ask your self, what about coonstexpr functions, what about static functions, functions in anonymous namespace In theory they also may be catch here. Even that inline keyword could be redundant.

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

66

NOTE: If this warning were printed then we got other issue in this check. Implicit functions are checked, would be good to exclude them: functionDecl(unless(isImplicit())).

[PiotrZSL]

Fix Linux build before committing & resolve all comments.

[jhuber6]

In D148444#4272036, @PiotrZSL wrote:

Fix Linux build before committing & resolve all comments.

The log says that it failed because of the CMake version. I don't think I can fix that.

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

66

That was printed without this patch. Are you saying we should have a separate check for these types of functions?

[PiotrZSL]

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

66

I'm just telling that this shouldn't be printed in first place because they are implicit, so there is no place to add macro.
For me this is another bug in this check that should be addressed (probably in separate patch).

[PiotrZSL]

Rebase code, this will fix a build.

[jhuber6]

Rebasing on main

[dblaikie]

Additionally, lambdas are always going to have internal linkage so they will not differ accross TUs.

This isn't true - if a lambda appears in a header in any way, it probably has linkage the same as an inline function, not internal.

(eg: a lambda inside an inline function has linkage so it gets deduplicated with other copies from different inclusions of a header)

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

if (FuncDecl == nullptr || !FuncDecl->isInlined())
     return;
 
   SourceLocation SrcBegin = FuncDecl->getBeginLoc();
   // 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 (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/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

public:
   }
 
   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;
   }
 };
 
+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 __llvm_libc
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_LLVMLIBC_INLINEFUNCTIONDECL_H