[clang-tools-extra] [clang-tidy] Create bugprone-public-enable-shared-from-this check (PR #102299)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 7 03:22:33 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: None (MichelleCDjunaidi)
<details>
<summary>Changes</summary>
This checks that classes/structs inheriting from ``std::enable_shared_from_this`` derive it with the ``public`` access specifier, so it prevents crashes due to ``std::make_shared`` and ``shared_from_this()`` getting called on a non-public class/struct.
---
Full diff: https://github.com/llvm/llvm-project/pull/102299.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
- (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
- (added) clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp (+45)
- (added) clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h (+33)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
- (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst (+26)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp (+45)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 689eb92a3d8d1..f12f0cd1c47da 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 cb0d8ae98bac5..c9bea094241ed 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 0000000000000..dab3e0ac596fe
--- /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 0000000000000..8a4013c3a46b7
--- /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 642ad39cc0c1c..67f6b1275a1ad 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.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst
new file mode 100644
index 0000000000000..616b2b52271e3
--- /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/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index a931ebf025a10..2b918de89f7c1 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 0000000000000..b0954898ddb78
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp
@@ -0,0 +1,45 @@
+// 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
``````````
</details>
https://github.com/llvm/llvm-project/pull/102299
More information about the cfe-commits
mailing list