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