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