[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas
Piotr Zegar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 16 06:09:39 PDT 2023
PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
>From functional point of view it's looking good.
================
Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:42
+ // Consider only functions with an external and visible declaration.
+ if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl)) {
----------------
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.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp:66
+// 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]
----------------
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()))`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148444/new/
https://reviews.llvm.org/D148444
More information about the cfe-commits
mailing list