https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/71095
>From 0ca1191c2235881493a82f040c437d8da8a82862 Mon Sep 17 00:00:00 2001 From: Michael Jones <michae...@google.com> Date: Thu, 2 Nov 2023 12:23:34 -0700 Subject: [PATCH 1/3] [clang-tidy][libc] Ignore implicit function inline This patch adjusts the inline function decl check for LLVM libc to ignore implicit functions. For the moment the plan is to ignore these and mark the class with a macro so that it can be given the appropriate properties without explicitly defining all its ctors/dtors. --- .../clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp | 4 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++ .../checks/llvmlibc/inline-function-decl.rst | 7 ++++--- .../checkers/llvmlibc/inline-function-decl.hpp | 11 +++++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp index c119e393d3ab692..7f51d0b8c51e7f4 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp @@ -79,6 +79,10 @@ void InlineFunctionDeclCheck::check(const MatchFinder::MatchResult &Result) { if (MethodDecl->getParent()->isLambda()) return; + // Ignore implicit functions (e.g. implicit constructors or destructors) + if (FuncDecl->isImplicit()) + return; + // Check if decl starts with LIBC_INLINE auto Loc = FullSourceLoc(Result.SourceManager->getFileLoc(SrcBegin), *Result.SourceManager); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ecfb3aa9267f140..c47a73b022037d9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -304,6 +304,10 @@ Changes in existing checks customizable namespace. This further allows for testing the libc when the system-libc is also LLVM's libc. +- Improved :doc:`llvmlibc-inline-function-decl + <clang-tidy/checks/llvmlibc/inline-function-decl>` to properly ignore implicit + functions, such as struct constructors. + - Improved :doc:`misc-const-correctness <clang-tidy/checks/misc/const-correctness>` check to avoid false positive when using pointer to member function. diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst index 101217b64c82852..87beb628ac91985 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst @@ -3,6 +3,7 @@ llvmlibc-inline-function-decl ============================= -Checks that all implicit and explicit inline functions in header files are -tagged with the ``LIBC_INLINE`` macro. See the `libc style guide -<https://libc.llvm.org/dev/code_style.html>`_ for more information about this macro. +Checks that all implicitly and explicitly inline functions in header files are +tagged with the ``LIBC_INLINE`` macro, except for functions implicit to classes. +See the `libc style guide<https://libc.llvm.org/dev/code_style.html>`_ for more +information about this macro. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp index 0028c575b1d6871..b2a67a08d0b0d69 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp @@ -278,6 +278,17 @@ template <typename T>LIBC_INLINE void goodWeirdFormatting() {} template <typename T>void LIBC_INLINE badWeirdFormatting() {} // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'badWeirdFormatting' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl] + +template <unsigned int NumberOfBits> struct HasMemberAndTemplate { + char Data[NumberOfBits]; + + void explicitFunction() {} +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'explicitFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl] +// CHECK-FIXES: LIBC_INLINE void explicitFunction() {} +}; + +static auto instanceOfStruct = HasMemberAndTemplate<16>(); + } // namespace issue_62746 } // namespace __llvm_libc >From 78d5ecaf9a3cbc514c466e6baa9ceeba5ded4981 Mon Sep 17 00:00:00 2001 From: Michael Jones <michae...@google.com> Date: Thu, 2 Nov 2023 14:24:39 -0700 Subject: [PATCH 2/3] Move implicit check to matcher, add deleted check Also add tests for defaulted and deleted functions --- .../clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp | 4 ++-- .../clang-tidy/llvmlibc/InlineFunctionDeclCheck.h | 5 +++++ .../checks/llvmlibc/inline-function-decl.rst | 2 +- .../checkers/llvmlibc/inline-function-decl.hpp | 11 +++++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp index 7f51d0b8c51e7f4..3e3662efc35c212 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp @@ -79,8 +79,8 @@ void InlineFunctionDeclCheck::check(const MatchFinder::MatchResult &Result) { if (MethodDecl->getParent()->isLambda()) return; - // Ignore implicit functions (e.g. implicit constructors or destructors) - if (FuncDecl->isImplicit()) + // Ignore functions that have been deleted. + if (FuncDecl->isDeleted()) return; // Check if decl starts with LIBC_INLINE diff --git a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h index 662a592abd9be12..52516f776ad49ce 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h +++ b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.h @@ -33,6 +33,11 @@ class InlineFunctionDeclCheck : public ClangTidyCheck { void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + // Ignore implicit functions (e.g. implicit constructors or destructors) + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + private: FileExtensionsSet HeaderFileExtensions; }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst index 87beb628ac91985..a8d540ba095eb21 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst @@ -5,5 +5,5 @@ llvmlibc-inline-function-decl Checks that all implicitly and explicitly inline functions in header files are tagged with the ``LIBC_INLINE`` macro, except for functions implicit to classes. -See the `libc style guide<https://libc.llvm.org/dev/code_style.html>`_ for more +See the `libc style guide <https://libc.llvm.org/dev/code_style.html>`_ for more information about this macro. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp index b2a67a08d0b0d69..fd2a59289f7682e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp @@ -289,6 +289,17 @@ template <unsigned int NumberOfBits> struct HasMemberAndTemplate { static auto instanceOfStruct = HasMemberAndTemplate<16>(); +struct HasMemberAndExplicitDefault { + int TrivialMember; + + HasMemberAndExplicitDefault() = default; +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'HasMemberAndExplicitDefault' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl] +// CHECK-FIXES: LIBC_INLINE HasMemberAndExplicitDefault() = default; + + ~HasMemberAndExplicitDefault() = delete; +}; + + } // namespace issue_62746 } // namespace __llvm_libc >From da64c4ba7fcaeb5ff653536e00b2a7bdb12b4aaa Mon Sep 17 00:00:00 2001 From: Michael Jones <michae...@google.com> Date: Thu, 2 Nov 2023 15:08:59 -0700 Subject: [PATCH 3/3] Move deleted check to matcher, update docs --- .../clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp | 8 +++----- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../clang-tidy/checks/llvmlibc/inline-function-decl.rst | 3 ++- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp index 3e3662efc35c212..0607fd9d5add869 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp @@ -45,7 +45,9 @@ InlineFunctionDeclCheck::InlineFunctionDeclCheck(StringRef Name, HeaderFileExtensions(Context->getHeaderFileExtensions()) {} void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this); + // Ignore functions that have been deleted. + Finder->addMatcher(decl(functionDecl(unless(isDeleted()))).bind("func_decl"), + this); } void InlineFunctionDeclCheck::check(const MatchFinder::MatchResult &Result) { @@ -79,10 +81,6 @@ void InlineFunctionDeclCheck::check(const MatchFinder::MatchResult &Result) { if (MethodDecl->getParent()->isLambda()) return; - // Ignore functions that have been deleted. - if (FuncDecl->isDeleted()) - return; - // Check if decl starts with LIBC_INLINE auto Loc = FullSourceLoc(Result.SourceManager->getFileLoc(SrcBegin), *Result.SourceManager); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c47a73b022037d9..68feb39da63c898 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -306,7 +306,7 @@ Changes in existing checks - Improved :doc:`llvmlibc-inline-function-decl <clang-tidy/checks/llvmlibc/inline-function-decl>` to properly ignore implicit - functions, such as struct constructors. + functions, such as struct constructors, and explicitly deleted functions. - Improved :doc:`misc-const-correctness <clang-tidy/checks/misc/const-correctness>` check to avoid false positive when diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst index a8d540ba095eb21..71d963969005bad 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl.rst @@ -4,6 +4,7 @@ llvmlibc-inline-function-decl ============================= Checks that all implicitly and explicitly inline functions in header files are -tagged with the ``LIBC_INLINE`` macro, except for functions implicit to classes. +tagged with the ``LIBC_INLINE`` macro, except for functions implicit to classes +or deleted functions. See the `libc style guide <https://libc.llvm.org/dev/code_style.html>`_ for more information about this macro. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits