[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check #102299

MichelleCDjunaidi · 2024-08-07T10:21:38Z

This checks that classes/structs inheriting from std::enable_shared_from_this does so with public inheritance, so it prevents crashes due to std::make_shared and shared_from_this() getting called when the internal weak pointer was not initialized (e.g. due to private inheritance).

github-actions · 2024-08-07T10:21:56Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-08-07T10:22:27Z

@llvm/pr-subscribers-clang-tools-extra

Author: None (MichelleCDjunaidi)

Changes

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 --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

llvmbot · 2024-08-07T10:22:27Z

@llvm/pr-subscribers-clang-tidy

Author: None (MichelleCDjunaidi)

Changes

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 --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

PiotrZSL

I'm fine with idea behind this check. Just few fixes are needed.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp

clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp

clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp

clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp

njames93

How are classes that derive other classes that implement enable_shared_from_this

class A : public std::enable_shared_from_this {};
class B : private A{};

clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp

MichelleCDjunaidi · 2024-08-08T09:43:46Z

How are classes that derive other classes that implement enable_shared_from_this
class A : public std::enable_shared_from_this {};
class B : private A{};

Looking at the AST dump that clang-check gives, it seems that B doesn't have any knowledge of A's internals, so the current approach will not work. Correct me if I'm wrong though

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp

clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp

njames93 · 2024-08-08T16:19:34Z

How are classes that derive other classes that implement enable_shared_from_this
class A : public std::enable_shared_from_this {};
class B : private A{};
Looking at the AST dump that clang-check gives, it seems that B doesn't have any knowledge of A's internals, so the current approach will not work. Correct me if I'm wrong though

I think you'd have to recursively check the inheritance of a class and if you have any private that derives from std::enable_shared_from_this

PiotrZSL · 2024-08-08T19:02:42Z

Yu could use isDerivedFrom, should do a trick

njames93 · 2024-08-09T06:57:00Z

Actually an ASTVisitor approach could be better here, this way you could create a set of all the CXXRecordDecls that publicly inherit from or transitively inherit from ::std::enable_shared_from_this during traversal

MichelleCDjunaidi · 2024-08-12T01:26:10Z

Actually an ASTVisitor approach could be better here, this way you could create a set of all the CXXRecordDecls that publicly inherit from or transitively inherit from ::std::enable_shared_from_this during traversal

To confirm, @njames93 do you mean clang::RecursiveASTVisitor? What I'm getting from this conversation is maintaining the name of the matched classes in a vector and checking for every other class if it inherits from ::std::enable_shared_from_this or classes in the vector.

I'm also unsure regarding best practices for using RecursiveASTVisitor. Wouldn't this necessitate changing the matchers and the check to instead allow RecursiveASTVisitor to handle the context instead?

Or would it be an additional matcher to get every CXXRecordDecl, pass it into RecursiveASTVisitor for it to check for parents whose names are registered from the previous two matchers? I'm leaning towards this option considering there's pre-existing implementations in other checks.

For example, looking at this from readability MakeMemberFunctionConstCheck.cpp:

  template <class T> const T *getParent(const Expr *E) {
    DynTypedNodeList Parents = Ctxt.getParents(*E);
    if (Parents.size() != 1)
      return nullptr;

    return Parents.begin()->get<T>();
  }

and

  bool visitUser(const ImplicitCastExpr *Cast) {
    if (Cast->getCastKind() != CK_NoOp)
      return false; // Stop traversal.

...

    const auto *Parent = getParent<Stmt>(Cast);
    if (!Parent)
      return false; // Stop traversal.

    if (isa<ReturnStmt>(Parent))
      return true; // return (const S*)this;

    if (isa<CallExpr>(Parent))
      return true; // use((const S*)this);

    // ((const S*)this)->Member
    if (const auto *Member = dyn_cast<MemberExpr>(Parent))
      return visitUser(Member, /*OnConstObject=*/true);

    return false; // Stop traversal.
  }

that seems like a way to do the latter option?

Otherwise, an example of the approach you're suggesting would be very helpful, or resources outside of the Doxygen; I'm not very familiar with RecursiveASTVisitor (yet), and looking at other checks like bugprone AssignmentInIfConditionCheck.cpp and readability FunctionSizeCheck.cpp yields very different ideas of how to use it.

5chmidti

maintaining the name of the matched classes in a vector and checking for every other class if it inherits from ::std::enable_shared_from_this or classes in the vector.

You wouldn't want to save the name, because then you would do a lot of string comparisons when you could do pointer comparisons (which would require using RDecl->getCanonical() to ensure you get the same record decl, I believe).
Instead of a vector, you could use a set. Then checking the base classes would simply be a call to contains (llvm::SmallPtrSet?).

Wouldn't this necessitate changing the matchers and the check to instead allow RecursiveASTVisitor to handle the context instead?

Kind of. In this case, you would match the translation unit and call TraverseTranslationUnitDecl on it.
Then you implement bool VisitCXXRecordDecl(CXXRecordDecl* RDecl) and check if RDecl is of type std::enable_shared_from_this and adding that to the set if it is. Otherwise, checking if it publicly inherits from any class that is contained in the set, and adding it to the set if it is. If the inheritance is not public, then you can issue a diagnostic. You can save a pointer to the check (ClangTidyCheck* Check) to do that (Check->diag(...)).

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h

clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp

clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst

MichelleCDjunaidi · 2024-08-13T02:34:51Z

maintaining the name of the matched classes in a vector and checking for every other class if it inherits from ::std::enable_shared_from_this or classes in the vector.

You wouldn't want to save the name, because then you would do a lot of string comparisons when you could do pointer comparisons (which would require using RDecl->getCanonical() to ensure you get the same record decl, I believe). Instead of a vector, you could use a set. Then checking the base classes would simply be a call to contains (llvm::SmallPtrSet?).

Wouldn't this necessitate changing the matchers and the check to instead allow RecursiveASTVisitor to handle the context instead?

Kind of. In this case, you would match the translation unit and call TraverseTranslationUnitDecl on it. Then you implement bool VisitCXXRecordDecl(CXXRecordDecl* RDecl) and check if RDecl is of type std::enable_shared_from_this and adding that to the set if it is. Otherwise, checking if it publicly inherits from any class that is contained in the set, and adding it to the set if it is. If the inheritance is not public, then you can issue a diagnostic. You can save a pointer to the check (ClangTidyCheck* Check) to do that (Check->diag(...)).

Thanks for the pointers! It's really helpful to have a starting point to learn the best practices, I'll try to figure this out and report back.

MichelleCDjunaidi · 2024-08-13T10:33:52Z

@5chmidti so something like this then? AssignmentInIfConditionCheck in bugprone nests the RecursiveASTVisitor, so I tried following suit, but this feels unreadable and I'll refactor it once it's working. Right now this first draft somehow misses some classes and so fails the llvm-lit test (will troubleshoot and commit once it's working; if anyone has any idea why this doesn't work the same way as the previous checks, do tell).

//===--- 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.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#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 *Finder) {
  Finder->addMatcher(translationUnitDecl(), this);
}

