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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
