carlosgalvezp added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:27
+ // Consider only explicitly or implicitly inline functions.
+ if (!FuncDecl->isInlined())
+ return;
----------------
Check if FuncDecl is not a nullptr before dereferencing. You can do an early
return like:
```
if (!FuncDecl)
return;
```
================
Comment at:
clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h:27-36
+ InlineFunctionDeclCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+ "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions,
+ utils::defaultFileExtensionDelimiters())) {
----------------
Please implement constructor in the .cpp file.
================
Comment at:
clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h:39-41
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
----------------
This is OK to keep in the header as it's only one line.
================
Comment at:
clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h:43-51
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
+ Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+ if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions,
+ utils::defaultFileExtensionDelimiters())) {
+ this->configurationDiag("Invalid header file extension: '%0'")
+ << RawStringHeaderFileExtensions;
----------------
Ditto
================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:100
+New `llvmlibc-inline-function-decl-check
<http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc/inline-function-decl-check.html>`_
+check.
----------------
I believe you'll need a rebase since a new check was recently added.
================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:104
+Checks that all implicit and explicit inline functions in header files are
+tagged with the ``LIBC_INLINE`` macro.
+
----------------
You need to also add the check to
`clang-tools-extra/docs/clang-tidy/checks/list.rst`
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp:2
+// RUN: %check_clang_tidy %s llvmlibc-inline-function-decl-check %t \
+// RUN: -- -header-filter=.* -- -I %S
+
----------------
I believe this can be removed.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp:20
+constexpr long long addll(long long a, long long b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the
LIBC_INLINE macro
+ return a + b;
----------------
Missing check name:
```
... LIBC_INLINE macro [llvmlibc-inline-function-decl]
```
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp:45
+ static int getVal(const MyClass &V) {
+ // CHECK-MESSAGES: warning: 'getVal' must be tagged with the LIBC_INLINE
macro
+ return V.A;
----------------
Missing line and column info
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
