https://github.com/MichelleCDjunaidi updated https://github.com/llvm/llvm-project/pull/102299
>From 75306bd83eb43d0606630f9f059fc04ad1b20b06 Mon Sep 17 00:00:00 2001 From: Michelle C Djunaidi <michellechrisa...@gmail.com> Date: Wed, 7 Aug 2024 13:10:02 +0800 Subject: [PATCH 01/11] [clang-tidy] Add bugprone-public-enable-shared-from-this check This check identifies classes deriving from std::enable_shared_from_this that does not inherit with the public keyword, which may cause problems when std::make_shared is called for that class. --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../PublicEnableSharedFromThisCheck.cpp | 45 +++++++++++++++ .../PublicEnableSharedFromThisCheck.h | 33 +++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../bugprone/public-enable-shared-from-this | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../public-enable-shared-from-this.cpp | 56 +++++++++++++++++++ 8 files changed, 150 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 689eb92a3d8d17..f12f0cd1c47da3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -53,6 +53,7 @@ #include "ParentVirtualCallCheck.h" #include "PointerArithmeticOnPolymorphicObjectCheck.h" #include "PosixReturnCheck.h" +#include "PublicEnableSharedFromThisCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" #include "ReturnConstRefFromParameterCheck.h" @@ -139,6 +140,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); + CheckFactories.registerCheck<PublicEnableSharedFromThisCheck>( + "bugprone-public-enable-shared-from-this"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index cb0d8ae98bac58..c9bea094241edc 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule ImplicitWideningOfMultiplicationResultCheck.cpp InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp + PublicEnableSharedFromThisCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp new file mode 100644 index 00000000000000..dab3e0ac596fe7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp @@ -0,0 +1,45 @@ +//===--- PublicEnableSharedFromThisCheck.cpp - clang-tidy -------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "PublicEnableSharedFromThisCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + + void PublicEnableSharedFromThisCheck::registerMatchers(MatchFinder *match_finder) { + match_finder->addMatcher( + cxxRecordDecl( + hasAnyBase( + cxxBaseSpecifier(unless(isPublic()), + hasType( + cxxRecordDecl( + hasName("::std::enable_shared_from_this")))) + ) + ) + .bind("not-public-enable-shared"), this); + } + + void PublicEnableSharedFromThisCheck::check(const MatchFinder::MatchResult &result) { + const auto *EnableSharedClassDecl = + result.Nodes.getNodeAs<CXXRecordDecl>("not-public-enable-shared"); + + for (const auto &Base : EnableSharedClassDecl->bases()) { + const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); + if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { + SourceLocation InsertLoc = Base.getBeginLoc(); + FixItHint Hint = FixItHint::CreateInsertion(InsertLoc, "public "); + diag(EnableSharedClassDecl->getLocation(), "class %0 is not public even though it's derived from std::enable_shared_from_this", DiagnosticIDs::Warning) + << EnableSharedClassDecl->getNameAsString() + << Hint; + break; + } + } + } +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h new file mode 100644 index 00000000000000..8a4013c3a46b7f --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h @@ -0,0 +1,33 @@ +//===--- PublicEnableSharedFromThisCheck.h - clang-tidy ---------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/public-enable-shared-from-this.html +class PublicEnableSharedFromThisCheck : public ClangTidyCheck { +public: + PublicEnableSharedFromThisCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 642ad39cc0c1c5..67f6b1275a1adf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -98,6 +98,11 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-public-enable-shared-from-this + <clang-tidy/checks/bugprone/public-enable-shared-from-this>` check. + + Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this new file mode 100644 index 00000000000000..d3f500eaa51460 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this @@ -0,0 +1,6 @@ +.. title:: clang-tidy - bugprone-public-enable-shared-from-this + +bugprone-public-enable-shared-from-this +======================================= + +FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index a931ebf025a10e..2b918de89f7c19 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -120,6 +120,7 @@ Clang-Tidy Checks :doc:`bugprone-parent-virtual-call <bugprone/parent-virtual-call>`, "Yes" :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`, :doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes" + :doc:`bugprone-public-enable-shared-from-this <bugprone/public-enable-shared-from-this>`, "Yes" :doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes" :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" :doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp new file mode 100644 index 00000000000000..a3e6d0ee6ca952 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp @@ -0,0 +1,56 @@ +// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- -I %S/Inputs/ +#include <memory> + +class BadExample : std::enable_shared_from_this<BadExample> { +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] +// CHECK-FIXES: :[[@LINE-2]]:19 public + public: + BadExample* foo() { return shared_from_this().get(); } + void bar() { return; } +}; + +void using_not_public() { + auto bad_example = std::make_shared<BadExample>(); + auto* b_ex = bad_example->foo(); + b_ex->bar(); +} + +class GoodExample : public std::enable_shared_from_this<GoodExample> { + public: + GoodExample* foo() { return shared_from_this().get(); } + void bar() { return; } +}; + +void using_public() { + auto good_example = std::make_shared<GoodExample>(); + auto* g_ex = good_example->foo(); + g_ex->bar(); +} + +struct BaseClass { + + void print() { + (void) State; + (void) Requester; + } + bool State; + int Requester; +}; + +class InheritPrivateBaseClass : BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; +}; + +class InheritPublicBaseClass : public BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; +}; \ No newline at end of file >From 4461045503c3efc2170da750911c20d83afdb0f1 Mon Sep 17 00:00:00 2001 From: Michelle C Djunaidi <michellechrisa...@gmail.com> Date: Wed, 7 Aug 2024 17:58:42 +0800 Subject: [PATCH 02/11] Fix test to be compatible with llvm-lit, add docs --- .../bugprone/public-enable-shared-from-this | 6 -- .../public-enable-shared-from-this.rst | 26 ++++++ .../public-enable-shared-from-this.cpp | 93 ++++++++----------- 3 files changed, 67 insertions(+), 58 deletions(-) delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this deleted file mode 100644 index d3f500eaa51460..00000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this +++ /dev/null @@ -1,6 +0,0 @@ -.. title:: clang-tidy - bugprone-public-enable-shared-from-this - -bugprone-public-enable-shared-from-this -======================================= - -FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst new file mode 100644 index 00000000000000..616b2b52271e3f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst @@ -0,0 +1,26 @@ +.. title:: clang-tidy - bugprone-public-enable-shared-from-this + +bugprone-public-enable-shared-from-this +======================================= + +Checks that classes/structs inheriting from ``std::enable_shared_from_this`` derive it with the ``public`` access specifier. If not, then issue a FixItHint that can be applied. + +Consider the following code: +.. code-block:: c++ + #include <memory> + + class BadExample : std::enable_shared_from_this<BadExample> { + // warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] + // will insert the public keyword if -fix is applied + public: + BadExample* foo() { return shared_from_this().get(); } + void bar() { return; } + }; + + void using_not_public() { + auto bad_example = std::make_shared<BadExample>(); + auto* b_ex = bad_example->foo(); + b_ex->bar(); + } + +When ``using_not_public()`` is called, this code will crash. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp index a3e6d0ee6ca952..b0954898ddb783 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp @@ -1,56 +1,45 @@ -// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- -I %S/Inputs/ -#include <memory> +// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- -class BadExample : std::enable_shared_from_this<BadExample> { -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] -// CHECK-FIXES: :[[@LINE-2]]:19 public - public: - BadExample* foo() { return shared_from_this().get(); } - void bar() { return; } -}; +namespace std { -void using_not_public() { - auto bad_example = std::make_shared<BadExample>(); - auto* b_ex = bad_example->foo(); - b_ex->bar(); -} + template <typename T> class enable_shared_from_this {}; -class GoodExample : public std::enable_shared_from_this<GoodExample> { - public: - GoodExample* foo() { return shared_from_this().get(); } - void bar() { return; } -}; - -void using_public() { - auto good_example = std::make_shared<GoodExample>(); - auto* g_ex = good_example->foo(); - g_ex->bar(); -} - -struct BaseClass { - - void print() { - (void) State; - (void) Requester; - } - bool State; - int Requester; -}; - -class InheritPrivateBaseClass : BaseClass { - public: - void additionalFunction() { - (void) ID; - } - private: - int ID; -}; - -class InheritPublicBaseClass : public BaseClass { - public: - void additionalFunction() { - (void) ID; + class BadExample : enable_shared_from_this<BadExample> {}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] + // CHECK-FIXES: public enable_shared_from_this<BadExample> + + class Bad2Example : std::enable_shared_from_this<Bad2Example> {}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class Bad2Example is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] + // CHECK-FIXES: public std::enable_shared_from_this<Bad2Example> + + class GoodExample : public enable_shared_from_this<GoodExample> { + }; + + struct BaseClass { + + void print() { + (void) State; + (void) Requester; } - private: - int ID; -}; \ No newline at end of file + bool State; + int Requester; + }; + + class InheritPrivateBaseClass : BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; + }; + + class InheritPublicBaseClass : public BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; + }; +} \ No newline at end of file >From ecdb9a53abfe5556155ba4293a2bd60374cb8608 Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Thu, 8 Aug 2024 13:44:15 +0800 Subject: [PATCH 03/11] Change name to bugprone-incorrect-enable-shared-from-this. WIP in regards to test file and matchers, do not re-review yet. --- .../bugprone/BugproneTidyModule.cpp | 6 +-- .../clang-tidy/bugprone/CMakeLists.txt | 2 +- ...=> IncorrectEnableSharedFromThisCheck.cpp} | 10 ++--- ...h => IncorrectEnableSharedFromThisCheck.h} | 16 +++---- clang-tools-extra/docs/ReleaseNotes.rst | 8 ++-- .../incorrect-enable-shared-from-this.rst | 32 +++++++++++++ .../public-enable-shared-from-this.rst | 26 ----------- .../docs/clang-tidy/checks/list.rst | 2 +- .../incorrect-enable-shared-from-this.cpp | 23 ++++++++++ .../public-enable-shared-from-this.cpp | 45 ------------------- 10 files changed, 78 insertions(+), 92 deletions(-) rename clang-tools-extra/clang-tidy/bugprone/{PublicEnableSharedFromThisCheck.cpp => IncorrectEnableSharedFromThisCheck.cpp} (75%) rename clang-tools-extra/clang-tidy/bugprone/{PublicEnableSharedFromThisCheck.h => IncorrectEnableSharedFromThisCheck.h} (59%) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index f12f0cd1c47da3..f8e40cd0f830d2 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -53,7 +53,7 @@ #include "ParentVirtualCallCheck.h" #include "PointerArithmeticOnPolymorphicObjectCheck.h" #include "PosixReturnCheck.h" -#include "PublicEnableSharedFromThisCheck.h" +#include "IncorrectEnableSharedFromThisCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" #include "ReturnConstRefFromParameterCheck.h" @@ -140,8 +140,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); - CheckFactories.registerCheck<PublicEnableSharedFromThisCheck>( - "bugprone-public-enable-shared-from-this"); + CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( + "bugprone-incorrect-enable-shared-from-this"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index c9bea094241edc..9ee1140722e67a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -26,7 +26,7 @@ add_clang_library(clangTidyBugproneModule ImplicitWideningOfMultiplicationResultCheck.cpp InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp - PublicEnableSharedFromThisCheck.cpp + IncorrectEnableSharedFromThisCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp similarity index 75% rename from clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp rename to clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index dab3e0ac596fe7..0bae400f3e8b4e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -1,4 +1,4 @@ -//===--- PublicEnableSharedFromThisCheck.cpp - clang-tidy -------------------------===// +//===--- IncorrectEnableSharedFromThisCheck.cpp - clang-tidy -------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,14 +6,14 @@ // //===----------------------------------------------------------------------===// -#include "PublicEnableSharedFromThisCheck.h" +#include "IncorrectEnableSharedFromThisCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" using namespace clang::ast_matchers; namespace clang::tidy::bugprone { - void PublicEnableSharedFromThisCheck::registerMatchers(MatchFinder *match_finder) { + void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *match_finder) { match_finder->addMatcher( cxxRecordDecl( hasAnyBase( @@ -26,7 +26,7 @@ namespace clang::tidy::bugprone { .bind("not-public-enable-shared"), this); } - void PublicEnableSharedFromThisCheck::check(const MatchFinder::MatchResult &result) { + void IncorrectEnableSharedFromThisCheck::check(const MatchFinder::MatchResult &result) { const auto *EnableSharedClassDecl = result.Nodes.getNodeAs<CXXRecordDecl>("not-public-enable-shared"); @@ -35,7 +35,7 @@ namespace clang::tidy::bugprone { if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { SourceLocation InsertLoc = Base.getBeginLoc(); FixItHint Hint = FixItHint::CreateInsertion(InsertLoc, "public "); - diag(EnableSharedClassDecl->getLocation(), "class %0 is not public even though it's derived from std::enable_shared_from_this", DiagnosticIDs::Warning) + diag(EnableSharedClassDecl->getLocation(), "inheritance from std::enable_shared_from_this should be public inheritance", DiagnosticIDs::Warning) << EnableSharedClassDecl->getNameAsString() << Hint; break; diff --git a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h similarity index 59% rename from clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h rename to clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h index 8a4013c3a46b7f..fd20ecc9e3c21a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h @@ -1,4 +1,4 @@ -//===--- PublicEnableSharedFromThisCheck.h - clang-tidy ---------*- C++ -*-===// +//===--- IncorrectEnableSharedFromThisCheck.h - clang-tidy ------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H #include "../ClangTidyCheck.h" @@ -16,18 +16,18 @@ namespace clang::tidy::bugprone { /// Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. /// /// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/public-enable-shared-from-this.html -class PublicEnableSharedFromThisCheck : public ClangTidyCheck { +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.html +class IncorrectEnableSharedFromThisCheck : public ClangTidyCheck { public: - PublicEnableSharedFromThisCheck(StringRef Name, ClangTidyContext *Context) + IncorrectEnableSharedFromThisCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { - return LangOpts.CPlusPlus; + return LangOpts.CPlusPlus11; } }; } // namespace clang::tidy::bugprone -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 67f6b1275a1adf..0787d21c71fffe 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -98,10 +98,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ -- New :doc:`bugprone-public-enable-shared-from-this - <clang-tidy/checks/bugprone/public-enable-shared-from-this>` check. +- New :doc:`bugprone-incorrect-enable-shared-from-this + <clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check. - Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. + Check if class/structs publicly inherits from ``std::enable_shared_from_this``, + because otherwise when ``shared_from_this`` is called, the code will throw + ``std::bad_weak_ptr``. New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst new file mode 100644 index 00000000000000..2dfe5c7b77fe33 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst @@ -0,0 +1,32 @@ +.. title:: clang-tidy - bugprone-incorrect-enable-shared-from-this + +bugprone-incorrect-enable-shared-from-this +======================================= + +Check if class/struct publicly derives from ``std::enable_shared_from_this``, +because otherwise when ``shared_from_this`` is called it will throw +``std::bad_weak_ptr``. Issues a ``FixItHint`` that can be applied. + +Consider the following code: + +.. code-block:: c++ + #include <memory> + + class BadExample : std::enable_shared_from_this<BadExample> { + // warning: inheritance from std::enable_shared_from_this + // should be public inheritance, + // otherwise the internal weak_ptr won't be initialized + // [bugprone-incorrect-enable-shared-from-this] + public: + BadExample* foo() { return shared_from_this().get(); } + void bar() { return; } + }; + + void using_not_public() { + auto bad_example = std::make_shared<BadExample>(); + auto* b_ex = bad_example->foo(); + b_ex->bar(); + } + +When ``using_not_public()`` is called, this code will crash without exception +handling. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst deleted file mode 100644 index 616b2b52271e3f..00000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst +++ /dev/null @@ -1,26 +0,0 @@ -.. title:: clang-tidy - bugprone-public-enable-shared-from-this - -bugprone-public-enable-shared-from-this -======================================= - -Checks that classes/structs inheriting from ``std::enable_shared_from_this`` derive it with the ``public`` access specifier. If not, then issue a FixItHint that can be applied. - -Consider the following code: -.. code-block:: c++ - #include <memory> - - class BadExample : std::enable_shared_from_this<BadExample> { - // warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] - // will insert the public keyword if -fix is applied - public: - BadExample* foo() { return shared_from_this().get(); } - void bar() { return; } - }; - - void using_not_public() { - auto bad_example = std::make_shared<BadExample>(); - auto* b_ex = bad_example->foo(); - b_ex->bar(); - } - -When ``using_not_public()`` is called, this code will crash. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2b918de89f7c19..db18d0cac8764d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -100,6 +100,7 @@ Clang-Tidy Checks :doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes" :doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`, :doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes" + :doc:`bugprone-incorrect-enable-shared-from-this <bugprone/incorrect-enable-shared-from-this>`, "Yes" :doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`, :doc:`bugprone-infinite-loop <bugprone/infinite-loop>`, :doc:`bugprone-integer-division <bugprone/integer-division>`, @@ -120,7 +121,6 @@ Clang-Tidy Checks :doc:`bugprone-parent-virtual-call <bugprone/parent-virtual-call>`, "Yes" :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`, :doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes" - :doc:`bugprone-public-enable-shared-from-this <bugprone/public-enable-shared-from-this>`, "Yes" :doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes" :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" :doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp new file mode 100644 index 00000000000000..c7f5fd7516932d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -0,0 +1,23 @@ +// RUN: %check_clang_tidy %s bugprone-incorrect-enable-shared-from-this %t -- -- + +namespace std { + + template <typename T> class enable_shared_from_this {}; + +} + +class BadClassExample : std::enable_shared_from_this<BadClassExample> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public enable_shared_from_this<BadExample> + +class BadClass2Example : private std::enable_shared_from_this<BadClass2Example> {}; + +struct BadStructExample : private std::enable_shared_from_this<BadStructExample> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadStructExample is not public even though it's derived from std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public std::enable_shared_from_this<BadStructExample> + +class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> {}; + +struct GoodStructExample : public std::enable_shared_from_this<GoodStructExample> {}; + +struct GoodStruct2Example : std::enable_shared_from_this<GoodStruct2Example> {}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp deleted file mode 100644 index b0954898ddb783..00000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp +++ /dev/null @@ -1,45 +0,0 @@ -// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- - -namespace std { - - template <typename T> class enable_shared_from_this {}; - - class BadExample : enable_shared_from_this<BadExample> {}; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] - // CHECK-FIXES: public enable_shared_from_this<BadExample> - - class Bad2Example : std::enable_shared_from_this<Bad2Example> {}; - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class Bad2Example is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] - // CHECK-FIXES: public std::enable_shared_from_this<Bad2Example> - - class GoodExample : public enable_shared_from_this<GoodExample> { - }; - - struct BaseClass { - - void print() { - (void) State; - (void) Requester; - } - bool State; - int Requester; - }; - - class InheritPrivateBaseClass : BaseClass { - public: - void additionalFunction() { - (void) ID; - } - private: - int ID; - }; - - class InheritPublicBaseClass : public BaseClass { - public: - void additionalFunction() { - (void) ID; - } - private: - int ID; - }; -} \ No newline at end of file >From 6261cedae512075fcbbe51cd9a31cd0c5290adad Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Thu, 8 Aug 2024 17:35:24 +0800 Subject: [PATCH 04/11] Fix ``FixItHint`` of public inheritance, implement ``std::`` check Note: Maybe should be separate checks --- .../IncorrectEnableSharedFromThisCheck.cpp | 100 +++++++++++++----- clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../incorrect-enable-shared-from-this.rst | 2 + .../incorrect-enable-shared-from-this.cpp | 46 ++++++-- 4 files changed, 119 insertions(+), 32 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index 0bae400f3e8b4e..fabbac3f43b00b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -1,4 +1,5 @@ -//===--- IncorrectEnableSharedFromThisCheck.cpp - clang-tidy -------------------------===// +//===--- IncorrectEnableSharedFromThisCheck.cpp - clang-tidy +//-------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -13,33 +14,82 @@ using namespace clang::ast_matchers; namespace clang::tidy::bugprone { - void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *match_finder) { - match_finder->addMatcher( - cxxRecordDecl( - hasAnyBase( - cxxBaseSpecifier(unless(isPublic()), - hasType( - cxxRecordDecl( - hasName("::std::enable_shared_from_this")))) - ) - ) - .bind("not-public-enable-shared"), this); - } +void IncorrectEnableSharedFromThisCheck::registerMatchers( + MatchFinder *match_finder) { + match_finder->addMatcher( + cxxRecordDecl( + unless(isExpansionInSystemHeader()), + hasAnyBase(cxxBaseSpecifier(unless(isPublic()), + hasType(cxxRecordDecl(hasName( + "::std::enable_shared_from_this")))))) + .bind("class-base-std-enable"), + this); + match_finder->addMatcher( + cxxRecordDecl(unless(isExpansionInSystemHeader()), + hasAnyBase(cxxBaseSpecifier(hasType( + cxxRecordDecl(hasName("enable_shared_from_this")))))) + .bind("class-missing-std"), + this); +} - void IncorrectEnableSharedFromThisCheck::check(const MatchFinder::MatchResult &result) { - const auto *EnableSharedClassDecl = - result.Nodes.getNodeAs<CXXRecordDecl>("not-public-enable-shared"); +void IncorrectEnableSharedFromThisCheck::check( + const MatchFinder::MatchResult &result) { + const auto *StdEnableSharedClassDecl = + result.Nodes.getNodeAs<CXXRecordDecl>("class-base-std-enable"); + const auto *MissingStdSharedClassDecl = + result.Nodes.getNodeAs<CXXRecordDecl>("class-missing-std"); - for (const auto &Base : EnableSharedClassDecl->bases()) { - const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); - if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { - SourceLocation InsertLoc = Base.getBeginLoc(); - FixItHint Hint = FixItHint::CreateInsertion(InsertLoc, "public "); - diag(EnableSharedClassDecl->getLocation(), "inheritance from std::enable_shared_from_this should be public inheritance", DiagnosticIDs::Warning) - << EnableSharedClassDecl->getNameAsString() + if (StdEnableSharedClassDecl) { + for (const auto &Base : StdEnableSharedClassDecl->bases()) { + const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); + if (BaseType && BaseType->getQualifiedNameAsString() == + "std::enable_shared_from_this") { + SourceRange ReplacementRange = Base.getSourceRange(); + std::string ReplacementString = + "public " + Base.getType().getAsString(); + FixItHint Hint = + FixItHint::CreateReplacement(ReplacementRange, ReplacementString); + diag( + StdEnableSharedClassDecl->getLocation(), + "inheritance from std::enable_shared_from_this should be public " + "inheritance, otherwise the internal weak_ptr won't be initialized", + DiagnosticIDs::Warning) + << Hint; + break; + } + } + } + + if (MissingStdSharedClassDecl) { + for (const auto &Base : MissingStdSharedClassDecl->bases()) { + const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); + if (BaseType && + BaseType->getQualifiedNameAsString() == "enable_shared_from_this") { + clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); + if (AccessSpec == clang::AS_public) { + SourceLocation InsertLocation = Base.getBaseTypeLoc(); + FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::"); + diag(MissingStdSharedClassDecl->getLocation(), + "Should be std::enable_shared_from_this", DiagnosticIDs::Warning) + << Hint; + break; + } else { + SourceRange ReplacementRange = Base.getSourceRange(); + std::string ReplacementString = + "public std::" + Base.getType().getAsString(); + FixItHint Hint = + FixItHint::CreateReplacement(ReplacementRange, ReplacementString); + diag(MissingStdSharedClassDecl->getLocation(), + "Should be std::enable_shared_from_this and " + "inheritance from std::enable_shared_from_this should be public " + "inheritance, otherwise the internal weak_ptr won't be " + "initialized", + DiagnosticIDs::Warning) << Hint; - break; - } + break; + } } + } } +} } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0787d21c71fffe..61c0283029dc0a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -103,7 +103,8 @@ New checks Check if class/structs publicly inherits from ``std::enable_shared_from_this``, because otherwise when ``shared_from_this`` is called, the code will throw - ``std::bad_weak_ptr``. + ``std::bad_weak_ptr``. Also checks for ``std::`` in + ``std::enable_shared_from_this``. New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst index 2dfe5c7b77fe33..a5bff6b51ad7ca 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst @@ -30,3 +30,5 @@ Consider the following code: When ``using_not_public()`` is called, this code will crash without exception handling. + +Also checks for ``std::`` in ``std::enable_shared_from_this``. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp index c7f5fd7516932d..901b40bb829635 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -1,19 +1,19 @@ // RUN: %check_clang_tidy %s bugprone-incorrect-enable-shared-from-this %t -- -- namespace std { - template <typename T> class enable_shared_from_this {}; - -} +} //namespace std class BadClassExample : std::enable_shared_from_this<BadClassExample> {}; -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] -// CHECK-FIXES: public enable_shared_from_this<BadExample> +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public std::enable_shared_from_this<BadClassExample> class BadClass2Example : private std::enable_shared_from_this<BadClass2Example> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public std::enable_shared_from_this<BadClass2Example> struct BadStructExample : private std::enable_shared_from_this<BadStructExample> {}; -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadStructExample is not public even though it's derived from std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] // CHECK-FIXES: public std::enable_shared_from_this<BadStructExample> class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> {}; @@ -21,3 +21,37 @@ class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> { struct GoodStructExample : public std::enable_shared_from_this<GoodStructExample> {}; struct GoodStruct2Example : std::enable_shared_from_this<GoodStruct2Example> {}; + +class dummy_class1 {}; +class dummy_class2 {}; + +class BadMultiClassExample : std::enable_shared_from_this<BadMultiClassExample>, dummy_class1 {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public std::enable_shared_from_this<BadMultiClassExample>, dummy_class1 + +class BadMultiClass2Example : dummy_class1, std::enable_shared_from_this<BadMultiClass2Example>, dummy_class2 {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: dummy_class1, public std::enable_shared_from_this<BadMultiClass2Example>, dummy_class2 + +class BadMultiClass3Example : dummy_class1, dummy_class2, std::enable_shared_from_this<BadMultiClass3Example> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: dummy_class1, dummy_class2, public std::enable_shared_from_this<BadMultiClass3Example> + +template <typename T> class enable_shared_from_this {}; + +class NoStdClassExample : public enable_shared_from_this<NoStdClassExample> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Should be std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public std::enable_shared_from_this<NoStdClassExample> + +struct NoStdStructExample : enable_shared_from_this<NoStdStructExample> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Should be std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: std::enable_shared_from_this<NoStdStructExample> + +class BadMixedProblemExample : enable_shared_from_this<BadMixedProblemExample> {}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Should be std::enable_shared_from_this and inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: public std::enable_shared_from_this<BadMixedProblemExample> + +//FIXME: can't do anything about this, clang-check -ast-dump doesn't show A's internals in class B's AST +class A : public std::enable_shared_from_this<A> {}; +class B : private A{}; + >From 675fa29c5140408f81564d8688a12425912b07d6 Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Thu, 8 Aug 2024 17:40:24 +0800 Subject: [PATCH 05/11] Stated type instead of using auto in the ``check`` method --- .../bugprone/IncorrectEnableSharedFromThisCheck.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index fabbac3f43b00b..16f0ceb86ab6b0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "IncorrectEnableSharedFromThisCheck.h" +#include "clang/AST/DeclCXX.h" #include "clang/ASTMatchers/ASTMatchFinder.h" using namespace clang::ast_matchers; @@ -40,8 +41,8 @@ void IncorrectEnableSharedFromThisCheck::check( result.Nodes.getNodeAs<CXXRecordDecl>("class-missing-std"); if (StdEnableSharedClassDecl) { - for (const auto &Base : StdEnableSharedClassDecl->bases()) { - const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); + for (const CXXBaseSpecifier &Base : StdEnableSharedClassDecl->bases()) { + const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { SourceRange ReplacementRange = Base.getSourceRange(); @@ -61,8 +62,8 @@ void IncorrectEnableSharedFromThisCheck::check( } if (MissingStdSharedClassDecl) { - for (const auto &Base : MissingStdSharedClassDecl->bases()) { - const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); + for (const CXXBaseSpecifier &Base : MissingStdSharedClassDecl->bases()) { + const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); if (BaseType && BaseType->getQualifiedNameAsString() == "enable_shared_from_this") { clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); >From 3aacb94376aa9275e1d17a899fd0f641c552f76b Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:35:26 +0800 Subject: [PATCH 06/11] Refactored const variables and cleanup newlines/formatting --- .../IncorrectEnableSharedFromThisCheck.cpp | 22 +++++++++---------- .../incorrect-enable-shared-from-this.rst | 4 ++-- .../incorrect-enable-shared-from-this.cpp | 3 +-- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index 16f0ceb86ab6b0..78ddfd7406ca7c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -1,5 +1,4 @@ -//===--- IncorrectEnableSharedFromThisCheck.cpp - clang-tidy -//-------------------------===// +//===--- IncorrectEnableSharedFromThisCheck.cpp - clang-tidy --------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -45,10 +44,10 @@ void IncorrectEnableSharedFromThisCheck::check( const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { - SourceRange ReplacementRange = Base.getSourceRange(); - std::string ReplacementString = + const SourceRange ReplacementRange = Base.getSourceRange(); + const std::string ReplacementString = "public " + Base.getType().getAsString(); - FixItHint Hint = + const FixItHint Hint = FixItHint::CreateReplacement(ReplacementRange, ReplacementString); diag( StdEnableSharedClassDecl->getLocation(), @@ -66,19 +65,19 @@ void IncorrectEnableSharedFromThisCheck::check( const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); if (BaseType && BaseType->getQualifiedNameAsString() == "enable_shared_from_this") { - clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); + const clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); if (AccessSpec == clang::AS_public) { - SourceLocation InsertLocation = Base.getBaseTypeLoc(); - FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::"); + const SourceLocation InsertLocation = Base.getBaseTypeLoc(); + const FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::"); diag(MissingStdSharedClassDecl->getLocation(), "Should be std::enable_shared_from_this", DiagnosticIDs::Warning) << Hint; break; } else { - SourceRange ReplacementRange = Base.getSourceRange(); - std::string ReplacementString = + const SourceRange ReplacementRange = Base.getSourceRange(); + const std::string ReplacementString = "public std::" + Base.getType().getAsString(); - FixItHint Hint = + const FixItHint Hint = FixItHint::CreateReplacement(ReplacementRange, ReplacementString); diag(MissingStdSharedClassDecl->getLocation(), "Should be std::enable_shared_from_this and " @@ -93,4 +92,5 @@ void IncorrectEnableSharedFromThisCheck::check( } } } + } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst index a5bff6b51ad7ca..30498befe9b3f8 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst @@ -3,9 +3,9 @@ bugprone-incorrect-enable-shared-from-this ======================================= -Check if class/struct publicly derives from ``std::enable_shared_from_this``, +Checks if class/struct publicly derives from ``std::enable_shared_from_this``, because otherwise when ``shared_from_this`` is called it will throw -``std::bad_weak_ptr``. Issues a ``FixItHint`` that can be applied. +``std::bad_weak_ptr``. Consider the following code: diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp index 901b40bb829635..ac990bcdee14c1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -53,5 +53,4 @@ class BadMixedProblemExample : enable_shared_from_this<BadMixedProblemExample> { //FIXME: can't do anything about this, clang-check -ast-dump doesn't show A's internals in class B's AST class A : public std::enable_shared_from_this<A> {}; -class B : private A{}; - +class B : private A{}; \ No newline at end of file >From 6b9f253c7160a05d1fdc83d488aa248178b947d3 Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:30:15 +0800 Subject: [PATCH 07/11] Fix documentation, comments, newlines, sort module registration properly --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 2 +- .../bugprone/IncorrectEnableSharedFromThisCheck.h | 5 ++++- .../incorrect-enable-shared-from-this.rst | 15 +++++++-------- .../incorrect-enable-shared-from-this.cpp | 10 +++++----- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index f8e40cd0f830d2..7576161a668894 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -33,6 +33,7 @@ #include "InaccurateEraseCheck.h" #include "IncDecInConditionsCheck.h" #include "IncorrectEnableIfCheck.h" +#include "IncorrectEnableSharedFromThisCheck.h" #include "IncorrectRoundingsCheck.h" #include "InfiniteLoopCheck.h" #include "IntegerDivisionCheck.h" @@ -53,7 +54,6 @@ #include "ParentVirtualCallCheck.h" #include "PointerArithmeticOnPolymorphicObjectCheck.h" #include "PosixReturnCheck.h" -#include "IncorrectEnableSharedFromThisCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" #include "ReturnConstRefFromParameterCheck.h" diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h index fd20ecc9e3c21a..b7b15e90b1cd59 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h @@ -13,7 +13,10 @@ namespace clang::tidy::bugprone { -/// Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. +/// Checks if class/struct publicly inherits from +/// ``std::enable_shared_from_this``, because otherwise when +/// ``shared_from_this`` is called it will throw +/// ``std::bad_weak_ptr``. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.html diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst index 30498befe9b3f8..058eb046a4eb47 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst @@ -1,22 +1,21 @@ .. title:: clang-tidy - bugprone-incorrect-enable-shared-from-this bugprone-incorrect-enable-shared-from-this -======================================= +========================================== -Checks if class/struct publicly derives from ``std::enable_shared_from_this``, -because otherwise when ``shared_from_this`` is called it will throw -``std::bad_weak_ptr``. +Checks if class/struct publicly inherits from +``std::enable_shared_from_this``, because otherwise when ``shared_from_this`` +is called it will throw ``std::bad_weak_ptr``. Consider the following code: .. code-block:: c++ #include <memory> + // private inheritance class BadExample : std::enable_shared_from_this<BadExample> { - // warning: inheritance from std::enable_shared_from_this - // should be public inheritance, - // otherwise the internal weak_ptr won't be initialized - // [bugprone-incorrect-enable-shared-from-this] + + // ``shared_from_this``` returns uninitialized ``weak_ptr`` public: BadExample* foo() { return shared_from_this().get(); } void bar() { return; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp index ac990bcdee14c1..d2d328596c0416 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -39,13 +39,13 @@ class BadMultiClass3Example : dummy_class1, dummy_class2, std::enable_shared_fro template <typename T> class enable_shared_from_this {}; -class NoStdClassExample : public enable_shared_from_this<NoStdClassExample> {}; +class BadInitClassExample : public enable_shared_from_this<BadInitClassExample> {}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Should be std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] -// CHECK-FIXES: public std::enable_shared_from_this<NoStdClassExample> +// CHECK-FIXES: public std::enable_shared_from_this<BadInitClassExample> -struct NoStdStructExample : enable_shared_from_this<NoStdStructExample> {}; +struct BadInitStructExample : enable_shared_from_this<BadInitStructExample> {}; // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Should be std::enable_shared_from_this [bugprone-incorrect-enable-shared-from-this] -// CHECK-FIXES: std::enable_shared_from_this<NoStdStructExample> +// CHECK-FIXES: std::enable_shared_from_this<BadInitStructExample> class BadMixedProblemExample : enable_shared_from_this<BadMixedProblemExample> {}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Should be std::enable_shared_from_this and inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] @@ -53,4 +53,4 @@ class BadMixedProblemExample : enable_shared_from_this<BadMixedProblemExample> { //FIXME: can't do anything about this, clang-check -ast-dump doesn't show A's internals in class B's AST class A : public std::enable_shared_from_this<A> {}; -class B : private A{}; \ No newline at end of file +class B : private A{}; >From 1fcf61d7d48169494aab30acb1a5218d3e3bc7fc Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Wed, 14 Aug 2024 18:03:34 +0800 Subject: [PATCH 08/11] Switch implementation to RecursiveASTVisitor --- .../IncorrectEnableSharedFromThisCheck.cpp | 129 ++++++++++-------- .../incorrect-enable-shared-from-this.cpp | 22 ++- 2 files changed, 94 insertions(+), 57 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index 78ddfd7406ca7c..bd7c485ccba921 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -8,89 +8,110 @@ #include "IncorrectEnableSharedFromThisCheck.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/Specifiers.h" +#include "llvm/ADT/SmallPtrSet.h" using namespace clang::ast_matchers; namespace clang::tidy::bugprone { void IncorrectEnableSharedFromThisCheck::registerMatchers( - MatchFinder *match_finder) { - match_finder->addMatcher( - cxxRecordDecl( - unless(isExpansionInSystemHeader()), - hasAnyBase(cxxBaseSpecifier(unless(isPublic()), - hasType(cxxRecordDecl(hasName( - "::std::enable_shared_from_this")))))) - .bind("class-base-std-enable"), - this); - match_finder->addMatcher( - cxxRecordDecl(unless(isExpansionInSystemHeader()), - hasAnyBase(cxxBaseSpecifier(hasType( - cxxRecordDecl(hasName("enable_shared_from_this")))))) - .bind("class-missing-std"), - this); + MatchFinder *Finder) { + Finder->addMatcher(translationUnitDecl(), this); } void IncorrectEnableSharedFromThisCheck::check( - const MatchFinder::MatchResult &result) { - const auto *StdEnableSharedClassDecl = - result.Nodes.getNodeAs<CXXRecordDecl>("class-base-std-enable"); - const auto *MissingStdSharedClassDecl = - result.Nodes.getNodeAs<CXXRecordDecl>("class-missing-std"); + const MatchFinder::MatchResult &Result) { - if (StdEnableSharedClassDecl) { - for (const CXXBaseSpecifier &Base : StdEnableSharedClassDecl->bases()) { - const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); - if (BaseType && BaseType->getQualifiedNameAsString() == - "std::enable_shared_from_this") { - const SourceRange ReplacementRange = Base.getSourceRange(); - const std::string ReplacementString = - "public " + Base.getType().getAsString(); - const FixItHint Hint = - FixItHint::CreateReplacement(ReplacementRange, ReplacementString); - diag( - StdEnableSharedClassDecl->getLocation(), - "inheritance from std::enable_shared_from_this should be public " - "inheritance, otherwise the internal weak_ptr won't be initialized", - DiagnosticIDs::Warning) - << Hint; - break; + class Visitor : public RecursiveASTVisitor<Visitor> { + IncorrectEnableSharedFromThisCheck &Check; + llvm::SmallPtrSet<const CXXRecordDecl*, 16> EnableSharedClassSet; + + public: + explicit Visitor(IncorrectEnableSharedFromThisCheck &Check) : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {} + + bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) { + for (const auto &Base : RDecl->bases()) { + VisitCXXBaseSpecifier(Base, RDecl); + } + for (const auto &Base : RDecl->bases()) { + const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); + if (BaseType && BaseType->getQualifiedNameAsString() == + "std::enable_shared_from_this") { + EnableSharedClassSet.insert(RDecl->getCanonicalDecl()); + } } + return true; } - } - if (MissingStdSharedClassDecl) { - for (const CXXBaseSpecifier &Base : MissingStdSharedClassDecl->bases()) { + bool VisitCXXBaseSpecifier(const CXXBaseSpecifier &Base, CXXRecordDecl *RDecl) { const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); - if (BaseType && - BaseType->getQualifiedNameAsString() == "enable_shared_from_this") { - const clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); + const clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); + + if ( BaseType && BaseType->getQualifiedNameAsString() == + "enable_shared_from_this") { if (AccessSpec == clang::AS_public) { const SourceLocation InsertLocation = Base.getBaseTypeLoc(); const FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::"); - diag(MissingStdSharedClassDecl->getLocation(), - "Should be std::enable_shared_from_this", DiagnosticIDs::Warning) + Check.diag(RDecl->getLocation(), + "Should be std::enable_shared_from_this", DiagnosticIDs::Warning) << Hint; - break; + } else { const SourceRange ReplacementRange = Base.getSourceRange(); const std::string ReplacementString = "public std::" + Base.getType().getAsString(); const FixItHint Hint = FixItHint::CreateReplacement(ReplacementRange, ReplacementString); - diag(MissingStdSharedClassDecl->getLocation(), - "Should be std::enable_shared_from_this and " - "inheritance from std::enable_shared_from_this should be public " - "inheritance, otherwise the internal weak_ptr won't be " - "initialized", - DiagnosticIDs::Warning) + Check.diag(RDecl->getLocation(), + "Should be std::enable_shared_from_this and " + "inheritance from std::enable_shared_from_this should be public " + "inheritance, otherwise the internal weak_ptr won't be " + "initialized", + DiagnosticIDs::Warning) << Hint; - break; } } + else if ( BaseType && BaseType->getQualifiedNameAsString() == + "std::enable_shared_from_this") { + if (AccessSpec != clang::AS_public) { + const SourceRange ReplacementRange = Base.getSourceRange(); + const std::string ReplacementString = + "public " + Base.getType().getAsString(); + const FixItHint Hint = + FixItHint::CreateReplacement(ReplacementRange, ReplacementString); + Check.diag( + RDecl->getLocation(), + "inheritance from std::enable_shared_from_this should be public " + "inheritance, otherwise the internal weak_ptr won't be initialized", + DiagnosticIDs::Warning) + << Hint; + } + } + else if (EnableSharedClassSet.contains(BaseType->getCanonicalDecl())) { + if (AccessSpec != clang::AS_public) { + const SourceRange ReplacementRange = Base.getSourceRange(); + const std::string ReplacementString = + "public " + Base.getType().getAsString(); + const FixItHint Hint = + FixItHint::CreateReplacement(ReplacementRange, ReplacementString); + Check.diag( + RDecl->getLocation(), + "inheritance from std::enable_shared_from_this should be public " + "inheritance, otherwise the internal weak_ptr won't be initialized", + DiagnosticIDs::Warning) + << Hint; + } + } + return true; } - } + }; + + Visitor(*this).TraverseAST(*Result.Context); + } } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp index d2d328596c0416..399d35d0d6bfe4 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -51,6 +51,22 @@ class BadMixedProblemExample : enable_shared_from_this<BadMixedProblemExample> { // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Should be std::enable_shared_from_this and inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] // CHECK-FIXES: public std::enable_shared_from_this<BadMixedProblemExample> -//FIXME: can't do anything about this, clang-check -ast-dump doesn't show A's internals in class B's AST -class A : public std::enable_shared_from_this<A> {}; -class B : private A{}; +class ClassBase : public std::enable_shared_from_this<ClassBase> {}; +class PrivateInheritClassBase : private ClassBase{}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase{}; + +class DefaultInheritClassBase : ClassBase{}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: class DefaultInheritClassBase : public ClassBase{}; + +class PublicInheritClassBase : public ClassBase{}; + +struct StructBase : public std::enable_shared_from_this<StructBase> {}; +struct PrivateInheritStructBase : private StructBase{}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] +// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase{}; + +struct DefaultInheritStructBase : StructBase{}; + +struct PublicInheritStructBase : StructBase{}; >From 89f58ce0031713765e2daed596510abc2e61f6ac Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Wed, 14 Aug 2024 18:05:51 +0800 Subject: [PATCH 09/11] Fix test case formatting --- .../incorrect-enable-shared-from-this.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp index 399d35d0d6bfe4..11975c4887ade7 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -52,21 +52,21 @@ class BadMixedProblemExample : enable_shared_from_this<BadMixedProblemExample> { // CHECK-FIXES: public std::enable_shared_from_this<BadMixedProblemExample> class ClassBase : public std::enable_shared_from_this<ClassBase> {}; -class PrivateInheritClassBase : private ClassBase{}; +class PrivateInheritClassBase : private ClassBase {}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] -// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase{}; +// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase {}; -class DefaultInheritClassBase : ClassBase{}; +class DefaultInheritClassBase : ClassBase {}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] -// CHECK-FIXES: class DefaultInheritClassBase : public ClassBase{}; +// CHECK-FIXES: class DefaultInheritClassBase : public ClassBase {}; -class PublicInheritClassBase : public ClassBase{}; +class PublicInheritClassBase : public ClassBase {}; struct StructBase : public std::enable_shared_from_this<StructBase> {}; -struct PrivateInheritStructBase : private StructBase{}; +struct PrivateInheritStructBase : private StructBase {}; // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] -// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase{}; +// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase {}; -struct DefaultInheritStructBase : StructBase{}; +struct DefaultInheritStructBase : StructBase {}; -struct PublicInheritStructBase : StructBase{}; +struct PublicInheritStructBase : StructBase {}; >From 9eb379da5412d42958223a048967a7c30d932092 Mon Sep 17 00:00:00 2001 From: MichelleCDjunaidi <87893361+michellecdjuna...@users.noreply.github.com> Date: Wed, 14 Aug 2024 18:08:52 +0800 Subject: [PATCH 10/11] Fix check code formatting to follow LLVM style --- .../IncorrectEnableSharedFromThisCheck.cpp | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index bd7c485ccba921..54a8e542fbe0bc 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -7,8 +7,8 @@ //===----------------------------------------------------------------------===// #include "IncorrectEnableSharedFromThisCheck.h" -#include "clang/AST/DeclCXX.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/Specifiers.h" @@ -18,21 +18,21 @@ using namespace clang::ast_matchers; namespace clang::tidy::bugprone { -void IncorrectEnableSharedFromThisCheck::registerMatchers( - MatchFinder *Finder) { +void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(translationUnitDecl(), this); } void IncorrectEnableSharedFromThisCheck::check( - const MatchFinder::MatchResult &Result) { + const MatchFinder::MatchResult &Result) { class Visitor : public RecursiveASTVisitor<Visitor> { IncorrectEnableSharedFromThisCheck &Check; - llvm::SmallPtrSet<const CXXRecordDecl*, 16> EnableSharedClassSet; + llvm::SmallPtrSet<const CXXRecordDecl *, 16> EnableSharedClassSet; public: - explicit Visitor(IncorrectEnableSharedFromThisCheck &Check) : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {} - + explicit Visitor(IncorrectEnableSharedFromThisCheck &Check) + : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {} + bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) { for (const auto &Base : RDecl->bases()) { VisitCXXBaseSpecifier(Base, RDecl); @@ -44,29 +44,32 @@ void IncorrectEnableSharedFromThisCheck::check( EnableSharedClassSet.insert(RDecl->getCanonicalDecl()); } } - return true; + return true; } - bool VisitCXXBaseSpecifier(const CXXBaseSpecifier &Base, CXXRecordDecl *RDecl) { + bool VisitCXXBaseSpecifier(const CXXBaseSpecifier &Base, + CXXRecordDecl *RDecl) { const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl(); const clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier(); - if ( BaseType && BaseType->getQualifiedNameAsString() == - "enable_shared_from_this") { + if (BaseType && + BaseType->getQualifiedNameAsString() == "enable_shared_from_this") { if (AccessSpec == clang::AS_public) { const SourceLocation InsertLocation = Base.getBaseTypeLoc(); const FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::"); Check.diag(RDecl->getLocation(), - "Should be std::enable_shared_from_this", DiagnosticIDs::Warning) + "Should be std::enable_shared_from_this", + DiagnosticIDs::Warning) << Hint; - + } else { const SourceRange ReplacementRange = Base.getSourceRange(); const std::string ReplacementString = "public std::" + Base.getType().getAsString(); const FixItHint Hint = FixItHint::CreateReplacement(ReplacementRange, ReplacementString); - Check.diag(RDecl->getLocation(), + Check.diag( + RDecl->getLocation(), "Should be std::enable_shared_from_this and " "inheritance from std::enable_shared_from_this should be public " "inheritance, otherwise the internal weak_ptr won't be " @@ -74,9 +77,8 @@ void IncorrectEnableSharedFromThisCheck::check( DiagnosticIDs::Warning) << Hint; } - } - else if ( BaseType && BaseType->getQualifiedNameAsString() == - "std::enable_shared_from_this") { + } else if (BaseType && BaseType->getQualifiedNameAsString() == + "std::enable_shared_from_this") { if (AccessSpec != clang::AS_public) { const SourceRange ReplacementRange = Base.getSourceRange(); const std::string ReplacementString = @@ -86,12 +88,12 @@ void IncorrectEnableSharedFromThisCheck::check( Check.diag( RDecl->getLocation(), "inheritance from std::enable_shared_from_this should be public " - "inheritance, otherwise the internal weak_ptr won't be initialized", + "inheritance, otherwise the internal weak_ptr won't be " + "initialized", DiagnosticIDs::Warning) << Hint; } - } - else if (EnableSharedClassSet.contains(BaseType->getCanonicalDecl())) { + } else if (EnableSharedClassSet.contains(BaseType->getCanonicalDecl())) { if (AccessSpec != clang::AS_public) { const SourceRange ReplacementRange = Base.getSourceRange(); const std::string ReplacementString = @@ -101,17 +103,17 @@ void IncorrectEnableSharedFromThisCheck::check( Check.diag( RDecl->getLocation(), "inheritance from std::enable_shared_from_this should be public " - "inheritance, otherwise the internal weak_ptr won't be initialized", + "inheritance, otherwise the internal weak_ptr won't be " + "initialized", DiagnosticIDs::Warning) << Hint; - } + } } return true; } }; - + Visitor(*this).TraverseAST(*Result.Context); - } } // namespace clang::tidy::bugprone >From 12aa8c0e6d2639af8aaf15400907ad02e83ef6ce Mon Sep 17 00:00:00 2001 From: Michelle C Djunaidi <michellechrisa...@gmail.com> Date: Thu, 15 Aug 2024 14:53:50 +0800 Subject: [PATCH 11/11] Optimize check and cleanup llvm-lit flags --- .../bugprone/IncorrectEnableSharedFromThisCheck.cpp | 3 ++- .../checkers/bugprone/incorrect-enable-shared-from-this.cpp | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp index 54a8e542fbe0bc..5925bcf389c867 100644 --- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp @@ -31,7 +31,7 @@ void IncorrectEnableSharedFromThisCheck::check( public: explicit Visitor(IncorrectEnableSharedFromThisCheck &Check) - : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {} + : Check(Check) {} bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) { for (const auto &Base : RDecl->bases()) { @@ -42,6 +42,7 @@ void IncorrectEnableSharedFromThisCheck::check( if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { EnableSharedClassSet.insert(RDecl->getCanonicalDecl()); + return true; } } return true; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp index 11975c4887ade7..c984d6ef39c6a6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp @@ -1,8 +1,10 @@ -// RUN: %check_clang_tidy %s bugprone-incorrect-enable-shared-from-this %t -- -- +// RUN: %check_clang_tidy -std=c++11 %s bugprone-incorrect-enable-shared-from-this %t +// NOLINTBEGIN namespace std { template <typename T> class enable_shared_from_this {}; } //namespace std +// NOLINTEND class BadClassExample : std::enable_shared_from_this<BadClassExample> {}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: inheritance from std::enable_shared_from_this should be public inheritance, otherwise the internal weak_ptr won't be initialized [bugprone-incorrect-enable-shared-from-this] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits