[clang-tools-extra] [clang-tidy] Create bugprone-incorrect-enable-shared-from-this check (PR #102299)

[Julian Schmidt via cfe-commits]

================
@@ -0,0 +1,62 @@
+//===--- 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/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *Finder) {
+  const auto EnableSharedFromThis =
+      cxxRecordDecl(hasName("enable_shared_from_this"), isInStdNamespace());
+  const auto QType = hasCanonicalType(hasDeclaration(
+      cxxRecordDecl(
+          anyOf(EnableSharedFromThis.bind("enable_rec"),
+                cxxRecordDecl(hasAnyBase(cxxBaseSpecifier(
+                    isPublic(), hasType(hasCanonicalType(
+                                    hasDeclaration(EnableSharedFromThis))))))))
+          .bind("base_rec")));
+  Finder->addMatcher(
+      cxxRecordDecl(
+          hasDirectBase(cxxBaseSpecifier(unless(isPublic()), hasType(QType))
+                            .bind("base")))
+          .bind("derived"),
+      this);
+}
+
+void IncorrectEnableSharedFromThisCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *BaseSpec = Result.Nodes.getNodeAs<CXXBaseSpecifier>("base");
+  const auto *Base = Result.Nodes.getNodeAs<CXXRecordDecl>("base_rec");
+  const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
+  const bool IsEnableSharedFromThisDirectBase =
+      Result.Nodes.getNodeAs<CXXRecordDecl>("enable_rec") == Base;
+  const SourceRange ReplacementRange = BaseSpec->getSourceRange();
+  const std::string ReplacementString =
+      // BaseSpec->getType().getAsString() results in
+      // std::enable_shared_from_this<ClassName> or
+      // alias/typedefs of std::enable_shared_from_this<ClassName>
+      "public " + BaseSpec->getType().getAsString();
----------------
5chmidti wrote:

> also, side note: MACRO_A doesn't count as direct inheritance either, so the fix won't emit on MACRO_A if we do the "issue FixItHint on direct inheritance only", which sort of resolves this comment indirectly as the issue is with FixItHint.

IMO macros are similar to aliases in this context, and we can do a replacement as well.

---

I found a way that works:

```diff
-  const SourceRange ReplacementRange = BaseSpec->getSourceRange();
-  const std::string ReplacementString =
-      // BaseSpec->getType().getAsString() results in
-      // std::enable_shared_from_this<ClassName> or
-      // alias/typedefs of std::enable_shared_from_this<ClassName>
-      "public " + BaseSpec->getType().getAsString();
+  const bool HasWrittenAccessSpecifier =
+      BaseSpec->getAccessSpecifierAsWritten() != AS_none;
+  const auto ReplacementRange = CharSourceRange(
+      SourceRange(BaseSpec->getBeginLoc(), BaseSpec->getBeginLoc()),
+      HasWrittenAccessSpecifier);
+  const llvm::StringRef Replacement =
+      HasWrittenAccessSpecifier ? "public" : "public ";
   const FixItHint Hint =
       IsEnableSharedFromThisDirectBase
-          ? FixItHint::CreateReplacement(ReplacementRange, ReplacementString)
+          ? FixItHint::CreateReplacement(ReplacementRange, Replacement)
```

with the following tests:
```c++
#define MACRO_PARAM(CLASS) std::enable_shared_from_this<CLASS>
class E : MACRO_PARAM(E) {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'E' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class E : public MACRO_PARAM(E) {};
class F : private MACRO_PARAM(F) {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class F : public MACRO_PARAM(F) {};
class G : public MACRO_PARAM(G) {};
```

https://github.com/llvm/llvm-project/pull/102299