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

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:20
+void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
+}
----------------
ClockMan wrote:
> carlosgalvezp wrote:
> > ClockMan wrote:
> > > or maybe even better:
> > > Finder->addMatcher(functionDecl(unless(isExpansionInMainFile()), isInline()).bind("func_decl"), this);
> > > Instead of line 26 and 32.
> > 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.
> 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.
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.


================
Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:29
+
+  auto SrcBegin = FuncDecl->getSourceRange().getBegin();
+  // Consider functions only in header files.
----------------
carlosgalvezp wrote:
> Eugene.Zelenko wrote:
> > Please don't use `auto` unless type is explicitly stated in same statement or iterator.
> We have a well-established pattern for detecting code in headers, grep for `HeaderFileExtensions` in existing checks.
Thanks! I have now switched over to use the `HeaderFileExtensions` pattern.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142592/new/

https://reviews.llvm.org/D142592