[clang-tools-extra] [clang-tidy] Create bugprone-incorrect-enable-shared-from-this check (PR #102299)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Nov 9 05:20:56 PST 2024
https://github.com/MichelleCDjunaidi updated https://github.com/llvm/llvm-project/pull/102299
>From 062fcab3717d848e8f523dd7a14433b09a9d8142 Mon Sep 17 00:00:00 2001
From: Michelle C Djunaidi <michellechrisalyn at gmail.com>
Date: Wed, 7 Aug 2024 13:10:02 +0800
Subject: [PATCH 01/29] [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 9120c4b6c0d9ae..b0a411039f8768 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -54,6 +54,7 @@
#include "ParentVirtualCallCheck.h"
#include "PointerArithmeticOnPolymorphicObjectCheck.h"
#include "PosixReturnCheck.h"
+#include "PublicEnableSharedFromThisCheck.h"
#include "RedundantBranchConditionCheck.h"
#include "ReservedIdentifierCheck.h"
#include "ReturnConstRefFromParameterCheck.h"
@@ -143,6 +144,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 f0667bbfdd87f7..fe3891899372fd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -27,6 +27,7 @@ add_clang_library(clangTidyBugproneModule STATIC
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 876689c40fcdb2..6bbd78fbd47cf2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -131,6 +131,11 @@ New checks
Finds cases when an uninstantiated virtual member function in a template class
causes cross-compiler incompatibility.
+- 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 0082234f5ed31b..cfc74a10dbfde8 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -121,6 +121,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 acf73fd6f8c9288e33416d0659bfb7bc9edf3a62 Mon Sep 17 00:00:00 2001
From: Michelle C Djunaidi <michellechrisalyn at gmail.com>
Date: Wed, 7 Aug 2024 17:58:42 +0800
Subject: [PATCH 02/29] 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 52186f8c334e8acd1799f80763e0313cde0385f4 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Thu, 8 Aug 2024 13:44:15 +0800
Subject: [PATCH 03/29] 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 b0a411039f8768..2f5104de000b5d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -54,7 +54,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"
@@ -144,8 +144,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 fe3891899372fd..0180a104ff0e35 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -27,7 +27,7 @@ add_clang_library(clangTidyBugproneModule STATIC
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 6bbd78fbd47cf2..00eb002a8e052f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -131,10 +131,12 @@ New checks
Finds cases when an uninstantiated virtual member function in a template class
causes cross-compiler incompatibility.
-- 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 cfc74a10dbfde8..3d07afaa5cd72b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -101,6 +101,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>`,
@@ -121,7 +122,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 2f05bbc85b62a81e74e106cc9f84120f0d493330 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Thu, 8 Aug 2024 17:35:24 +0800
Subject: [PATCH 04/29] 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 00eb002a8e052f..49b05d3d9d3e9f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,7 +136,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 772a8ebaf65a62a49ccceb8c15e23487e27e78b3 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Thu, 8 Aug 2024 17:40:24 +0800
Subject: [PATCH 05/29] 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 7aa92320203387370844d025d4f0af782e3c8724 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Mon, 12 Aug 2024 10:35:26 +0800
Subject: [PATCH 06/29] 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 47a6f64bbf90d1f83956329a9bf90f82a3768fe3 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Tue, 13 Aug 2024 10:30:15 +0800
Subject: [PATCH 07/29] 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 2f5104de000b5d..41362195117f77 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -34,6 +34,7 @@
#include "InaccurateEraseCheck.h"
#include "IncDecInConditionsCheck.h"
#include "IncorrectEnableIfCheck.h"
+#include "IncorrectEnableSharedFromThisCheck.h"
#include "IncorrectRoundingsCheck.h"
#include "InfiniteLoopCheck.h"
#include "IntegerDivisionCheck.h"
@@ -54,7 +55,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 65eb8244d1144c36da43b240bb0bad6191c11ac4 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Wed, 14 Aug 2024 18:03:34 +0800
Subject: [PATCH 08/29] 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 a61eff908d5f6bb92d9c8757761aedbce40e35c8 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Wed, 14 Aug 2024 18:05:51 +0800
Subject: [PATCH 09/29] 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 5d489b80d78a97a48cebee258c65faabf4f71ac5 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Wed, 14 Aug 2024 18:08:52 +0800
Subject: [PATCH 10/29] 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 9bbc97b5b44d48a5e447fe6ee9327675638fac78 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Fri, 16 Aug 2024 15:00:33 +0800
Subject: [PATCH 11/29] Optimize traversal, add alias and typedef testing,
remove missing std:: check
In accordance to https://github.com/llvm/llvm-project/pull/102299#discussion_r1719046306, the
feature that was suggested in https://github.com/llvm/llvm-project/pull/102299#discussion_r1706831159
has been removed.
---
.../IncorrectEnableSharedFromThisCheck.cpp | 61 +++++-------
.../IncorrectEnableSharedFromThisCheck.h | 7 +-
clang-tools-extra/docs/ReleaseNotes.rst | 5 +-
.../incorrect-enable-shared-from-this.rst | 12 +--
.../incorrect-enable-shared-from-this.cpp | 97 ++++++++++++++-----
5 files changed, 108 insertions(+), 74 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index 54a8e542fbe0bc..7f91b53cf40e76 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -31,17 +31,20 @@ void IncorrectEnableSharedFromThisCheck::check(
public:
explicit Visitor(IncorrectEnableSharedFromThisCheck &Check)
- : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {}
+ : Check(Check) {}
- bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {
+ bool TraverseCXXRecordHelper(CXXRecordDecl *RDecl) {
+ if (!RDecl->hasDefinition()) {
+ return true;
+ }
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") {
+ if (BaseType && isStdEnableSharedFromThis(BaseType)) {
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
+ return true;
}
}
return true;
@@ -52,44 +55,21 @@ void IncorrectEnableSharedFromThisCheck::check(
const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl();
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::");
- Check.diag(RDecl->getLocation(),
- "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(),
- "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;
- }
- } else if (BaseType && BaseType->getQualifiedNameAsString() ==
- "std::enable_shared_from_this") {
+ if (BaseType && isStdEnableSharedFromThis(BaseType)) {
if (AccessSpec != clang::AS_public) {
const SourceRange ReplacementRange = Base.getSourceRange();
const std::string ReplacementString =
+ // Base.getType().getAsString() results in
+ // std::enable_shared_from_this<ClassName> or alias/typedefs of
+ // std::enable_shared_from_this<ClassName>
"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",
+ "this is not publicly inheriting from "
+ "std::enable_shared_from_this, will cause unintended behaviour "
+ "on shared_from_this. fix this by making it public inheritance",
DiagnosticIDs::Warning)
<< Hint;
}
@@ -102,15 +82,22 @@ void IncorrectEnableSharedFromThisCheck::check(
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",
+ "this is not publicly inheriting from "
+ "std::enable_shared_from_this, will cause unintended behaviour "
+ "on shared_from_this. fix this by making it public inheritance",
DiagnosticIDs::Warning)
<< Hint;
}
}
return true;
}
+
+ private:
+ // FIXME: configure this for boost in the future
+ bool isStdEnableSharedFromThis(const CXXRecordDecl *BaseType) {
+ return BaseType->getName() == "enable_shared_from_this" &&
+ BaseType->isInStdNamespace();
+ }
};
Visitor(*this).TraverseAST(*Result.Context);
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
index b7b15e90b1cd59..5daff7327d926f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
@@ -13,10 +13,9 @@
namespace clang::tidy::bugprone {
-/// 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``.
+// Checks if class/struct publicly inherits from
+// ``std::enable_shared_from_this``, because otherwise when ``shared_from_this``
+// is called unintended behaviour will occur
///
/// 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/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 49b05d3d9d3e9f..dbaaa336a13afd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -135,9 +135,8 @@ New checks
<clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
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``. Also checks for ``std::`` in
- ``std::enable_shared_from_this``.
+ because otherwise when ``shared_from_this`` is called unintended behaviour will
+ occur.
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 058eb046a4eb47..761e760bfdfa67 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
@@ -5,7 +5,7 @@ bugprone-incorrect-enable-shared-from-this
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``.
+is called unintended behaviour will occur.
Consider the following code:
@@ -15,7 +15,8 @@ Consider the following code:
// private inheritance
class BadExample : std::enable_shared_from_this<BadExample> {
- // ``shared_from_this``` returns uninitialized ``weak_ptr``
+ // ``shared_from_this``` unintended behaviour
+ // libstd implementation returns uninitialized ``weak_ptr``
public:
BadExample* foo() { return shared_from_this().get(); }
void bar() { return; }
@@ -27,7 +28,6 @@ Consider the following code:
b_ex->bar();
}
-When ``using_not_public()`` is called, this code will crash without exception
-handling.
-
-Also checks for ``std::`` in ``std::enable_shared_from_this``.
+Using ``libstd`` implementation, `shared_from_this`` will throw
+``std::bad_weak_ptr``. When ``using_not_public()`` is called, this code will
+crash without exception handling.
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..57e6ef0cda3e8c 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,21 @@
-// 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]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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]]: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-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadStructExample>
class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> {};
@@ -26,47 +28,94 @@ 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-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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 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<BadInitClassExample>
-
-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<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]
-// CHECK-FIXES: public std::enable_shared_from_this<BadMixedProblemExample>
-
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-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase {};
struct DefaultInheritStructBase : StructBase {};
struct PublicInheritStructBase : StructBase {};
+
+//alias the template itself
+template <typename T> using esft_template = std::enable_shared_from_this<T>;
+
+class PrivateAliasTemplateClassBase : private esft_template<PrivateAliasTemplateClassBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class PrivateAliasTemplateClassBase : public esft_template<PrivateAliasTemplateClassBase> {};
+
+class DefaultAliasTemplateClassBase : esft_template<DefaultAliasTemplateClassBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class DefaultAliasTemplateClassBase : public esft_template<DefaultAliasTemplateClassBase> {};
+
+class PublicAliasTemplateClassBase : public esft_template<PublicAliasTemplateClassBase> {};
+
+struct PrivateAliasTemplateStructBase : private esft_template<PrivateAliasTemplateStructBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: struct PrivateAliasTemplateStructBase : public esft_template<PrivateAliasTemplateStructBase> {};
+
+struct DefaultAliasTemplateStructBase : esft_template<DefaultAliasTemplateStructBase> {};
+
+struct PublicAliasTemplateStructBase : public esft_template<PublicAliasTemplateStructBase> {};
+
+//alias with specific instance
+using esft = std::enable_shared_from_this<ClassBase>;
+class PrivateAliasClassBase : private esft {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class PrivateAliasClassBase : public esft {};
+
+class DefaultAliasClassBase : esft {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class DefaultAliasClassBase : public esft {};
+
+class PublicAliasClassBase : public esft {};
+
+struct PrivateAliasStructBase : private esft {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: struct PrivateAliasStructBase : public esft {};
+
+struct DefaultAliasStructBase : esft {};
+
+struct PublicAliasStructBase : public esft {};
+
+//we can only typedef a specific instance of the template
+typedef std::enable_shared_from_this<ClassBase> EnableSharedFromThis;
+class PrivateTypedefClassBase : private EnableSharedFromThis {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class PrivateTypedefClassBase : public EnableSharedFromThis {};
+
+class DefaultTypedefClassBase : EnableSharedFromThis {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class DefaultTypedefClassBase : public EnableSharedFromThis {};
+
+class PublicTypedefClassBase : public EnableSharedFromThis {};
+
+struct PrivateTypedefStructBase : private EnableSharedFromThis {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: struct PrivateTypedefStructBase : public EnableSharedFromThis {};
+
+struct DefaultTypedefStructBase : EnableSharedFromThis {};
+
+struct PublicTypedefStructBase : public EnableSharedFromThis {};
>From 3f845747fcd713ac4e8e90848bd9f0aaaebca113 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Mon, 26 Aug 2024 13:33:43 +0800
Subject: [PATCH 12/29] refactor: simplify to VisitCXXRecordDecl only instead
of TraverseCXXRecordDecl, VisitCXXRecordDecl, VisitCXXBaseSpecifier
Logic remains unchanged.
---
.../IncorrectEnableSharedFromThisCheck.cpp | 86 +++++++------------
1 file changed, 31 insertions(+), 55 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index 7f91b53cf40e76..63176f20b066d0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -33,60 +33,34 @@ void IncorrectEnableSharedFromThisCheck::check(
explicit Visitor(IncorrectEnableSharedFromThisCheck &Check)
: Check(Check) {}
- bool TraverseCXXRecordHelper(CXXRecordDecl *RDecl) {
- if (!RDecl->hasDefinition()) {
- return true;
- }
- for (const auto &Base : RDecl->bases()) {
- VisitCXXBaseSpecifier(Base, RDecl);
- }
- for (const auto &Base : RDecl->bases()) {
- const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl();
- if (BaseType && isStdEnableSharedFromThis(BaseType)) {
- EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
- return true;
- }
- }
- return true;
- }
+ bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {
+ if (isStdEnableSharedFromThis(RDecl))
+ EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
- bool VisitCXXBaseSpecifier(const CXXBaseSpecifier &Base,
- CXXRecordDecl *RDecl) {
- const CXXRecordDecl *BaseType = Base.getType()->getAsCXXRecordDecl();
- const clang::AccessSpecifier AccessSpec = Base.getAccessSpecifier();
+ for (const auto &Base : RDecl->bases()) {
+ const auto *BaseRecord =
+ Base.getType()->getAsCXXRecordDecl()->getCanonicalDecl();
- if (BaseType && isStdEnableSharedFromThis(BaseType)) {
- if (AccessSpec != clang::AS_public) {
- const SourceRange ReplacementRange = Base.getSourceRange();
- const std::string ReplacementString =
- // Base.getType().getAsString() results in
- // std::enable_shared_from_this<ClassName> or alias/typedefs of
- // std::enable_shared_from_this<ClassName>
- "public " + Base.getType().getAsString();
- const FixItHint Hint =
- FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
- Check.diag(
- RDecl->getLocation(),
- "this is not publicly inheriting from "
- "std::enable_shared_from_this, will cause unintended behaviour "
- "on shared_from_this. fix this by making it public inheritance",
- 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(),
- "this is not publicly inheriting from "
- "std::enable_shared_from_this, will cause unintended behaviour "
- "on shared_from_this. fix this by making it public inheritance",
- DiagnosticIDs::Warning)
- << Hint;
+ if (EnableSharedClassSet.contains(BaseRecord) ||
+ isStdEnableSharedFromThis(BaseRecord)) {
+ if (Base.getAccessSpecifier() != clang::AS_public) {
+ const SourceRange ReplacementRange = Base.getSourceRange();
+ const std::string ReplacementString =
+ // Base.getType().getAsString() results in
+ // std::enable_shared_from_this<ClassName> or
+ // alias/typedefs of std::enable_shared_from_this<ClassName>
+ "public " + Base.getType().getAsString();
+ const FixItHint Hint = FixItHint::CreateReplacement(
+ ReplacementRange, ReplacementString);
+ Check.diag(
+ RDecl->getLocation(),
+ "this is not publicly inheriting from "
+ "std::enable_shared_from_this, will cause unintended behaviour "
+ "on shared_from_this. fix this by making it public inheritance",
+ DiagnosticIDs::Warning)
+ << Hint;
+ }
+ EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
}
}
return true;
@@ -94,9 +68,11 @@ void IncorrectEnableSharedFromThisCheck::check(
private:
// FIXME: configure this for boost in the future
- bool isStdEnableSharedFromThis(const CXXRecordDecl *BaseType) {
- return BaseType->getName() == "enable_shared_from_this" &&
- BaseType->isInStdNamespace();
+ bool isStdEnableSharedFromThis(const CXXRecordDecl *RDecl) {
+ // this is the equivalent of
+ // RDecl->getQualifiedNameAsString() == "std::enable_shared_from_this"
+ return RDecl->getName() == "enable_shared_from_this" &&
+ RDecl->isInStdNamespace();
}
};
>From 7f964eef320f7f2ba0ed688a3ebc628bbff94f9a Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Mon, 26 Aug 2024 13:51:34 +0800
Subject: [PATCH 13/29] refactor: exclude non-definitions from repeated
processing
---
.../bugprone/IncorrectEnableSharedFromThisCheck.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index 63176f20b066d0..860fb0f9b0747a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -34,6 +34,10 @@ void IncorrectEnableSharedFromThisCheck::check(
: Check(Check) {}
bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {
+ if (!RDecl->hasDefinition()) {
+ return true;
+ }
+
if (isStdEnableSharedFromThis(RDecl))
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
>From c7d8559140a10cc84898eab93b72ad6d260803bf Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <87893361+MichelleCDjunaidi at users.noreply.github.com>
Date: Mon, 26 Aug 2024 14:41:25 +0800
Subject: [PATCH 14/29] more detailed diag for classes with bases from
EnableSharedClassSet
---
.../IncorrectEnableSharedFromThisCheck.cpp | 21 ++++++++++++++++++-
.../incorrect-enable-shared-from-this.cpp | 6 +++---
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index 860fb0f9b0747a..cc08bf4ea8bc9a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -47,7 +47,9 @@ void IncorrectEnableSharedFromThisCheck::check(
if (EnableSharedClassSet.contains(BaseRecord) ||
isStdEnableSharedFromThis(BaseRecord)) {
- if (Base.getAccessSpecifier() != clang::AS_public) {
+
+ if (isStdEnableSharedFromThis(BaseRecord) &&
+ Base.getAccessSpecifier() != clang::AS_public) {
const SourceRange ReplacementRange = Base.getSourceRange();
const std::string ReplacementString =
// Base.getType().getAsString() results in
@@ -63,7 +65,24 @@ void IncorrectEnableSharedFromThisCheck::check(
"on shared_from_this. fix this by making it public inheritance",
DiagnosticIDs::Warning)
<< Hint;
+ } else if (EnableSharedClassSet.contains(BaseRecord) &&
+ Base.getAccessSpecifier() != clang::AS_public) {
+ // actually the same FixItHint, just different diag
+ const SourceRange ReplacementRange = Base.getSourceRange();
+ const std::string ReplacementString =
+ "public " + Base.getType().getAsString();
+ const FixItHint Hint = FixItHint::CreateReplacement(
+ ReplacementRange, ReplacementString);
+ Check.diag(
+ RDecl->getLocation(),
+ "this is not publicly inheriting from %0 "
+ "which inherits from std::enable_shared_from_this, "
+ "will cause unintended behaviour "
+ "on shared_from_this. fix this by making it public inheritance",
+ DiagnosticIDs::Warning)
+ << BaseRecord->getNameAsString() << Hint;
}
+
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
}
}
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 57e6ef0cda3e8c..9801dc208f3b97 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
@@ -41,18 +41,18 @@ class BadMultiClass3Example : dummy_class1, dummy_class2, std::enable_shared_fro
class ClassBase : public std::enable_shared_from_this<ClassBase> {};
class PrivateInheritClassBase : private ClassBase {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from ClassBase which inherits from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase {};
class DefaultInheritClassBase : ClassBase {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from ClassBase which inherits from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [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: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from StructBase which inherits from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase {};
struct DefaultInheritStructBase : StructBase {};
>From 5849a106ffd8b4e91d6628600380ea492f6635cb Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <michellechrisalyn at gmail.com>
Date: Thu, 12 Sep 2024 16:32:24 +0800
Subject: [PATCH 15/29] refactor to use diag %select, fix docs typo
---
.../IncorrectEnableSharedFromThisCheck.cpp | 40 +++++++------------
.../incorrect-enable-shared-from-this.rst | 2 +-
2 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index cc08bf4ea8bc9a..56d8b9f17708dc 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -44,12 +44,13 @@ void IncorrectEnableSharedFromThisCheck::check(
for (const auto &Base : RDecl->bases()) {
const auto *BaseRecord =
Base.getType()->getAsCXXRecordDecl()->getCanonicalDecl();
+ const auto isStdEnableSharedFromThisBool =
+ isStdEnableSharedFromThis(BaseRecord);
if (EnableSharedClassSet.contains(BaseRecord) ||
- isStdEnableSharedFromThis(BaseRecord)) {
+ isStdEnableSharedFromThisBool) {
- if (isStdEnableSharedFromThis(BaseRecord) &&
- Base.getAccessSpecifier() != clang::AS_public) {
+ if (Base.getAccessSpecifier() != clang::AS_public) {
const SourceRange ReplacementRange = Base.getSourceRange();
const std::string ReplacementString =
// Base.getType().getAsString() results in
@@ -58,29 +59,18 @@ void IncorrectEnableSharedFromThisCheck::check(
"public " + Base.getType().getAsString();
const FixItHint Hint = FixItHint::CreateReplacement(
ReplacementRange, ReplacementString);
- Check.diag(
- RDecl->getLocation(),
- "this is not publicly inheriting from "
- "std::enable_shared_from_this, will cause unintended behaviour "
- "on shared_from_this. fix this by making it public inheritance",
- DiagnosticIDs::Warning)
+ Check.diag(RDecl->getLocation(),
+ "%2 is not publicly inheriting from "
+ "%select{%1|'std::enable_shared_from_this',}0 "
+ "%select{which inherits from "
+ "'std::enable_shared_from_this', |}0 "
+ "will cause unintended behaviour "
+ "on 'shared_from_this'. fix this by making it public "
+ "inheritance",
+ DiagnosticIDs::Warning)
+ << isStdEnableSharedFromThisBool
+ << BaseRecord->getNameAsString() << RDecl->getNameAsString()
<< Hint;
- } else if (EnableSharedClassSet.contains(BaseRecord) &&
- Base.getAccessSpecifier() != clang::AS_public) {
- // actually the same FixItHint, just different diag
- const SourceRange ReplacementRange = Base.getSourceRange();
- const std::string ReplacementString =
- "public " + Base.getType().getAsString();
- const FixItHint Hint = FixItHint::CreateReplacement(
- ReplacementRange, ReplacementString);
- Check.diag(
- RDecl->getLocation(),
- "this is not publicly inheriting from %0 "
- "which inherits from std::enable_shared_from_this, "
- "will cause unintended behaviour "
- "on shared_from_this. fix this by making it public inheritance",
- DiagnosticIDs::Warning)
- << BaseRecord->getNameAsString() << Hint;
}
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
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 761e760bfdfa67..90336f3a9c9e88 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
@@ -28,6 +28,6 @@ Consider the following code:
b_ex->bar();
}
-Using ``libstd`` implementation, `shared_from_this`` will throw
+Using ``libstd`` implementation, ``shared_from_this`` will throw
``std::bad_weak_ptr``. When ``using_not_public()`` is called, this code will
crash without exception handling.
>From 15b381a783b1189d85891fad5b7421d655b7df75 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <michellechrisalyn at gmail.com>
Date: Thu, 12 Sep 2024 16:41:28 +0800
Subject: [PATCH 16/29] add space to code block in docs
---
.../checks/bugprone/incorrect-enable-shared-from-this.rst | 1 +
1 file changed, 1 insertion(+)
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 90336f3a9c9e88..a0832a0010bb55 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
@@ -10,6 +10,7 @@ is called unintended behaviour will occur.
Consider the following code:
.. code-block:: c++
+
#include <memory>
// private inheritance
>From b95ad5da13df407378d65351d4a0d4376d5f07f1 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <michellechrisalyn at gmail.com>
Date: Thu, 12 Sep 2024 17:07:09 +0800
Subject: [PATCH 17/29] update test CHECK-MESSAGES to check for class name
---
.../incorrect-enable-shared-from-this.cpp | 36 +++++++++----------
1 file changed, 18 insertions(+), 18 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 9801dc208f3b97..72cdb00e1d9059 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
@@ -7,15 +7,15 @@ namespace std {
// NOLINTEND
class BadClassExample : std::enable_shared_from_this<BadClassExample> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadClassExample is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [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: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadClass2Example is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [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]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: BadStructExample is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadStructExample>
class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> {};
@@ -28,31 +28,31 @@ class dummy_class1 {};
class dummy_class2 {};
class BadMultiClassExample : std::enable_shared_from_this<BadMultiClassExample>, dummy_class1 {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadMultiClassExample is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [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: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadMultiClass2Example is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [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: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadMultiClass3Example is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: dummy_class1, dummy_class2, public std::enable_shared_from_this<BadMultiClass3Example>
class ClassBase : public std::enable_shared_from_this<ClassBase> {};
class PrivateInheritClassBase : private ClassBase {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from ClassBase which inherits from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateInheritClassBase is not publicly inheriting from ClassBase which inherits from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase {};
class DefaultInheritClassBase : ClassBase {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from ClassBase which inherits from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultInheritClassBase is not publicly inheriting from ClassBase which inherits from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [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: this is not publicly inheriting from StructBase which inherits from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateInheritStructBase is not publicly inheriting from StructBase which inherits from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase {};
struct DefaultInheritStructBase : StructBase {};
@@ -63,17 +63,17 @@ struct PublicInheritStructBase : StructBase {};
template <typename T> using esft_template = std::enable_shared_from_this<T>;
class PrivateAliasTemplateClassBase : private esft_template<PrivateAliasTemplateClassBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateAliasTemplateClassBase : public esft_template<PrivateAliasTemplateClassBase> {};
class DefaultAliasTemplateClassBase : esft_template<DefaultAliasTemplateClassBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultAliasTemplateClassBase : public esft_template<DefaultAliasTemplateClassBase> {};
class PublicAliasTemplateClassBase : public esft_template<PublicAliasTemplateClassBase> {};
struct PrivateAliasTemplateStructBase : private esft_template<PrivateAliasTemplateStructBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateAliasTemplateStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateAliasTemplateStructBase : public esft_template<PrivateAliasTemplateStructBase> {};
struct DefaultAliasTemplateStructBase : esft_template<DefaultAliasTemplateStructBase> {};
@@ -83,17 +83,17 @@ struct PublicAliasTemplateStructBase : public esft_template<PublicAliasTemplateS
//alias with specific instance
using esft = std::enable_shared_from_this<ClassBase>;
class PrivateAliasClassBase : private esft {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateAliasClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateAliasClassBase : public esft {};
class DefaultAliasClassBase : esft {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultAliasClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultAliasClassBase : public esft {};
class PublicAliasClassBase : public esft {};
struct PrivateAliasStructBase : private esft {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateAliasStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateAliasStructBase : public esft {};
struct DefaultAliasStructBase : esft {};
@@ -103,17 +103,17 @@ struct PublicAliasStructBase : public esft {};
//we can only typedef a specific instance of the template
typedef std::enable_shared_from_this<ClassBase> EnableSharedFromThis;
class PrivateTypedefClassBase : private EnableSharedFromThis {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateTypedefClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateTypedefClassBase : public EnableSharedFromThis {};
class DefaultTypedefClassBase : EnableSharedFromThis {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultTypedefClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultTypedefClassBase : public EnableSharedFromThis {};
class PublicTypedefClassBase : public EnableSharedFromThis {};
struct PrivateTypedefStructBase : private EnableSharedFromThis {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: this is not publicly inheriting from std::enable_shared_from_this, will cause unintended behaviour on shared_from_this. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateTypedefStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateTypedefStructBase : public EnableSharedFromThis {};
struct DefaultTypedefStructBase : EnableSharedFromThis {};
>From 05017dee5bc44f033e6f4465bde5b8c5204519ff Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <michellechrisalyn at gmail.com>
Date: Sun, 22 Sep 2024 12:57:17 +0800
Subject: [PATCH 18/29] [NFC] in progress change diag
---
.../IncorrectEnableSharedFromThisCheck.cpp | 23 +++++-----
.../IncorrectEnableSharedFromThisCheck.h | 2 +-
.../incorrect-enable-shared-from-this.rst | 8 ++--
.../incorrect-enable-shared-from-this.cpp | 44 +++++++++++++++++++
4 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index 56d8b9f17708dc..f1bfd43be3d51e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -34,9 +34,8 @@ void IncorrectEnableSharedFromThisCheck::check(
: Check(Check) {}
bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {
- if (!RDecl->hasDefinition()) {
+ if (!RDecl->hasDefinition())
return true;
- }
if (isStdEnableSharedFromThis(RDecl))
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
@@ -44,11 +43,11 @@ void IncorrectEnableSharedFromThisCheck::check(
for (const auto &Base : RDecl->bases()) {
const auto *BaseRecord =
Base.getType()->getAsCXXRecordDecl()->getCanonicalDecl();
- const auto isStdEnableSharedFromThisBool =
+ const auto IsStdEnableSharedFromThisBool =
isStdEnableSharedFromThis(BaseRecord);
if (EnableSharedClassSet.contains(BaseRecord) ||
- isStdEnableSharedFromThisBool) {
+ IsStdEnableSharedFromThisBool) {
if (Base.getAccessSpecifier() != clang::AS_public) {
const SourceRange ReplacementRange = Base.getSourceRange();
@@ -61,16 +60,14 @@ void IncorrectEnableSharedFromThisCheck::check(
ReplacementRange, ReplacementString);
Check.diag(RDecl->getLocation(),
"%2 is not publicly inheriting from "
- "%select{%1|'std::enable_shared_from_this',}0 "
- "%select{which inherits from "
- "'std::enable_shared_from_this', |}0 "
- "will cause unintended behaviour "
- "on 'shared_from_this'. fix this by making it public "
- "inheritance",
+ "%select{%1 which inherits from "
+ "'std::enable_shared_from_this',|'std::enable_shared_"
+ "from_this',}0 "
+ "which will cause unintended behaviour "
+ "when using 'shared_from_this'; make the inheritance "
+ "public",
DiagnosticIDs::Warning)
- << isStdEnableSharedFromThisBool
- << BaseRecord->getNameAsString() << RDecl->getNameAsString()
- << Hint;
+ << IsStdEnableSharedFromThisBool << BaseRecord << RDecl << Hint;
}
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
index 5daff7327d926f..386383a3f16a41 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
@@ -13,7 +13,7 @@
namespace clang::tidy::bugprone {
-// Checks if class/struct publicly inherits from
+// Checks if a class or struct publicly inherits from
// ``std::enable_shared_from_this``, because otherwise when ``shared_from_this``
// is called unintended behaviour will occur
///
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 a0832a0010bb55..26425f5e48fb37 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,14 +3,14 @@
bugprone-incorrect-enable-shared-from-this
==========================================
-Checks if class/struct publicly inherits from
-``std::enable_shared_from_this``, because otherwise when ``shared_from_this``
-is called unintended behaviour will occur.
+Checks if a class or struct publicly inherits from
+``std::enable_shared_from_this``, because unintended behavior will
+otherwise occur when calling ``shared_from_this``.
Consider the following code:
.. code-block:: c++
-
+
#include <memory>
// private inheritance
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 72cdb00e1d9059..f78ce416fbb6c3 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
@@ -119,3 +119,47 @@ struct PrivateTypedefStructBase : private EnableSharedFromThis {};
struct DefaultTypedefStructBase : EnableSharedFromThis {};
struct PublicTypedefStructBase : public EnableSharedFromThis {};
+
+#define PRIVATE_ESFT_CLASS(ClassName) \
+ class ClassName: private std::enable_shared_from_this<ClassName> { \
+ };
+
+PRIVATE_ESFT_CLASS(PrivateEsftClass);
+
+class EsftClass<PrivateAliasTemplateClassBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class PrivateAliasTemplateClassBase : public EsftClass<PrivateAliasTemplateClassBase> {};
+
+#define DEFAULT_ESFT_CLASS(ClassName) \
+ class ClassName: std::enable_shared_from_this<ClassName> { \
+ };
+
+DEFAULT_ESFT_CLASS(DefaultEsftClass);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class DefaultAliasTemplateClassBase : public EsftClass<DefaultAliasTemplateClassBase> {};
+
+#define PUBLIC_ESFT_CLASS(ClassName) \
+ class ClassName: public std::enable_shared_from_this<ClassName> { \
+ };
+
+PUBLIC_ESFT_CLASS(PublicEsftClass);
+
+#define PRIVATE_ESFT_STRUCT(StructName) \
+ struct StructName: private std::enable_shared_from_this<StructName> { \
+ };
+
+PRIVATE_ESFT_STRUCT(PrivateEsftStruct);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateAliasTemplateStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: struct PrivateAliasTemplateStructBase : public EsftClass<PrivateAliasTemplateStructBase> {};
+
+#define DEFAULT_ESFT_STRUCT(StructName) \
+ struct StructName: std::enable_shared_from_this<StructName> { \
+ };
+
+DEFAULT_ESFT_STRUCT(PrivateEsftStruct);
+
+#define PUBLIC_ESFT_STRUCT(StructName) \
+ struct StructName: std::enable_shared_from_this<StructName> { \
+ };
+
+PUBLIC_ESFT_STRUCT(PublicEsftStruct);
>From a2c5cffdde5216ff0044e113de12a21dd95e1af4 Mon Sep 17 00:00:00 2001
From: MichelleCDjunaidi <michellechrisalyn at gmail.com>
Date: Mon, 23 Sep 2024 01:10:58 +0800
Subject: [PATCH 19/29] add macros to test file and revise CHECK-MESSAGES
---
.../incorrect-enable-shared-from-this.cpp | 49 +++++++++----------
1 file changed, 22 insertions(+), 27 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 f78ce416fbb6c3..cc1907eb8167cf 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
@@ -7,15 +7,15 @@ namespace std {
// NOLINTEND
class BadClassExample : std::enable_shared_from_this<BadClassExample> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadClassExample is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadClassExample' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [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: BadClass2Example is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadClass2Example' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [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]]:8: warning: BadStructExample is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'BadStructExample' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadStructExample>
class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> {};
@@ -28,31 +28,31 @@ class dummy_class1 {};
class dummy_class2 {};
class BadMultiClassExample : std::enable_shared_from_this<BadMultiClassExample>, dummy_class1 {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: BadMultiClassExample is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadMultiClassExample' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [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: BadMultiClass2Example is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadMultiClass2Example' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [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: BadMultiClass3Example is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadMultiClass3Example' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: dummy_class1, dummy_class2, public std::enable_shared_from_this<BadMultiClass3Example>
class ClassBase : public std::enable_shared_from_this<ClassBase> {};
class PrivateInheritClassBase : private ClassBase {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateInheritClassBase is not publicly inheriting from ClassBase which inherits from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateInheritClassBase' is not publicly inheriting from 'ClassBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase {};
class DefaultInheritClassBase : ClassBase {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultInheritClassBase is not publicly inheriting from ClassBase which inherits from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultInheritClassBase' is not publicly inheriting from 'ClassBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [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: PrivateInheritStructBase is not publicly inheriting from StructBase which inherits from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateInheritStructBase' is not publicly inheriting from 'StructBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase {};
struct DefaultInheritStructBase : StructBase {};
@@ -63,17 +63,17 @@ struct PublicInheritStructBase : StructBase {};
template <typename T> using esft_template = std::enable_shared_from_this<T>;
class PrivateAliasTemplateClassBase : private esft_template<PrivateAliasTemplateClassBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateAliasTemplateClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateAliasTemplateClassBase : public esft_template<PrivateAliasTemplateClassBase> {};
class DefaultAliasTemplateClassBase : esft_template<DefaultAliasTemplateClassBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultAliasTemplateClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultAliasTemplateClassBase : public esft_template<DefaultAliasTemplateClassBase> {};
class PublicAliasTemplateClassBase : public esft_template<PublicAliasTemplateClassBase> {};
struct PrivateAliasTemplateStructBase : private esft_template<PrivateAliasTemplateStructBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateAliasTemplateStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateAliasTemplateStructBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateAliasTemplateStructBase : public esft_template<PrivateAliasTemplateStructBase> {};
struct DefaultAliasTemplateStructBase : esft_template<DefaultAliasTemplateStructBase> {};
@@ -83,17 +83,17 @@ struct PublicAliasTemplateStructBase : public esft_template<PublicAliasTemplateS
//alias with specific instance
using esft = std::enable_shared_from_this<ClassBase>;
class PrivateAliasClassBase : private esft {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateAliasClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateAliasClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateAliasClassBase : public esft {};
class DefaultAliasClassBase : esft {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultAliasClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultAliasClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultAliasClassBase : public esft {};
class PublicAliasClassBase : public esft {};
struct PrivateAliasStructBase : private esft {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateAliasStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateAliasStructBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateAliasStructBase : public esft {};
struct DefaultAliasStructBase : esft {};
@@ -103,17 +103,17 @@ struct PublicAliasStructBase : public esft {};
//we can only typedef a specific instance of the template
typedef std::enable_shared_from_this<ClassBase> EnableSharedFromThis;
class PrivateTypedefClassBase : private EnableSharedFromThis {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateTypedefClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateTypedefClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateTypedefClassBase : public EnableSharedFromThis {};
class DefaultTypedefClassBase : EnableSharedFromThis {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultTypedefClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultTypedefClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultTypedefClassBase : public EnableSharedFromThis {};
class PublicTypedefClassBase : public EnableSharedFromThis {};
struct PrivateTypedefStructBase : private EnableSharedFromThis {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateTypedefStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateTypedefStructBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateTypedefStructBase : public EnableSharedFromThis {};
struct DefaultTypedefStructBase : EnableSharedFromThis {};
@@ -125,18 +125,14 @@ struct PublicTypedefStructBase : public EnableSharedFromThis {};
};
PRIVATE_ESFT_CLASS(PrivateEsftClass);
-
-class EsftClass<PrivateAliasTemplateClassBase> {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: PrivateAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
-// CHECK-FIXES: class PrivateAliasTemplateClassBase : public EsftClass<PrivateAliasTemplateClassBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'PrivateEsftClass' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
#define DEFAULT_ESFT_CLASS(ClassName) \
class ClassName: std::enable_shared_from_this<ClassName> { \
};
DEFAULT_ESFT_CLASS(DefaultEsftClass);
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: DefaultAliasTemplateClassBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
-// CHECK-FIXES: class DefaultAliasTemplateClassBase : public EsftClass<DefaultAliasTemplateClassBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'DefaultEsftClass' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
#define PUBLIC_ESFT_CLASS(ClassName) \
class ClassName: public std::enable_shared_from_this<ClassName> { \
@@ -149,14 +145,13 @@ PUBLIC_ESFT_CLASS(PublicEsftClass);
};
PRIVATE_ESFT_STRUCT(PrivateEsftStruct);
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: PrivateAliasTemplateStructBase is not publicly inheriting from 'std::enable_shared_from_this', will cause unintended behaviour on 'shared_from_this'. fix this by making it public inheritance [bugprone-incorrect-enable-shared-from-this]
-// CHECK-FIXES: struct PrivateAliasTemplateStructBase : public EsftClass<PrivateAliasTemplateStructBase> {};
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'PrivateEsftStruct' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
#define DEFAULT_ESFT_STRUCT(StructName) \
struct StructName: std::enable_shared_from_this<StructName> { \
};
-DEFAULT_ESFT_STRUCT(PrivateEsftStruct);
+DEFAULT_ESFT_STRUCT(DefaultEsftStruct);
#define PUBLIC_ESFT_STRUCT(StructName) \
struct StructName: std::enable_shared_from_this<StructName> { \
>From fd044a20233698899558233bb0f44e514b560df2 Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Mon, 23 Sep 2024 18:19:10 +0800
Subject: [PATCH 20/29] change auto vars to explicit typing
---
.../bugprone/IncorrectEnableSharedFromThisCheck.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index f1bfd43be3d51e..f05c7b533038cc 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -40,10 +40,10 @@ void IncorrectEnableSharedFromThisCheck::check(
if (isStdEnableSharedFromThis(RDecl))
EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
- for (const auto &Base : RDecl->bases()) {
- const auto *BaseRecord =
+ for (const CXXBaseSpecifier &Base : RDecl->bases()) {
+ const CXXRecordDecl *BaseRecord =
Base.getType()->getAsCXXRecordDecl()->getCanonicalDecl();
- const auto IsStdEnableSharedFromThisBool =
+ const bool IsStdEnableSharedFromThisBool =
isStdEnableSharedFromThis(BaseRecord);
if (EnableSharedClassSet.contains(BaseRecord) ||
>From 10c193362ae37c9bc4f71402aa62ece9c895c95d Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Wed, 9 Oct 2024 12:21:49 +0800
Subject: [PATCH 21/29] update docs to fix wording
---
.../bugprone/IncorrectEnableSharedFromThisCheck.h | 6 +++---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../checks/bugprone/incorrect-enable-shared-from-this.rst | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
index 386383a3f16a41..7d9643466c2774 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
@@ -13,9 +13,9 @@
namespace clang::tidy::bugprone {
-// Checks if a class or struct publicly inherits from
-// ``std::enable_shared_from_this``, because otherwise when ``shared_from_this``
-// is called unintended behaviour will occur
+/// Detects if a class or struct publicly inherits from
+/// ``std::enable_shared_from_this``, because otherwise when ``shared_from_this``
+/// is called unintended behaviour will occur
///
/// 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/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index dbaaa336a13afd..3f8a80cd2b6fd8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -134,7 +134,7 @@ New checks
- New :doc:`bugprone-incorrect-enable-shared-from-this
<clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
- Check if class/structs publicly inherits from ``std::enable_shared_from_this``,
+ Detects if class/structs publicly inherits from ``std::enable_shared_from_this``,
because otherwise when ``shared_from_this`` is called unintended behaviour will
occur.
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 26425f5e48fb37..cfc34fee4f7f94 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,7 +3,7 @@
bugprone-incorrect-enable-shared-from-this
==========================================
-Checks if a class or struct publicly inherits from
+Detects if a class or struct publicly inherits from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.
@@ -29,6 +29,6 @@ Consider the following code:
b_ex->bar();
}
-Using ``libstd`` implementation, ``shared_from_this`` will throw
+Using `libstdc++` implementation, ``shared_from_this`` will throw
``std::bad_weak_ptr``. When ``using_not_public()`` is called, this code will
crash without exception handling.
>From ff2d2ef586fad11ba2de2d3046d31d0a6bd616d5 Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Mon, 14 Oct 2024 12:27:09 +0800
Subject: [PATCH 22/29] change to matcher-based check and cleanup headers
---
.../IncorrectEnableSharedFromThisCheck.cpp | 104 +++++++-----------
1 file changed, 37 insertions(+), 67 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index f05c7b533038cc..aebef9dd075d5e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -9,84 +9,54 @@
#include "IncorrectEnableSharedFromThisCheck.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"
-#include "llvm/ADT/SmallPtrSet.h"
using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(translationUnitDecl(), this);
+ const auto EnableSharedFromThis =
+ cxxRecordDecl(hasName("enable_shared_from_this"), isInStdNamespace());
+ const auto QType = hasCanonicalType(hasDeclaration(
+ cxxRecordDecl(
+ anyOf(EnableSharedFromThis.bind("enable_rec"),
+ cxxRecordDecl(hasAnyBase(cxxBaseSpecifier(
+ isPublic(), hasType(hasCanonicalType(
+ hasDeclaration(EnableSharedFromThis))))))))
+ .bind("base_rec")));
+ Finder->addMatcher(
+ cxxRecordDecl(
+ hasDirectBase(cxxBaseSpecifier(unless(isPublic()), hasType(QType))
+ .bind("base")))
+ .bind("derived"),
+ this);
}
void IncorrectEnableSharedFromThisCheck::check(
const MatchFinder::MatchResult &Result) {
-
- class Visitor : public RecursiveASTVisitor<Visitor> {
- IncorrectEnableSharedFromThisCheck &Check;
- llvm::SmallPtrSet<const CXXRecordDecl *, 16> EnableSharedClassSet;
-
- public:
- explicit Visitor(IncorrectEnableSharedFromThisCheck &Check)
- : Check(Check) {}
-
- bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {
- if (!RDecl->hasDefinition())
- return true;
-
- if (isStdEnableSharedFromThis(RDecl))
- EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
-
- for (const CXXBaseSpecifier &Base : RDecl->bases()) {
- const CXXRecordDecl *BaseRecord =
- Base.getType()->getAsCXXRecordDecl()->getCanonicalDecl();
- const bool IsStdEnableSharedFromThisBool =
- isStdEnableSharedFromThis(BaseRecord);
-
- if (EnableSharedClassSet.contains(BaseRecord) ||
- IsStdEnableSharedFromThisBool) {
-
- if (Base.getAccessSpecifier() != clang::AS_public) {
- const SourceRange ReplacementRange = Base.getSourceRange();
- const std::string ReplacementString =
- // Base.getType().getAsString() results in
- // std::enable_shared_from_this<ClassName> or
- // alias/typedefs of std::enable_shared_from_this<ClassName>
- "public " + Base.getType().getAsString();
- const FixItHint Hint = FixItHint::CreateReplacement(
- ReplacementRange, ReplacementString);
- Check.diag(RDecl->getLocation(),
- "%2 is not publicly inheriting from "
- "%select{%1 which inherits from "
- "'std::enable_shared_from_this',|'std::enable_shared_"
- "from_this',}0 "
- "which will cause unintended behaviour "
- "when using 'shared_from_this'; make the inheritance "
- "public",
- DiagnosticIDs::Warning)
- << IsStdEnableSharedFromThisBool << BaseRecord << RDecl << Hint;
- }
-
- EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
- }
- }
- return true;
- }
-
- private:
- // FIXME: configure this for boost in the future
- bool isStdEnableSharedFromThis(const CXXRecordDecl *RDecl) {
- // this is the equivalent of
- // RDecl->getQualifiedNameAsString() == "std::enable_shared_from_this"
- return RDecl->getName() == "enable_shared_from_this" &&
- RDecl->isInStdNamespace();
- }
- };
-
- Visitor(*this).TraverseAST(*Result.Context);
+ const auto *BaseSpec = Result.Nodes.getNodeAs<CXXBaseSpecifier>("base");
+ const auto *Base = Result.Nodes.getNodeAs<CXXRecordDecl>("base_rec");
+ const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
+ const bool IsEnableSharedFromThisDirectBase =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("enable_rec") == Base;
+ const SourceRange ReplacementRange = BaseSpec->getSourceRange();
+ const std::string ReplacementString =
+ // BaseSpec->getType().getAsString() results in
+ // std::enable_shared_from_this<ClassName> or
+ // alias/typedefs of std::enable_shared_from_this<ClassName>
+ "public " + BaseSpec->getType().getAsString();
+ const FixItHint Hint =
+ FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
+ diag(Derived->getLocation(),
+ "%2 is not publicly inheriting from "
+ "%select{%1 which inherits from |}0'std::enable_shared_"
+ "from_this', "
+ "which will cause unintended behaviour "
+ "when using 'shared_from_this'; make the inheritance "
+ "public",
+ DiagnosticIDs::Warning)
+ << IsEnableSharedFromThisDirectBase << Base << Derived << Hint;
}
} // namespace clang::tidy::bugprone
>From 415fb12b688bbaaca8e67c4584049d2c2fd6660a Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Mon, 14 Oct 2024 12:29:26 +0800
Subject: [PATCH 23/29] synchronize check documentation wording
---
.../bugprone/IncorrectEnableSharedFromThisCheck.h | 4 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 6 +++---
.../checks/bugprone/incorrect-enable-shared-from-this.rst | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
index 7d9643466c2774..13b068b02d53ad 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
@@ -14,8 +14,8 @@
namespace clang::tidy::bugprone {
/// Detects if a class or struct publicly inherits from
-/// ``std::enable_shared_from_this``, because otherwise when ``shared_from_this``
-/// is called unintended behaviour will occur
+/// ``std::enable_shared_from_this``, because unintended behavior will
+/// otherwise occur when calling ``shared_from_this``.
///
/// 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/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3f8a80cd2b6fd8..e9f197bfb0e262 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -134,9 +134,9 @@ New checks
- New :doc:`bugprone-incorrect-enable-shared-from-this
<clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
- Detects if class/structs publicly inherits from ``std::enable_shared_from_this``,
- because otherwise when ``shared_from_this`` is called unintended behaviour will
- occur.
+ Detects if a class or struct publicly inherits from
+ ``std::enable_shared_from_this``, because unintended behavior will
+ otherwise occur when calling ``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 cfc34fee4f7f94..d0cb3fce9450ad 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
@@ -17,7 +17,7 @@ Consider the following code:
class BadExample : std::enable_shared_from_this<BadExample> {
// ``shared_from_this``` unintended behaviour
- // libstd implementation returns uninitialized ``weak_ptr``
+ // `libstdc++` implementation returns uninitialized ``weak_ptr``
public:
BadExample* foo() { return shared_from_this().get(); }
void bar() { return; }
>From 9e7689b592948251d7071f2902906481050918cc Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Fri, 25 Oct 2024 22:49:30 +0800
Subject: [PATCH 24/29] sort ReleaseNotes.rst new checks by alphabetical
---
clang-tools-extra/docs/ReleaseNotes.rst | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e9f197bfb0e262..e43f60caeaf71a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,13 @@ New checks
Warns about code that tries to cast between pointers by means of
``std::bit_cast`` or ``memcpy``.
+- New :doc:`bugprone-incorrect-enable-shared-from-this
+ <clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
+
+ Detects if a class or struct publicly inherits from
+ ``std::enable_shared_from_this``, because unintended behavior will
+ otherwise occur when calling ``shared_from_this``.
+
- New :doc:`bugprone-tagged-union-member-count
<clang-tidy/checks/bugprone/tagged-union-member-count>` check.
@@ -131,13 +138,6 @@ New checks
Finds cases when an uninstantiated virtual member function in a template class
causes cross-compiler incompatibility.
-- New :doc:`bugprone-incorrect-enable-shared-from-this
- <clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
-
- Detects if a class or struct publicly inherits from
- ``std::enable_shared_from_this``, because unintended behavior will
- otherwise occur when calling ``shared_from_this``.
-
New check aliases
^^^^^^^^^^^^^^^^^
>From 3978355fdc6027cb9a20c386925d9d33c214f754 Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Sat, 9 Nov 2024 17:40:02 +0800
Subject: [PATCH 25/29] Change fix to emit only on direct inheritance
---
.../bugprone/IncorrectEnableSharedFromThisCheck.cpp | 4 +++-
.../checkers/bugprone/incorrect-enable-shared-from-this.cpp | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index aebef9dd075d5e..127ba20f228127 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -47,7 +47,9 @@ void IncorrectEnableSharedFromThisCheck::check(
// alias/typedefs of std::enable_shared_from_this<ClassName>
"public " + BaseSpec->getType().getAsString();
const FixItHint Hint =
- FixItHint::CreateReplacement(ReplacementRange, ReplacementString);
+ IsEnableSharedFromThisDirectBase
+ ? FixItHint::CreateReplacement(ReplacementRange, ReplacementString)
+ : FixItHint();
diag(Derived->getLocation(),
"%2 is not publicly inheriting from "
"%select{%1 which inherits from |}0'std::enable_shared_"
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 cc1907eb8167cf..9035daa170a52a 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
@@ -42,18 +42,15 @@ class BadMultiClass3Example : dummy_class1, dummy_class2, std::enable_shared_fro
class ClassBase : public std::enable_shared_from_this<ClassBase> {};
class PrivateInheritClassBase : private ClassBase {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateInheritClassBase' is not publicly inheriting from 'ClassBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
-// CHECK-FIXES: class PrivateInheritClassBase : public ClassBase {};
class DefaultInheritClassBase : ClassBase {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultInheritClassBase' is not publicly inheriting from 'ClassBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [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: 'PrivateInheritStructBase' is not publicly inheriting from 'StructBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
-// CHECK-FIXES: struct PrivateInheritStructBase : public StructBase {};
struct DefaultInheritStructBase : StructBase {};
>From ebffebbdd8c23a44a8744db4e559ebbb29e79d8d Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Sat, 9 Nov 2024 17:43:42 +0800
Subject: [PATCH 26/29] reworded first sentence of docs
---
.../clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h | 2 +-
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../checks/bugprone/incorrect-enable-shared-from-this.rst | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
index 13b068b02d53ad..987c56059259b3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h
@@ -13,7 +13,7 @@
namespace clang::tidy::bugprone {
-/// Detects if a class or struct publicly inherits from
+/// Detect classes or structs that do not publicly inherit from
/// ``std::enable_shared_from_this``, because unintended behavior will
/// otherwise occur when calling ``shared_from_this``.
///
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e43f60caeaf71a..587778ccc67e89 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,7 +122,7 @@ New checks
- New :doc:`bugprone-incorrect-enable-shared-from-this
<clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
- Detects if a class or struct publicly inherits from
+ Detect classes or structs that do not publicly inherit from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.
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 d0cb3fce9450ad..cc9e7be70f6ea8 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,7 +3,7 @@
bugprone-incorrect-enable-shared-from-this
==========================================
-Detects if a class or struct publicly inherits from
+Detect classes or structs that do not publicly inherit from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.
>From b38109c7ba8ff969012f325eceab6695c78cdf80 Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Sat, 9 Nov 2024 17:54:55 +0800
Subject: [PATCH 27/29] additional macro testing
---
.../bugprone/incorrect-enable-shared-from-this.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
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 9035daa170a52a..bd82f4db3782c4 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
@@ -155,3 +155,11 @@ DEFAULT_ESFT_STRUCT(DefaultEsftStruct);
};
PUBLIC_ESFT_STRUCT(PublicEsftStruct);
+
+struct A : std::enable_shared_from_this<A> {};
+#define MACRO_A A
+class B : MACRO_A {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'B' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
+class C : private MACRO_A {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'C' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
+class D : public MACRO_A {};
\ No newline at end of file
>From f915a0c6430bad22e2a0233a37cda031354fc0c5 Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Sat, 9 Nov 2024 18:12:39 +0800
Subject: [PATCH 28/29] refactor add newline on eof of test file
---
.../checkers/bugprone/incorrect-enable-shared-from-this.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 bd82f4db3782c4..0e7e83a50c0c85 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
@@ -162,4 +162,4 @@ class B : MACRO_A {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'B' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
class C : private MACRO_A {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'C' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
-class D : public MACRO_A {};
\ No newline at end of file
+class D : public MACRO_A {};
>From 93fa16c8d11ffd6df2c4dc7cea3fb9166aa08d0a Mon Sep 17 00:00:00 2001
From: "Michelle C. Djunaidi" <michelle_djunaidi at mymail.sutd.edu.sg>
Date: Sat, 9 Nov 2024 21:20:26 +0800
Subject: [PATCH 29/29] refactor to only replace access specifier
---
.../IncorrectEnableSharedFromThisCheck.cpp | 15 ++++++++-------
.../incorrect-enable-shared-from-this.cpp | 15 +++++++++++++++
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
index 127ba20f228127..b4d3742b6d44ca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp
@@ -40,15 +40,16 @@ void IncorrectEnableSharedFromThisCheck::check(
const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
const bool IsEnableSharedFromThisDirectBase =
Result.Nodes.getNodeAs<CXXRecordDecl>("enable_rec") == Base;
- const SourceRange ReplacementRange = BaseSpec->getSourceRange();
- const std::string ReplacementString =
- // BaseSpec->getType().getAsString() results in
- // std::enable_shared_from_this<ClassName> or
- // alias/typedefs of std::enable_shared_from_this<ClassName>
- "public " + BaseSpec->getType().getAsString();
+ const bool HasWrittenAccessSpecifier =
+ BaseSpec->getAccessSpecifierAsWritten() != AS_none;
+ const auto ReplacementRange = CharSourceRange(
+ SourceRange(BaseSpec->getBeginLoc(), BaseSpec->getBeginLoc()),
+ HasWrittenAccessSpecifier);
+ const llvm::StringRef Replacement =
+ HasWrittenAccessSpecifier ? "public" : "public ";
const FixItHint Hint =
IsEnableSharedFromThisDirectBase
- ? FixItHint::CreateReplacement(ReplacementRange, ReplacementString)
+ ? FixItHint::CreateReplacement(ReplacementRange, Replacement)
: FixItHint();
diag(Derived->getLocation(),
"%2 is not publicly inheriting from "
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 0e7e83a50c0c85..888caaf1130282 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
@@ -158,8 +158,23 @@ PUBLIC_ESFT_STRUCT(PublicEsftStruct);
struct A : std::enable_shared_from_this<A> {};
#define MACRO_A A
+
class B : MACRO_A {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'B' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
+
class C : private MACRO_A {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'C' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
+
class D : public MACRO_A {};
+
+#define MACRO_PARAM(CLASS) std::enable_shared_from_this<CLASS>
+
+class E : MACRO_PARAM(E) {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'E' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class E : public MACRO_PARAM(E) {};
+
+class F : private MACRO_PARAM(F) {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
+// CHECK-FIXES: class F : public MACRO_PARAM(F) {};
+
+class G : public MACRO_PARAM(G) {};
More information about the cfe-commits
mailing list