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