void IncorrectEnableSharedFromThisCheck::check(
  const MatchFinder::MatchResult &Result) {

  class Visitor : public RecursiveASTVisitor<Visitor> {
    IncorrectEnableSharedFromThisCheck &Check;
    llvm::SmallPtrSet<const CXXRecordDecl*, 8> EnableSharedClassSet;

  public:
    explicit Visitor(IncorrectEnableSharedFromThisCheck &Check) : Check(Check), EnableSharedClassSet(EnableSharedClassSet) {}
  
    bool VisitCXXRecordDecl(CXXRecordDecl *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());
        }
      }      
      for (const auto &Base : RDecl->bases()) {
        VisitCXXBaseSpecifier(Base, RDecl);
      }
      return true;
    }

    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 (AccessSpec == clang::AS_public) {
          const SourceLocation InsertLocation = Base.getBaseTypeLoc();
          const FixItHint Hint = FixItHint::CreateInsertion(InsertLocation, "std::");
          Check.diag(BaseType->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(BaseType->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;
        }
      }

      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(
              BaseType->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

PiotrZSL

for me oryginal version that used registerMatchers instead of visitor were way better, specially that there are matchers that could be used for this, or could be written.
And even for performance reasons it could be implemented in slightly better way by simply very quickly exclude all classes that do not derive from std::enable_shared_from_this.

If boost::enable_shared_from_this got same flaw, then this check should also support it. And this mean it maybe should be configurable.

Edit: I understand why @njames93 mentioned Visitor, but thing is that in most projects not many classes actually used std::enable_shared_from_this and there will be no visible performance gain from "remembering" list of classes, and as we could see this approach makes this check more complicated.

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp

5chmidti

next time I shouldn't rebase and just wait to fix the conflict on merge

PRs that are quite outdated should, and those that have conflicts must be updated before the PR is merged. You can either rebase with upstream changes, or merge with upstream. Which one you chose is up to you, but merging seems to be less likely to cause issues with commits that are not yours from appearing in the PR. E.g., right now there is a conflict on the release notes. You could use the GitHub UI to fix the conflict, or use your local git by fetching from the remote, and either using git rebase origin/main or git merge origin/main

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp

5chmidti

Minus my comment, this looks good to me. Thanks for working on this.

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp

MichelleCDjunaidi · 2024-11-09T13:45:55Z

Note: I'll need someone to merge this in for me, as I don't have write access.

MichelleCDjunaidi · 2024-11-13T03:08:26Z

@PiotrZSL @njames93 can I get your review on the latest state of this pull request?

5chmidti · 2024-11-25T13:37:20Z

Unless someone also wants to review this PR, I'll merge this tomorrow. The unresolved review comments of the other reviewers have, FWICT, been resolved.

EugeneZelenko · 2024-11-25T14:44:17Z

I think will be good idea to rebase branch again before merge and let others to see changes for some time.

…red-from-this

…prone-public-enable-shared-from-this

MichelleCDjunaidi · 2025-01-01T11:14:19Z

@5chmidti @EugeneZelenko Happy New Year! Is there anything left I need to do to get this merged?

PiotrZSL

LGTM

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp

clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst

clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp

MichelleCDjunaidi · 2025-01-08T15:25:50Z

Working on it.

MichelleCDjunaidi · 2025-01-10T08:09:29Z

@PiotrZSL @5chmidti I'll need help to merge this into the repo, as I don't have write permissions. Thank you so much and have a nice day!

github-actions · 2025-01-12T10:04:59Z

@MichelleCDjunaidi Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvm-ci · 2025-01-12T14:37:28Z

LLVM Buildbot has detected a new failure on builder clang-arm64-windows-msvc running on linaro-armv8-windows-msvc-04 while building clang-tools-extra at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/161/builds/4099

Here is the relevant piece of the build log for the reference

Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'LLVM :: CodeGen/AMDGPU/llvm.amdgcn.struct.tbuffer.store.d16.ll' FAILED ********************
Exit Code: 3221225539

Command Output (stdout):
--
# RUN: at line 2
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=tonga -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=PREGFX10-UNPACKED C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=tonga -verify-machineinstrs
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe' -check-prefixes=PREGFX10-UNPACKED 'C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll'
# RUN: at line 3
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=gfx810 -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=PREGFX10-PACKED C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=gfx810 -verify-machineinstrs
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe' -check-prefixes=PREGFX10-PACKED 'C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll'
# RUN: at line 4
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=PREGFX10-PACKED C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe' -check-prefixes=PREGFX10-PACKED 'C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll'
# RUN: at line 5
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1010 -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=GFX10-PACKED C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1010 -verify-machineinstrs
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe' -check-prefixes=GFX10-PACKED 'C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll'
# RUN: at line 6
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1100 -amdgpu-enable-vopd=0 -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=GFX11-PACKED C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1100 -amdgpu-enable-vopd=0 -verify-machineinstrs
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe' -check-prefixes=GFX11-PACKED 'C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll'
# RUN: at line 7
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1200 -amdgpu-enable-vopd=0 -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=GFX12-PACKED,GFX12-PACKED-SDAG C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1200 -amdgpu-enable-vopd=0 -verify-machineinstrs
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe' -check-prefixes=GFX12-PACKED,GFX12-PACKED-SDAG 'C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll'
# RUN: at line 8
c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1200 -amdgpu-enable-vopd=0 -verify-machineinstrs < C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll | c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\filecheck.exe -check-prefixes=GFX12-PACKED,GFX12-PACKED-SDAG C:\Users\tcwg\llvm-worker\clang-arm64-windows-msvc\llvm\llvm\test\CodeGen\AMDGPU\llvm.amdgcn.struct.tbuffer.store.d16.ll
# executed command: 'c:\users\tcwg\llvm-worker\clang-arm64-windows-msvc\stage1\bin\llc.exe' -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1200 -amdgpu-enable-vopd=0 -verify-machineinstrs
# .---command stdout------------
# | 	.amdgcn_target "amdgcn-amd-amdhsa--gfx1200"
# | 	.amdhsa_code_object_version 5
# | 	.text
# | 	.globl	tbuffer_store_d16_x             ; -- Begin function tbuffer_store_d16_x
# | 	.p2align	8
# | 	.type	tbuffer_store_d16_x,@function
# | tbuffer_store_d16_x:                    ; @tbuffer_store_d16_x
# | ; %bb.0:                                ; %main_body
# | 	s_clause 0x1
# | 	s_load_b64 s[6:7], s[4:5], 0x10
# | 	s_load_b128 s[0:3], s[4:5], 0x0
# | 	s_wait_kmcnt 0x0
# | 	v_mov_b32_e32 v0, s6
# | 	v_mov_b32_e32 v1, s7
# | 	tbuffer_store_d16_format_x v0, v1, s[0:3], null format:[BUF_FMT_10_10_10_2_SNORM] idxen
# | 	s_endpgm
# | 	.section	.rodata,"a",@progbits
...

llvmbot added clang-tools-extra clang-tidy labels Aug 7, 2024

PiotrZSL requested changes Aug 7, 2024

View reviewed changes

EugeneZelenko requested review from njames93, SimplyDanny, 5chmidti and HerrCai0907 August 7, 2024 14:22

EugeneZelenko reviewed Aug 7, 2024

View reviewed changes

njames93 reviewed Aug 7, 2024

View reviewed changes

clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp Outdated Show resolved Hide resolved

clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp Outdated Show resolved Hide resolved

MichelleCDjunaidi requested review from PiotrZSL, njames93 and EugeneZelenko August 8, 2024 09:41

EugeneZelenko reviewed Aug 8, 2024

View reviewed changes

MichelleCDjunaidi requested a review from EugeneZelenko August 12, 2024 03:27

5chmidti requested changes Aug 12, 2024

View reviewed changes

EugeneZelenko reviewed Aug 12, 2024

View reviewed changes

clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst Outdated Show resolved Hide resolved

MichelleCDjunaidi changed the title ~~[clang-tidy] Create bugprone-public-enable-shared-from-this check~~ [clang-tidy] Create bugprone-incorrect-enable-shared-from-this check Aug 13, 2024

MichelleCDjunaidi requested review from 5chmidti and EugeneZelenko August 14, 2024 10:07

PiotrZSL requested changes Aug 15, 2024

View reviewed changes

5chmidti reviewed Oct 28, 2024

View reviewed changes

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h Outdated Show resolved Hide resolved

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp Outdated Show resolved Hide resolved

MichelleCDjunaidi added 3 commits November 9, 2024 17:40

Change fix to emit only on direct inheritance

3978355

reworded first sentence of docs

ebffebb

additional macro testing

b38109c

MichelleCDjunaidi requested a review from 5chmidti November 9, 2024 09:55

refactor add newline on eof of test file

f915a0c

5chmidti approved these changes Nov 9, 2024

View reviewed changes

clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp Outdated Show resolved Hide resolved

MichelleCDjunaidi added 3 commits November 9, 2024 21:20

refactor to only replace access specifier

93fa16c

refactor to remove duplicated arg

fb1b306

Merge remote-tracking branch 'origin/main' into HEAD

1567de5

MichelleCDjunaidi requested a review from EugeneZelenko November 9, 2024 13:45

MichelleCDjunaidi requested a review from 5chmidti November 9, 2024 14:05

MichelleCDjunaidi and others added 3 commits November 26, 2024 11:42

Merge branch 'main' into clang-tidy-create-bugprone-public-enable-sha…

a10c4e1

…red-from-this

Merge branch 'main' into clang-tidy-create-bugprone-public-enable-sha…

a09955d

…red-from-this

Merge remote-tracking branch 'origin/main' into clang-tidy-create-bug…

33c1cad

…prone-public-enable-shared-from-this

PiotrZSL approved these changes Jan 1, 2025

View reviewed changes

exclude system header and update test flag

d8f9563

PiotrZSL merged commit 8ebc35f into llvm:main Jan 12, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check #102299

[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check #102299

MichelleCDjunaidi commented Aug 7, 2024 •

edited

Loading

github-actions bot commented Aug 7, 2024

llvmbot commented Aug 7, 2024

llvmbot commented Aug 7, 2024

PiotrZSL left a comment

njames93 left a comment

MichelleCDjunaidi commented Aug 8, 2024

njames93 commented Aug 8, 2024

PiotrZSL commented Aug 8, 2024

njames93 commented Aug 9, 2024

MichelleCDjunaidi commented Aug 12, 2024 •

edited

Loading

5chmidti left a comment

MichelleCDjunaidi commented Aug 13, 2024

MichelleCDjunaidi commented Aug 13, 2024 •

edited

Loading

PiotrZSL left a comment •

edited

Loading

5chmidti left a comment

5chmidti left a comment

MichelleCDjunaidi commented Nov 9, 2024

MichelleCDjunaidi commented Nov 13, 2024

5chmidti commented Nov 25, 2024

EugeneZelenko commented Nov 25, 2024

MichelleCDjunaidi commented Jan 1, 2025

PiotrZSL left a comment

MichelleCDjunaidi commented Jan 8, 2025

MichelleCDjunaidi commented Jan 10, 2025

github-actions bot commented Jan 12, 2025

llvm-ci commented Jan 12, 2025

[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check #102299

[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check #102299

Conversation

MichelleCDjunaidi commented Aug 7, 2024 • edited Loading

github-actions bot commented Aug 7, 2024

llvmbot commented Aug 7, 2024

llvmbot commented Aug 7, 2024

PiotrZSL left a comment

Choose a reason for hiding this comment

njames93 left a comment

Choose a reason for hiding this comment

MichelleCDjunaidi commented Aug 8, 2024

njames93 commented Aug 8, 2024

PiotrZSL commented Aug 8, 2024

njames93 commented Aug 9, 2024

MichelleCDjunaidi commented Aug 12, 2024 • edited Loading

5chmidti left a comment

Choose a reason for hiding this comment

MichelleCDjunaidi commented Aug 13, 2024

MichelleCDjunaidi commented Aug 13, 2024 • edited Loading

PiotrZSL left a comment • edited Loading

Choose a reason for hiding this comment

5chmidti left a comment

Choose a reason for hiding this comment

5chmidti left a comment

Choose a reason for hiding this comment

MichelleCDjunaidi commented Nov 9, 2024

MichelleCDjunaidi commented Nov 13, 2024

5chmidti commented Nov 25, 2024

EugeneZelenko commented Nov 25, 2024

MichelleCDjunaidi commented Jan 1, 2025

PiotrZSL left a comment

Choose a reason for hiding this comment

MichelleCDjunaidi commented Jan 8, 2025

MichelleCDjunaidi commented Jan 10, 2025

github-actions bot commented Jan 12, 2025

llvm-ci commented Jan 12, 2025

MichelleCDjunaidi commented Aug 7, 2024 •

edited

Loading

MichelleCDjunaidi commented Aug 12, 2024 •

edited

Loading

MichelleCDjunaidi commented Aug 13, 2024 •

edited

Loading

PiotrZSL left a comment •

edited

Loading