[clang-tools-extra] [clang-tidy] CRTP Constructor Accessibility Check (PR #82403)

via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 4 10:06:06 PST 2024


https://github.com/isuckatcs updated https://github.com/llvm/llvm-project/pull/82403

>From f309e3d77fda4a8ba56ea6040175686ff4630e0f Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 16 Feb 2024 00:25:29 +0100
Subject: [PATCH 01/31] added new checker

---
 .../bugprone/BugproneTidyModule.cpp           |  3 +
 .../clang-tidy/bugprone/CMakeLists.txt        |  1 +
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 92 +++++++++++++++++++
 .../clang-tidy/bugprone/UnsafeCrtpCheck.h     | 30 ++++++
 clang-tools-extra/docs/ReleaseNotes.rst       |  5 +
 .../checks/bugprone/unsafe-crtp.rst           |  6 ++
 .../docs/clang-tidy/checks/list.rst           |  1 +
 .../checkers/bugprone/unsafe-crtp.cpp         | 14 +++
 8 files changed, 152 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a8a23b045f80bb..b7b4d85a86cce2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -83,6 +83,7 @@
 #include "UnhandledExceptionAtNewCheck.h"
 #include "UnhandledSelfAssignmentCheck.h"
 #include "UniquePtrArrayMismatchCheck.h"
+#include "UnsafeCrtpCheck.h"
 #include "UnsafeFunctionsCheck.h"
 #include "UnusedLocalNonTrivialVariableCheck.h"
 #include "UnusedRaiiCheck.h"
@@ -237,6 +238,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-unhandled-exception-at-new");
     CheckFactories.registerCheck<UniquePtrArrayMismatchCheck>(
         "bugprone-unique-ptr-array-mismatch");
+    CheckFactories.registerCheck<UnsafeCrtpCheck>(
+        "bugprone-unsafe-crtp");
     CheckFactories.registerCheck<UnsafeFunctionsCheck>(
         "bugprone-unsafe-functions");
     CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 1cd6fb207d7625..956b12ad1cdecd 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -79,6 +79,7 @@ add_clang_library(clangTidyBugproneModule
   UnhandledExceptionAtNewCheck.cpp
   UnhandledSelfAssignmentCheck.cpp
   UniquePtrArrayMismatchCheck.cpp
+  UnsafeCrtpCheck.cpp
   UnsafeFunctionsCheck.cpp
   UnusedLocalNonTrivialVariableCheck.cpp
   UnusedRaiiCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
new file mode 100644
index 00000000000000..eb0f6187e58b2e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -0,0 +1,92 @@
+//===--- UnsafeCrtpCheck.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 "UnsafeCrtpCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+// Finds a node if it's already a bound node.
+AST_MATCHER_P(CXXRecordDecl, isBoundNode, std::string, ID) {
+  return Builder->removeBindings(
+      [&](const ast_matchers::internal::BoundNodesMap &Nodes) {
+        const auto *BoundRecord = Nodes.getNodeAs<CXXRecordDecl>(ID);
+        return BoundRecord != &Node;
+      });
+}
+} // namespace
+
+void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxRecordDecl(
+          decl().bind("record"),
+          hasAnyBase(hasType(
+              classTemplateSpecializationDecl(
+                  hasAnyTemplateArgument(refersToType(recordType(
+                      hasDeclaration(cxxRecordDecl(isBoundNode("record")))))))
+                  .bind("crtp")))),
+      this);
+}
+
+void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCRTP = Result.Nodes.getNodeAs<CXXRecordDecl>("crtp");
+
+  MatchedCRTP->dump();
+
+  for (auto &&ctor : MatchedCRTP->ctors()) {
+    if (ctor->getAccess() != AS_private) {
+      ctor->dump();
+    };
+  }
+
+  return;
+
+  // if (!MatchedDecl->hasDefinition())
+  //   return;
+
+  // for (auto &&Base : MatchedDecl->bases()) {
+  //   const auto *TemplateSpecDecl =
+  //       llvm::dyn_cast_if_present<ClassTemplateSpecializationDecl>(
+  //           Base.getType()->getAsTagDecl());
+
+  //   if (!TemplateSpecDecl)
+  //     continue;
+
+  //   TemplateSpecDecl->dump();
+
+  //   for (auto &&TemplateArg : TemplateSpecDecl->getTemplateArgs().asArray())
+  //   {
+  //     if (TemplateArg.getKind() != TemplateArgument::Type)
+  //       continue;
+
+  //     const auto *Record = TemplateArg.getAsType()->getAsCXXRecordDecl();
+
+  //     if (Record && Record == MatchedDecl) {
+  //       diag(Record->getLocation(), "CRTP found");
+
+  //       diag(TemplateSpecDecl->getLocation(), "used as CRTP",
+  //            DiagnosticIDs::Note);
+  //       TemplateSpecDecl->dump();
+  //     }
+  //   }
+  // }
+
+  // if (!MatchedDecl->getIdentifier() ||
+  //     MatchedDecl->getName().startswith("awesome_"))
+  //   return;
+  // diag(MatchedDecl->getLocation(), "function %0 is insufficiently awesome")
+  //     << MatchedDecl
+  //     << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
+  // diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
new file mode 100644
index 00000000000000..7c41bbb0678521
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
@@ -0,0 +1,30 @@
+//===--- UnsafeCrtpCheck.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_UNSAFECRTPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFECRTPCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-crtp.html
+class UnsafeCrtpCheck : public ClangTidyCheck {
+public:
+  UnsafeCrtpCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFECRTPCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 0d2467210fc664..3ecc35ff572ec6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -116,6 +116,11 @@ New checks
   Replaces certain conditional statements with equivalent calls to
   ``std::min`` or ``std::max``.
 
+- New :doc:`bugprone-unsafe-crtp
+  <clang-tidy/checks/bugprone/unsafe-crtp>` check.
+
+  FIXME: add release notes.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
new file mode 100644
index 00000000000000..3352af02a8744f
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - bugprone-unsafe-crtp
+
+bugprone-unsafe-crtp
+====================
+
+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 5e57bc0ee483fe..67e9413749b444 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -149,6 +149,7 @@ Clang-Tidy Checks
    :doc:`bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new>`,
    :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`,
    :doc:`bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch>`, "Yes"
+   :doc:`bugprone-unsafe-crtp <bugprone/unsafe-crtp>`, "Yes"
    :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`,
    :doc:`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>`,
    :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
new file mode 100644
index 00000000000000..90350ad7abc578
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s bugprone-unsafe-crtp %t
+
+// FIXME: Add something that triggers the check here.
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [bugprone-unsafe-crtp]
+
+// FIXME: Verify the applied fix.
+//   * Make the CHECK patterns specific enough and try to make verified lines
+//     unique to avoid incorrect matches.
+//   * Use {{}} for regular expressions.
+// CHECK-FIXES: {{^}}void awesome_f();{{$}}
+
+// FIXME: Add something that doesn't trigger the check here.
+void awesome_f2();

>From 7b519972e1401645006733e38cb84c45f84b8c12 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 16 Feb 2024 01:18:56 +0100
Subject: [PATCH 02/31] use a different matcher

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 53 ++++---------------
 1 file changed, 9 insertions(+), 44 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index eb0f6187e58b2e..33ad37d7fbbfdb 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -26,15 +26,12 @@ AST_MATCHER_P(CXXRecordDecl, isBoundNode, std::string, ID) {
 } // namespace
 
 void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-      cxxRecordDecl(
-          decl().bind("record"),
-          hasAnyBase(hasType(
-              classTemplateSpecializationDecl(
-                  hasAnyTemplateArgument(refersToType(recordType(
-                      hasDeclaration(cxxRecordDecl(isBoundNode("record")))))))
-                  .bind("crtp")))),
-      this);
+  Finder->addMatcher(classTemplateSpecializationDecl(
+                         decl().bind("crtp"),
+                         hasAnyTemplateArgument(refersToType(recordType(
+                             hasDeclaration(cxxRecordDecl(isDerivedFrom(
+                                 cxxRecordDecl(isBoundNode("crtp"))))))))),
+                     this);
 }
 
 void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
@@ -42,44 +39,12 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
 
   MatchedCRTP->dump();
 
-  for (auto &&ctor : MatchedCRTP->ctors()) {
-    if (ctor->getAccess() != AS_private) {
-      ctor->dump();
+  for (auto &&Ctor : MatchedCRTP->ctors()) {
+    if (Ctor->getAccess() != AS_private) {
+      Ctor->dump();
     };
   }
 
-  return;
-
-  // if (!MatchedDecl->hasDefinition())
-  //   return;
-
-  // for (auto &&Base : MatchedDecl->bases()) {
-  //   const auto *TemplateSpecDecl =
-  //       llvm::dyn_cast_if_present<ClassTemplateSpecializationDecl>(
-  //           Base.getType()->getAsTagDecl());
-
-  //   if (!TemplateSpecDecl)
-  //     continue;
-
-  //   TemplateSpecDecl->dump();
-
-  //   for (auto &&TemplateArg : TemplateSpecDecl->getTemplateArgs().asArray())
-  //   {
-  //     if (TemplateArg.getKind() != TemplateArgument::Type)
-  //       continue;
-
-  //     const auto *Record = TemplateArg.getAsType()->getAsCXXRecordDecl();
-
-  //     if (Record && Record == MatchedDecl) {
-  //       diag(Record->getLocation(), "CRTP found");
-
-  //       diag(TemplateSpecDecl->getLocation(), "used as CRTP",
-  //            DiagnosticIDs::Note);
-  //       TemplateSpecDecl->dump();
-  //     }
-  //   }
-  // }
-
   // if (!MatchedDecl->getIdentifier() ||
   //     MatchedDecl->getName().startswith("awesome_"))
   //   return;

>From 1bd75a20ade8ee79fca6c772c05b336f9837fb56 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sat, 17 Feb 2024 17:22:59 +0100
Subject: [PATCH 03/31] implemented implicit constructor and friend declaration
 logic

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 79 ++++++++++++++++---
 1 file changed, 67 insertions(+), 12 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 33ad37d7fbbfdb..9063b385c0d1b4 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -9,6 +9,7 @@
 #include "UnsafeCrtpCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
@@ -23,26 +24,80 @@ AST_MATCHER_P(CXXRecordDecl, isBoundNode, std::string, ID) {
         return BoundRecord != &Node;
       });
 }
+
+bool hasPrivateConstructor(const CXXRecordDecl *RD) {
+  for (auto &&Ctor : RD->ctors()) {
+    if (Ctor->getAccess() == AS_private)
+      return true;
+  }
+
+  return false;
+}
+
+bool isDerivedBefriended(const CXXRecordDecl *CRTP,
+                         const CXXRecordDecl *Derived) {
+  for (auto &&Friend : CRTP->friends()) {
+    if (Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived)
+      return true;
+  }
+
+  return false;
+}
 } // namespace
 
 void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(classTemplateSpecializationDecl(
-                         decl().bind("crtp"),
-                         hasAnyTemplateArgument(refersToType(recordType(
-                             hasDeclaration(cxxRecordDecl(isDerivedFrom(
-                                 cxxRecordDecl(isBoundNode("crtp"))))))))),
-                     this);
+  Finder->addMatcher(
+      classTemplateSpecializationDecl(
+          decl().bind("crtp"),
+          hasAnyTemplateArgument(refersToType(recordType(hasDeclaration(
+              cxxRecordDecl(isDerivedFrom(cxxRecordDecl(isBoundNode("crtp"))))
+                  .bind("derived")))))),
+      this);
 }
 
 void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *MatchedCRTP = Result.Nodes.getNodeAs<CXXRecordDecl>("crtp");
 
-  MatchedCRTP->dump();
+  const auto *MatchedCRTP =
+      Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
+  const auto *MatchedDerived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
+
+  if (!MatchedCRTP->hasUserDeclaredConstructor()) {
+    diag(MatchedCRTP->getLocation(),
+         "the implicit default constructor of the CRTP is publicly accessible")
+        << MatchedCRTP
+        << FixItHint::CreateInsertion(
+               MatchedCRTP->getBraceRange().getBegin().getLocWithOffset(1),
+               "private: " + MatchedCRTP->getNameAsString() + "() = default;");
+
+    diag(MatchedCRTP->getLocation(), "consider making it private",
+         DiagnosticIDs::Note);
+  }
+
+  // FIXME: Extract this.
+  size_t idx = 0;
+  for (auto &&TemplateArg : MatchedCRTP->getTemplateArgs().asArray()) {
+    if (TemplateArg.getKind() == TemplateArgument::Type &&
+        TemplateArg.getAsType()->getAsCXXRecordDecl() == MatchedDerived) {
+      break;
+    }
+    ++idx;
+  }
 
-  for (auto &&Ctor : MatchedCRTP->ctors()) {
-    if (Ctor->getAccess() != AS_private) {
-      Ctor->dump();
-    };
+  if (hasPrivateConstructor(MatchedCRTP) &&
+      !isDerivedBefriended(MatchedCRTP, MatchedDerived)) {
+    diag(MatchedCRTP->getLocation(),
+         "the CRTP cannot be constructed from the derived class")
+        << MatchedCRTP
+        << FixItHint::CreateInsertion(
+               MatchedCRTP->getBraceRange().getEnd().getLocWithOffset(-1),
+               "friend " +
+                   MatchedCRTP->getSpecializedTemplate()
+                       ->getTemplateParameters()
+                       ->asArray()[idx]
+                       ->getNameAsString() +
+                   ';');
+    diag(MatchedCRTP->getLocation(),
+         "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
 
   // if (!MatchedDecl->getIdentifier() ||

>From b930eee4efff6481d8ec07f785ca6791feeff8ee Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 18 Feb 2024 02:18:18 +0100
Subject: [PATCH 04/31] fix friend detection

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 75 +++++++++++--------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 9063b385c0d1b4..05d9218e9c3a71 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -34,15 +34,39 @@ bool hasPrivateConstructor(const CXXRecordDecl *RD) {
   return false;
 }
 
-bool isDerivedBefriended(const CXXRecordDecl *CRTP,
-                         const CXXRecordDecl *Derived) {
+bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
+                                  const NamedDecl *Derived) {
   for (auto &&Friend : CRTP->friends()) {
-    if (Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived)
+    const auto *TTPT =
+        dyn_cast<TemplateTypeParmType>(Friend->getFriendType()->getType());
+
+    if (TTPT && TTPT->getDecl() == Derived)
       return true;
   }
 
   return false;
 }
+
+std::optional<const NamedDecl *>
+getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
+                    const CXXRecordDecl *Derived) {
+  size_t Idx = 0;
+  bool Found = false;
+  for (auto &&TemplateArg : CRTP->getTemplateArgs().asArray()) {
+    if (TemplateArg.getKind() == TemplateArgument::Type &&
+        TemplateArg.getAsType()->getAsCXXRecordDecl() == Derived) {
+      Found = true;
+      break;
+    }
+    ++Idx;
+  }
+
+  if (!Found)
+    return std::nullopt;
+
+  return CRTP->getSpecializedTemplate()->getTemplateParameters()->getParam(Idx);
+}
+
 } // namespace
 
 void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
@@ -61,42 +85,33 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
   const auto *MatchedDerived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
 
-  if (!MatchedCRTP->hasUserDeclaredConstructor()) {
-    diag(MatchedCRTP->getLocation(),
+  const auto *CRTPTemplate =
+      MatchedCRTP->getSpecializedTemplate()->getTemplatedDecl();
+
+  if (!CRTPTemplate->hasUserDeclaredConstructor()) {
+    diag(CRTPTemplate->getLocation(),
          "the implicit default constructor of the CRTP is publicly accessible")
-        << MatchedCRTP
+        << CRTPTemplate
         << FixItHint::CreateInsertion(
-               MatchedCRTP->getBraceRange().getBegin().getLocWithOffset(1),
-               "private: " + MatchedCRTP->getNameAsString() + "() = default;");
+               CRTPTemplate->getBraceRange().getBegin().getLocWithOffset(1),
+               "private: " + CRTPTemplate->getNameAsString() + "() = default;");
 
-    diag(MatchedCRTP->getLocation(), "consider making it private",
+    diag(CRTPTemplate->getLocation(), "consider making it private",
          DiagnosticIDs::Note);
   }
 
-  // FIXME: Extract this.
-  size_t idx = 0;
-  for (auto &&TemplateArg : MatchedCRTP->getTemplateArgs().asArray()) {
-    if (TemplateArg.getKind() == TemplateArgument::Type &&
-        TemplateArg.getAsType()->getAsCXXRecordDecl() == MatchedDerived) {
-      break;
-    }
-    ++idx;
-  }
+  const auto *DerivedTemplateParameter =
+      *getDerivedParameter(MatchedCRTP, MatchedDerived);
 
-  if (hasPrivateConstructor(MatchedCRTP) &&
-      !isDerivedBefriended(MatchedCRTP, MatchedDerived)) {
-    diag(MatchedCRTP->getLocation(),
+  if (hasPrivateConstructor(CRTPTemplate) &&
+      !isDerivedParameterBefriended(CRTPTemplate, DerivedTemplateParameter)) {
+    diag(CRTPTemplate->getLocation(),
          "the CRTP cannot be constructed from the derived class")
-        << MatchedCRTP
+        << CRTPTemplate
         << FixItHint::CreateInsertion(
-               MatchedCRTP->getBraceRange().getEnd().getLocWithOffset(-1),
-               "friend " +
-                   MatchedCRTP->getSpecializedTemplate()
-                       ->getTemplateParameters()
-                       ->asArray()[idx]
-                       ->getNameAsString() +
-                   ';');
-    diag(MatchedCRTP->getLocation(),
+               CRTPTemplate->getBraceRange().getEnd().getLocWithOffset(-1),
+               "friend " + DerivedTemplateParameter->getNameAsString() + ';');
+    diag(CRTPTemplate->getLocation(),
          "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
 

>From 9ce552d2ca79c4e139a5fa82586dbbeec8bc5ab3 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 18 Feb 2024 02:28:22 +0100
Subject: [PATCH 05/31] cleanup

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 48 ++++++++-----------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 05d9218e9c3a71..09d2db4d993984 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -7,9 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "UnsafeCrtpCheck.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
@@ -80,48 +78,40 @@ void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
 }
 
 void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
-
-  const auto *MatchedCRTP =
+  const auto *CRTPInstantiation =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
-  const auto *MatchedDerived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
+  const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
 
-  const auto *CRTPTemplate =
-      MatchedCRTP->getSpecializedTemplate()->getTemplatedDecl();
+  const CXXRecordDecl *CRTPDeclaration =
+      CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
 
-  if (!CRTPTemplate->hasUserDeclaredConstructor()) {
-    diag(CRTPTemplate->getLocation(),
+  if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
+    diag(CRTPDeclaration->getLocation(),
          "the implicit default constructor of the CRTP is publicly accessible")
-        << CRTPTemplate
+        << CRTPDeclaration
         << FixItHint::CreateInsertion(
-               CRTPTemplate->getBraceRange().getBegin().getLocWithOffset(1),
-               "private: " + CRTPTemplate->getNameAsString() + "() = default;");
-
-    diag(CRTPTemplate->getLocation(), "consider making it private",
+               CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(1),
+               "private: " + CRTPDeclaration->getNameAsString() +
+                   "() = default;");
+    diag(CRTPDeclaration->getLocation(), "consider making it private",
          DiagnosticIDs::Note);
   }
 
   const auto *DerivedTemplateParameter =
-      *getDerivedParameter(MatchedCRTP, MatchedDerived);
+      *getDerivedParameter(CRTPInstantiation, Derived);
 
-  if (hasPrivateConstructor(CRTPTemplate) &&
-      !isDerivedParameterBefriended(CRTPTemplate, DerivedTemplateParameter)) {
-    diag(CRTPTemplate->getLocation(),
+  if (hasPrivateConstructor(CRTPDeclaration) &&
+      !isDerivedParameterBefriended(CRTPDeclaration,
+                                    DerivedTemplateParameter)) {
+    diag(CRTPDeclaration->getLocation(),
          "the CRTP cannot be constructed from the derived class")
-        << CRTPTemplate
+        << CRTPDeclaration
         << FixItHint::CreateInsertion(
-               CRTPTemplate->getBraceRange().getEnd().getLocWithOffset(-1),
+               CRTPDeclaration->getBraceRange().getEnd().getLocWithOffset(-1),
                "friend " + DerivedTemplateParameter->getNameAsString() + ';');
-    diag(CRTPTemplate->getLocation(),
+    diag(CRTPDeclaration->getLocation(),
          "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
-
-  // if (!MatchedDecl->getIdentifier() ||
-  //     MatchedDecl->getName().startswith("awesome_"))
-  //   return;
-  // diag(MatchedDecl->getLocation(), "function %0 is insufficiently awesome")
-  //     << MatchedDecl
-  //     << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
-  // diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note);
 }
 
 } // namespace clang::tidy::bugprone

>From 7d3152e5e8222a4b49d9e6c30c32e86ec28318ff Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 18 Feb 2024 04:17:25 +0100
Subject: [PATCH 06/31] implement ctor checks

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 09d2db4d993984..85d8b8a8ab8cd8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "UnsafeCrtpCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
@@ -65,6 +66,24 @@ getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
   return CRTP->getSpecializedTemplate()->getTemplateParameters()->getParam(Idx);
 }
 
+std::vector<FixItHint> hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
+                                           std::string OriginalAccess,
+                                           const SourceManager &SM,
+                                           const LangOptions &LangOpts) {
+  std::vector<FixItHint> Hints;
+
+  Hints.emplace_back(FixItHint::CreateInsertion(
+      Ctor->getBeginLoc().getLocWithOffset(-1), "private:\n"));
+
+  Hints.emplace_back(FixItHint::CreateInsertion(
+      Ctor->isExplicitlyDefaulted()
+          ? utils::lexer::findNextTerminator(Ctor->getEndLoc(), SM, LangOpts)
+                .getLocWithOffset(1)
+          : Ctor->getEndLoc().getLocWithOffset(1),
+      '\n' + OriginalAccess + ':' + '\n'));
+
+  return Hints;
+}
 } // namespace
 
 void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
@@ -112,6 +131,23 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
     diag(CRTPDeclaration->getLocation(),
          "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
+
+  for (auto &&Ctor : CRTPDeclaration->ctors()) {
+    if (Ctor->getAccess() == AS_private)
+      continue;
+
+    bool IsPublic = Ctor->getAccess() == AS_public;
+    std::string Access = IsPublic ? "public" : "protected";
+
+    diag(Ctor->getLocation(),
+         "%0 contructor allows the CRTP to be %select{inherited "
+         "from|constructed}1 as a regular template class")
+        << Access << IsPublic << Ctor
+        << hintMakeCtorPrivate(Ctor, Access, *Result.SourceManager,
+                               getLangOpts());
+    diag(Ctor->getLocation(), "consider making it private",
+         DiagnosticIDs::Note);
+  }
 }
 
 } // namespace clang::tidy::bugprone

>From cbc6672ddb70cb145c85a2e51432e6ee049d3dc7 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 18 Feb 2024 04:23:18 +0100
Subject: [PATCH 07/31] fix struct default ctor generation

---
 clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 85d8b8a8ab8cd8..9295ba0009ac24 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -111,7 +111,8 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
         << FixItHint::CreateInsertion(
                CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(1),
                "private: " + CRTPDeclaration->getNameAsString() +
-                   "() = default;");
+                   "() = default;" +
+                   (CRTPDeclaration->isStruct() ? "\npublic:\n" : ""));
     diag(CRTPDeclaration->getLocation(), "consider making it private",
          DiagnosticIDs::Note);
   }

>From 050efe2eb797342a16155565a8c16cad67a514b3 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sun, 18 Feb 2024 18:09:47 +0100
Subject: [PATCH 08/31] testing

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   |  11 +-
 .../checkers/bugprone/unsafe-crtp.cpp         | 172 ++++++++++++++++--
 2 files changed, 166 insertions(+), 17 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 9295ba0009ac24..b97e0142420be4 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -110,9 +110,9 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
         << CRTPDeclaration
         << FixItHint::CreateInsertion(
                CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(1),
-               "private: " + CRTPDeclaration->getNameAsString() +
-                   "() = default;" +
-                   (CRTPDeclaration->isStruct() ? "\npublic:\n" : ""));
+               (CRTPDeclaration->isStruct() ? "\nprivate:\n" : "\n") +
+                   CRTPDeclaration->getNameAsString() + "() = default;" +
+                   (CRTPDeclaration->isStruct() ? "\npublic:\n" : "\n"));
     diag(CRTPDeclaration->getLocation(), "consider making it private",
          DiagnosticIDs::Note);
   }
@@ -127,8 +127,9 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
          "the CRTP cannot be constructed from the derived class")
         << CRTPDeclaration
         << FixItHint::CreateInsertion(
-               CRTPDeclaration->getBraceRange().getEnd().getLocWithOffset(-1),
-               "friend " + DerivedTemplateParameter->getNameAsString() + ';');
+               CRTPDeclaration->getBraceRange().getEnd(),
+               "friend " + DerivedTemplateParameter->getNameAsString() + ';' +
+                   '\n');
     diag(CRTPDeclaration->getLocation(),
          "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
index 90350ad7abc578..b8e38aa18e5e41 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
@@ -1,14 +1,162 @@
 // RUN: %check_clang_tidy %s bugprone-unsafe-crtp %t
 
-// FIXME: Add something that triggers the check here.
-void f();
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [bugprone-unsafe-crtp]
-
-// FIXME: Verify the applied fix.
-//   * Make the CHECK patterns specific enough and try to make verified lines
-//     unique to avoid incorrect matches.
-//   * Use {{}} for regular expressions.
-// CHECK-FIXES: {{^}}void awesome_f();{{$}}
-
-// FIXME: Add something that doesn't trigger the check here.
-void awesome_f2();
+namespace class_implicit_ctor {
+template <typename T>
+class CRTP {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-FIXES: CRTP() = default;
+
+class A : CRTP<A> {};
+} // namespace class_implicit_ctor
+
+namespace class_uncostructible {
+template <typename T>
+class CRTP {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-FIXES: friend T;
+    CRTP() = default;
+};
+
+class A : CRTP<A> {};
+} // namespace class_uncostructible 
+
+namespace class_public_default_ctor {
+template <typename T>
+class CRTP {
+public:
+    CRTP() = default;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+};
+
+class A : CRTP<A> {};
+} // namespace class_public_default_ctor
+
+namespace class_public_user_provided_ctor {
+template <typename T>
+class CRTP {
+public:
+    CRTP(int) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
+};
+
+class A : CRTP<A> {};
+} // namespace class_public_user_provided_ctor
+
+namespace class_public_multiple_user_provided_ctors {
+template <typename T>
+class CRTP {
+public:
+    CRTP(int) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
+    CRTP(float) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}public:
+};
+
+class A : CRTP<A> {};
+} // namespace class_public_multiple_user_provided_ctors
+
+namespace class_protected_ctors {
+template <typename T>
+class CRTP {
+protected:
+    CRTP(int) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}protected:
+    CRTP() = default;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}protected:
+    CRTP(float) {}
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}protected:
+};
+
+class A : CRTP<A> {};
+} // namespace class_protected_ctors
+
+namespace struct_implicit_ctor {
+template <typename T>
+struct CRTP {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
+// CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+
+class A : CRTP<A> {};
+} // namespace struct_implicit_ctor
+
+namespace struct_default_ctor {
+template <typename T>
+struct CRTP {
+    CRTP() = default;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+};
+
+class A : CRTP<A> {};
+} // namespace struct_default_ctor
+
+namespace same_class_multiple_crtps {
+template <typename T>
+struct CRTP {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
+// CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+
+template <typename T>
+struct CRTP2 {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
+// CHECK-FIXES: private:{{[[:space:]]*}}CRTP2() = default;{{[[:space:]]*}}public:
+
+class A : CRTP<A>, CRTP2<A> {};
+} // namespace same_class_multiple_crtps
+
+namespace same_crtp_multiple_classes {
+template <typename T>
+class CRTP {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-FIXES: friend T;
+    CRTP() = default;
+};
+
+class A : CRTP<A> {};
+class B : CRTP<B> {};
+} // namespace same_crtp_multiple_classes
+
+namespace crtp_template {
+template <typename T, typename U>
+class CRTP {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-FIXES: friend U;
+    CRTP() = default;
+};
+
+class A : CRTP<int, A> {};
+} // namespace crtp_template
+
+namespace crtp_template2 {
+template <typename T, typename U>
+class CRTP {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-FIXES: friend T;
+    CRTP() = default;
+};
+
+class A : CRTP<A, A> {};
+} // namespace crtp_template2

>From c7cb548245f0dba36a93a7987a9b1f9cef68fdb4 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 20 Feb 2024 15:53:16 +0100
Subject: [PATCH 09/31] docs

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.h     |  2 +-
 clang-tools-extra/docs/ReleaseNotes.rst       |  2 +-
 .../checks/bugprone/unsafe-crtp.rst           | 58 ++++++++++++++++++-
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
index 7c41bbb0678521..ac1a8f09208f95 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
@@ -13,7 +13,7 @@
 
 namespace clang::tidy::bugprone {
 
-/// FIXME: Write a short description.
+/// Finds CRTP used in an error-prone way.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-crtp.html
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3ecc35ff572ec6..c9006b1c4aa83a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,7 +119,7 @@ New checks
 - New :doc:`bugprone-unsafe-crtp
   <clang-tidy/checks/bugprone/unsafe-crtp>` check.
 
-  FIXME: add release notes.
+  Detects error-prone CRTP.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
index 3352af02a8744f..8e6955999512a6 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
@@ -3,4 +3,60 @@
 bugprone-unsafe-crtp
 ====================
 
-FIXME: Describe what patterns does the check detect and why. Give examples.
+Finds CRTP used in an error-prone way.
+
+If the constructor of a class intended to be used in a CRTP is public, then
+it allows users to construct that class on its own.
+
+Example:
+
+.. code-block:: c++
+
+  template <typename T> class CRTP {
+  public:
+    CRTP() = default;
+  };
+
+  class Good : CRTP<Good> {};
+  Good GoodInstance;
+
+  CRTP<int> BadInstance;
+
+If the constructor is protected, the possibility of an accidental instantiation
+is prevented, however it can fade an error, when a different class is used as
+the template parameter instead of the derived one.
+
+Example:
+
+.. code-block:: c++
+
+  template <typename T> class CRTP {
+  protected:
+    CRTP() = default;
+  };
+
+  class Good : CRTP<Good> {};
+  Good GoodInstance;
+
+  class Bad : CRTP<Good> {};
+  Bad BadInstance;
+
+To ensure that no accidental instantiation happens, the best practice is to make
+the constructor private and declare the derived class as friend.
+
+Example:
+
+.. code-block:: c++
+
+  template <typename T> class CRTP {
+    CRTP() = default;
+    friend T;
+  };
+
+  class Good : CRTP<Good> {};
+  Good GoodInstance;
+
+  class Bad : CRTP<Good> {};
+  Bad CompileTimeError;
+
+  CRTP<int> AlsoCompileTimeError;

>From 212ae079b28bdc2c1f4d85cca686f4451e99df11 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 20 Feb 2024 18:36:19 +0100
Subject: [PATCH 10/31] cleanup

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 19 ++++++++++---------
 clang-tools-extra/docs/ReleaseNotes.rst       |  2 +-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index b97e0142420be4..a1caf934ef2cd6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -67,7 +67,7 @@ getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
 }
 
 std::vector<FixItHint> hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
-                                           std::string OriginalAccess,
+                                           const std::string &OriginalAccess,
                                            const SourceManager &SM,
                                            const LangOptions &LangOpts) {
   std::vector<FixItHint> Hints;
@@ -75,12 +75,12 @@ std::vector<FixItHint> hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
   Hints.emplace_back(FixItHint::CreateInsertion(
       Ctor->getBeginLoc().getLocWithOffset(-1), "private:\n"));
 
-  Hints.emplace_back(FixItHint::CreateInsertion(
+  SourceLocation CtorEndLoc =
       Ctor->isExplicitlyDefaulted()
           ? utils::lexer::findNextTerminator(Ctor->getEndLoc(), SM, LangOpts)
-                .getLocWithOffset(1)
-          : Ctor->getEndLoc().getLocWithOffset(1),
-      '\n' + OriginalAccess + ':' + '\n'));
+          : Ctor->getEndLoc();
+  Hints.emplace_back(FixItHint::CreateInsertion(
+      CtorEndLoc.getLocWithOffset(1), '\n' + OriginalAccess + ':' + '\n'));
 
   return Hints;
 }
@@ -100,19 +100,20 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CRTPInstantiation =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
   const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
-
   const CXXRecordDecl *CRTPDeclaration =
       CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
 
   if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
+    bool IsStruct = CRTPDeclaration->isStruct();
+
     diag(CRTPDeclaration->getLocation(),
          "the implicit default constructor of the CRTP is publicly accessible")
         << CRTPDeclaration
         << FixItHint::CreateInsertion(
                CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(1),
-               (CRTPDeclaration->isStruct() ? "\nprivate:\n" : "\n") +
-                   CRTPDeclaration->getNameAsString() + "() = default;" +
-                   (CRTPDeclaration->isStruct() ? "\npublic:\n" : "\n"));
+               (IsStruct ? "\nprivate:\n" : "\n") +
+                   CRTPDeclaration->getNameAsString() + "() = default;\n" +
+                   (IsStruct ? "public:\n" : ""));
     diag(CRTPDeclaration->getLocation(), "consider making it private",
          DiagnosticIDs::Note);
   }
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c9006b1c4aa83a..1f24dd52e29201 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,7 +119,7 @@ New checks
 - New :doc:`bugprone-unsafe-crtp
   <clang-tidy/checks/bugprone/unsafe-crtp>` check.
 
-  Detects error-prone CRTP.
+  Detects error-prone CRTP usage.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^

>From 3aca4c3cfe951eda511fdd722f7d6a9bf6adcb85 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 20 Feb 2024 18:50:41 +0100
Subject: [PATCH 11/31] more testing

---
 .../checkers/bugprone/unsafe-crtp.cpp         | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
index b8e38aa18e5e41..13bd19610ed942 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
@@ -160,3 +160,45 @@ class CRTP {
 
 class A : CRTP<A, A> {};
 } // namespace crtp_template2
+
+namespace template_derived {
+template <typename T>
+class CRTP {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-FIXES: CRTP() = default;
+
+template<typename T>
+class A : CRTP<A<T>> {};
+
+// FIXME: Ideally the warning should be triggered without instantiation.
+void foo() {
+  A<int> A;
+  (void) A;
+}
+} // namespace template_derived
+
+namespace template_derived_explicit_specialization {
+template <typename T>
+class CRTP {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-FIXES: CRTP() = default;
+
+template<typename T>
+class A : CRTP<A<T>> {};
+
+template<>
+class A<int> : CRTP<A<int>> {};
+} // namespace template_derived_explicit_specialization
+
+namespace no_warning {
+template <typename T>
+class CRTP
+{
+    CRTP() = default;
+    friend T;
+};
+
+class A : CRTP<A> {};
+} // namespace no_warning

>From 15846ed9e4ff207162bd5dd2b7ca7086f5eeed7a Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 20 Feb 2024 19:46:13 +0100
Subject: [PATCH 12/31] extend

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 21 ++++++++++----
 .../checkers/bugprone/unsafe-crtp.cpp         | 28 +++++++++++++++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index a1caf934ef2cd6..7bb50aaf950cbe 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -34,12 +34,22 @@ bool hasPrivateConstructor(const CXXRecordDecl *RD) {
 }
 
 bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
-                                  const NamedDecl *Derived) {
+                                  const NamedDecl *Param) {
   for (auto &&Friend : CRTP->friends()) {
     const auto *TTPT =
         dyn_cast<TemplateTypeParmType>(Friend->getFriendType()->getType());
 
-    if (TTPT && TTPT->getDecl() == Derived)
+    if (TTPT && TTPT->getDecl() == Param)
+      return true;
+  }
+
+  return false;
+}
+
+bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
+                              const CXXRecordDecl *Derived) {
+  for (auto &&Friend : CRTP->friends()) {
+    if (Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived)
       return true;
   }
 
@@ -99,7 +109,7 @@ void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
 void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CRTPInstantiation =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
-  const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
+  const auto *DerivedRecord = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
   const CXXRecordDecl *CRTPDeclaration =
       CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
 
@@ -119,11 +129,12 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
   }
 
   const auto *DerivedTemplateParameter =
-      *getDerivedParameter(CRTPInstantiation, Derived);
+      *getDerivedParameter(CRTPInstantiation, DerivedRecord);
 
   if (hasPrivateConstructor(CRTPDeclaration) &&
       !isDerivedParameterBefriended(CRTPDeclaration,
-                                    DerivedTemplateParameter)) {
+                                    DerivedTemplateParameter) &&
+      !isDerivedClassBefriended(CRTPDeclaration, DerivedRecord)) {
     diag(CRTPDeclaration->getLocation(),
          "the CRTP cannot be constructed from the derived class")
         << CRTPDeclaration
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
index 13bd19610ed942..edcfd67677fd7b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
@@ -192,6 +192,34 @@ template<>
 class A<int> : CRTP<A<int>> {};
 } // namespace template_derived_explicit_specialization
 
+namespace explicit_derived_friend {
+class A;
+
+template <typename T>
+class CRTP {
+    CRTP() = default;
+    friend A;
+};
+
+class A : CRTP<A> {};
+} // namespace explicit_derived_friend
+
+namespace explicit_derived_friend_multiple {
+class A;
+
+template <typename T>
+class CRTP {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-FIXES: friend T;
+    CRTP() = default;
+    friend A;
+};
+
+class A : CRTP<A> {};
+class B : CRTP<B> {};
+} // namespace explicit_derived_friend_multiple
+
 namespace no_warning {
 template <typename T>
 class CRTP

>From 5795cb2714c7d418c54263058061a74b41a673b3 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 20 Feb 2024 20:09:12 +0100
Subject: [PATCH 13/31] format

---
 clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index b7b4d85a86cce2..78e128fb0e5e6d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -238,8 +238,7 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-unhandled-exception-at-new");
     CheckFactories.registerCheck<UniquePtrArrayMismatchCheck>(
         "bugprone-unique-ptr-array-mismatch");
-    CheckFactories.registerCheck<UnsafeCrtpCheck>(
-        "bugprone-unsafe-crtp");
+    CheckFactories.registerCheck<UnsafeCrtpCheck>("bugprone-unsafe-crtp");
     CheckFactories.registerCheck<UnsafeFunctionsCheck>(
         "bugprone-unsafe-functions");
     CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>(

>From 42ce33fe87a731170ebdf4aa035d4a5f6878d058 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 20 Feb 2024 20:29:39 +0100
Subject: [PATCH 14/31] addressed comments

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp          | 12 ++++++++----
 .../clang-tidy/bugprone/UnsafeCrtpCheck.h            |  3 ++-
 clang-tools-extra/docs/ReleaseNotes.rst              | 10 +++++-----
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 7bb50aaf950cbe..227bd9803a1267 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -85,7 +85,7 @@ std::vector<FixItHint> hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
   Hints.emplace_back(FixItHint::CreateInsertion(
       Ctor->getBeginLoc().getLocWithOffset(-1), "private:\n"));
 
-  SourceLocation CtorEndLoc =
+  const SourceLocation CtorEndLoc =
       Ctor->isExplicitlyDefaulted()
           ? utils::lexer::findNextTerminator(Ctor->getEndLoc(), SM, LangOpts)
           : Ctor->getEndLoc();
@@ -114,7 +114,7 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
       CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
 
   if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
-    bool IsStruct = CRTPDeclaration->isStruct();
+    const bool IsStruct = CRTPDeclaration->isStruct();
 
     diag(CRTPDeclaration->getLocation(),
          "the implicit default constructor of the CRTP is publicly accessible")
@@ -150,8 +150,8 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
     if (Ctor->getAccess() == AS_private)
       continue;
 
-    bool IsPublic = Ctor->getAccess() == AS_public;
-    std::string Access = IsPublic ? "public" : "protected";
+    const bool IsPublic = Ctor->getAccess() == AS_public;
+    const std::string Access = IsPublic ? "public" : "protected";
 
     diag(Ctor->getLocation(),
          "%0 contructor allows the CRTP to be %select{inherited "
@@ -164,4 +164,8 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
   }
 }
 
+bool UnsafeCrtpCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}
 } // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
index ac1a8f09208f95..fb7245edeb0fa9 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
@@ -13,7 +13,7 @@
 
 namespace clang::tidy::bugprone {
 
-/// Finds CRTP used in an error-prone way.
+/// Detects error-prone CRTP usage.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-crtp.html
@@ -23,6 +23,7 @@ class UnsafeCrtpCheck : public ClangTidyCheck {
       : 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;
 };
 
 } // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1f24dd52e29201..d86ac48c6b3aec 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,11 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-unsafe-crtp
+  <clang-tidy/checks/bugprone/unsafe-crtp>` check.
+
+  Detects error-prone CRTP usage.
+
 - New :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check.
 
@@ -116,11 +121,6 @@ New checks
   Replaces certain conditional statements with equivalent calls to
   ``std::min`` or ``std::max``.
 
-- New :doc:`bugprone-unsafe-crtp
-  <clang-tidy/checks/bugprone/unsafe-crtp>` check.
-
-  Detects error-prone CRTP usage.
-
 New check aliases
 ^^^^^^^^^^^^^^^^^
 

>From ddc323b339ab822f72999ebc5c28c729e7473e0e Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 00:24:41 +0100
Subject: [PATCH 15/31] comments

---
 .../clang-tidy/bugprone/UnsafeCrtpCheck.cpp   | 34 +++++++------------
 .../clang-tidy/bugprone/UnsafeCrtpCheck.h     |  3 +-
 clang-tools-extra/docs/ReleaseNotes.rst       |  2 +-
 .../checkers/bugprone/unsafe-crtp.cpp         |  2 +-
 4 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
index 227bd9803a1267..bdbb2ab7da35e5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
@@ -14,17 +14,7 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::bugprone {
 
-namespace {
-// Finds a node if it's already a bound node.
-AST_MATCHER_P(CXXRecordDecl, isBoundNode, std::string, ID) {
-  return Builder->removeBindings(
-      [&](const ast_matchers::internal::BoundNodesMap &Nodes) {
-        const auto *BoundRecord = Nodes.getNodeAs<CXXRecordDecl>(ID);
-        return BoundRecord != &Node;
-      });
-}
-
-bool hasPrivateConstructor(const CXXRecordDecl *RD) {
+static bool hasPrivateConstructor(const CXXRecordDecl *RD) {
   for (auto &&Ctor : RD->ctors()) {
     if (Ctor->getAccess() == AS_private)
       return true;
@@ -33,8 +23,8 @@ bool hasPrivateConstructor(const CXXRecordDecl *RD) {
   return false;
 }
 
-bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
-                                  const NamedDecl *Param) {
+static bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
+                                         const NamedDecl *Param) {
   for (auto &&Friend : CRTP->friends()) {
     const auto *TTPT =
         dyn_cast<TemplateTypeParmType>(Friend->getFriendType()->getType());
@@ -46,8 +36,8 @@ bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
   return false;
 }
 
-bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
-                              const CXXRecordDecl *Derived) {
+static bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
+                                     const CXXRecordDecl *Derived) {
   for (auto &&Friend : CRTP->friends()) {
     if (Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived)
       return true;
@@ -56,7 +46,7 @@ bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
   return false;
 }
 
-std::optional<const NamedDecl *>
+static std::optional<const NamedDecl *>
 getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
                     const CXXRecordDecl *Derived) {
   size_t Idx = 0;
@@ -76,10 +66,10 @@ getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
   return CRTP->getSpecializedTemplate()->getTemplateParameters()->getParam(Idx);
 }
 
-std::vector<FixItHint> hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
-                                           const std::string &OriginalAccess,
-                                           const SourceManager &SM,
-                                           const LangOptions &LangOpts) {
+static std::vector<FixItHint>
+hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
+                    const std::string &OriginalAccess, const SourceManager &SM,
+                    const LangOptions &LangOpts) {
   std::vector<FixItHint> Hints;
 
   Hints.emplace_back(FixItHint::CreateInsertion(
@@ -94,14 +84,14 @@ std::vector<FixItHint> hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
 
   return Hints;
 }
-} // namespace
 
 void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       classTemplateSpecializationDecl(
           decl().bind("crtp"),
           hasAnyTemplateArgument(refersToType(recordType(hasDeclaration(
-              cxxRecordDecl(isDerivedFrom(cxxRecordDecl(isBoundNode("crtp"))))
+              cxxRecordDecl(
+                  isDerivedFrom(cxxRecordDecl(equalsBoundNode("crtp"))))
                   .bind("derived")))))),
       this);
 }
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
index fb7245edeb0fa9..96f252d6707dc9 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
@@ -13,7 +13,8 @@
 
 namespace clang::tidy::bugprone {
 
-/// Detects error-prone CRTP usage.
+/// Detects error-prone CRTP usage, when the CRTP can be constructed outside
+/// itself and the derived class.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-crtp.html
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d86ac48c6b3aec..74e64e97138d97 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -107,7 +107,7 @@ New checks
 - New :doc:`bugprone-unsafe-crtp
   <clang-tidy/checks/bugprone/unsafe-crtp>` check.
 
-  Detects error-prone CRTP usage.
+  Detects error-prone CRTP usage, when the CRTP can be constructed outside itself and the derived class.
 
 - New :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
index edcfd67677fd7b..f3dc9ea46d0ec9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-unsafe-crtp %t
+// RUN: %check_clang_tidy %s bugprone-unsafe-crtp %t -- -- -fno-delayed-template-parsing
 
 namespace class_implicit_ctor {
 template <typename T>

>From 2ccce59e8b8a91ae70cae200b32840d24b639e01 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 00:29:08 +0100
Subject: [PATCH 16/31] rename check

---
 .../bugprone/BugproneTidyModule.cpp           |  4 +-
 .../clang-tidy/bugprone/CMakeLists.txt        |  2 +-
 ... => CrtpConstructorAccessibilityCheck.cpp} | 10 ++---
 ....h => CrtpConstructorAccessibilityCheck.h} | 14 +++----
 clang-tools-extra/docs/ReleaseNotes.rst       |  4 +-
 ...rst => crtp-constructor-accessibility.rst} |  6 +--
 .../docs/clang-tidy/checks/list.rst           |  2 +-
 ...cpp => crtp-constructor-accessibility.cpp} | 40 +++++++++----------
 8 files changed, 41 insertions(+), 41 deletions(-)
 rename clang-tools-extra/clang-tidy/bugprone/{UnsafeCrtpCheck.cpp => CrtpConstructorAccessibilityCheck.cpp} (93%)
 rename clang-tools-extra/clang-tidy/bugprone/{UnsafeCrtpCheck.h => CrtpConstructorAccessibilityCheck.h} (59%)
 rename clang-tools-extra/docs/clang-tidy/checks/bugprone/{unsafe-crtp.rst => crtp-constructor-accessibility.rst} (89%)
 rename clang-tools-extra/test/clang-tidy/checkers/bugprone/{unsafe-crtp.cpp => crtp-constructor-accessibility.cpp} (84%)

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 78e128fb0e5e6d..0e2957ee21a791 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -83,7 +83,7 @@
 #include "UnhandledExceptionAtNewCheck.h"
 #include "UnhandledSelfAssignmentCheck.h"
 #include "UniquePtrArrayMismatchCheck.h"
-#include "UnsafeCrtpCheck.h"
+#include "CrtpConstructorAccessibilityCheck.h"
 #include "UnsafeFunctionsCheck.h"
 #include "UnusedLocalNonTrivialVariableCheck.h"
 #include "UnusedRaiiCheck.h"
@@ -238,7 +238,7 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-unhandled-exception-at-new");
     CheckFactories.registerCheck<UniquePtrArrayMismatchCheck>(
         "bugprone-unique-ptr-array-mismatch");
-    CheckFactories.registerCheck<UnsafeCrtpCheck>("bugprone-unsafe-crtp");
+    CheckFactories.registerCheck<CrtpConstructorAccessibilityCheck>("bugprone-crtp-constructor-accessibility");
     CheckFactories.registerCheck<UnsafeFunctionsCheck>(
         "bugprone-unsafe-functions");
     CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 956b12ad1cdecd..638fba03a43587 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -79,7 +79,7 @@ add_clang_library(clangTidyBugproneModule
   UnhandledExceptionAtNewCheck.cpp
   UnhandledSelfAssignmentCheck.cpp
   UniquePtrArrayMismatchCheck.cpp
-  UnsafeCrtpCheck.cpp
+  CrtpConstructorAccessibilityCheck.cpp
   UnsafeFunctionsCheck.cpp
   UnusedLocalNonTrivialVariableCheck.cpp
   UnusedRaiiCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
similarity index 93%
rename from clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
rename to clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index bdbb2ab7da35e5..8c78d293239c9e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -1,4 +1,4 @@
-//===--- UnsafeCrtpCheck.cpp - clang-tidy ---------------------------------===//
+//===--- CrtpConstructorAccessibilityCheck.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,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "UnsafeCrtpCheck.h"
+#include "CrtpConstructorAccessibilityCheck.h"
 #include "../utils/LexerUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
@@ -85,7 +85,7 @@ hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
   return Hints;
 }
 
-void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
+void CrtpConstructorAccessibilityCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       classTemplateSpecializationDecl(
           decl().bind("crtp"),
@@ -96,7 +96,7 @@ void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
       this);
 }
 
-void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
+void CrtpConstructorAccessibilityCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CRTPInstantiation =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
   const auto *DerivedRecord = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
@@ -154,7 +154,7 @@ void UnsafeCrtpCheck::check(const MatchFinder::MatchResult &Result) {
   }
 }
 
-bool UnsafeCrtpCheck::isLanguageVersionSupported(
+bool CrtpConstructorAccessibilityCheck::isLanguageVersionSupported(
     const LangOptions &LangOpts) const {
   return LangOpts.CPlusPlus;
 }
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h
similarity index 59%
rename from clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
rename to clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h
index 96f252d6707dc9..cf267c756bd041 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeCrtpCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h
@@ -1,4 +1,4 @@
-//===--- UnsafeCrtpCheck.h - clang-tidy -------------------------*- C++ -*-===//
+//===--- CrtpConstructorAccessibilityCheck.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_UNSAFECRTPCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFECRTPCHECK_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CRTPCONSTRUCTORACCESSIBILITYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CRTPCONSTRUCTORACCESSIBILITYCHECK_H
 
 #include "../ClangTidyCheck.h"
 
@@ -17,10 +17,10 @@ namespace clang::tidy::bugprone {
 /// itself and the derived class.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-crtp.html
-class UnsafeCrtpCheck : public ClangTidyCheck {
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/crtp-constructor-accessibility.html
+class CrtpConstructorAccessibilityCheck : public ClangTidyCheck {
 public:
-  UnsafeCrtpCheck(StringRef Name, ClangTidyContext *Context)
+  CrtpConstructorAccessibilityCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -29,4 +29,4 @@ class UnsafeCrtpCheck : public ClangTidyCheck {
 
 } // namespace clang::tidy::bugprone
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFECRTPCHECK_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CRTPCONSTRUCTORACCESSIBILITYCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 74e64e97138d97..016c6e81cccbef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,8 +104,8 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
-- New :doc:`bugprone-unsafe-crtp
-  <clang-tidy/checks/bugprone/unsafe-crtp>` check.
+- New :doc:`bugprone-crtp-constructor-accessibility
+  <clang-tidy/checks/bugprone/crtp-constructor-accessibility>` check.
 
   Detects error-prone CRTP usage, when the CRTP can be constructed outside itself and the derived class.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
similarity index 89%
rename from clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
rename to clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
index 8e6955999512a6..cbbffeafcf90fc 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-crtp.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
@@ -1,7 +1,7 @@
-.. title:: clang-tidy - bugprone-unsafe-crtp
+.. title:: clang-tidy - bugprone-crtp-constructor-accessibility
 
-bugprone-unsafe-crtp
-====================
+bugprone-crtp-constructor-accessibility
+=======================================
 
 Finds CRTP used in an error-prone way.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 67e9413749b444..d03e7af688f007 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@ Clang-Tidy Checks
    :doc:`bugprone-chained-comparison <bugprone/chained-comparison>`,
    :doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
    :doc:`bugprone-copy-constructor-init <bugprone/copy-constructor-init>`, "Yes"
+   :doc:`bugprone-crtp-constructor-accessibility <bugprone/crtp-constructor-accessibility>`, "Yes"
    :doc:`bugprone-dangling-handle <bugprone/dangling-handle>`,
    :doc:`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers>`,
    :doc:`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters>`,
@@ -149,7 +150,6 @@ Clang-Tidy Checks
    :doc:`bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new>`,
    :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`,
    :doc:`bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch>`, "Yes"
-   :doc:`bugprone-unsafe-crtp <bugprone/unsafe-crtp>`, "Yes"
    :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`,
    :doc:`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>`,
    :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
similarity index 84%
rename from clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
rename to clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index f3dc9ea46d0ec9..e4efd01007c9dd 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-crtp.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s bugprone-unsafe-crtp %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s bugprone-crtp-constructor-accessibility %t -- -- -fno-delayed-template-parsing
 
 namespace class_implicit_ctor {
 template <typename T>
 class CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
 // CHECK-FIXES: CRTP() = default;
 
@@ -13,7 +13,7 @@ class A : CRTP<A> {};
 namespace class_uncostructible {
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
 // CHECK-FIXES: friend T;
     CRTP() = default;
@@ -27,7 +27,7 @@ template <typename T>
 class CRTP {
 public:
     CRTP() = default;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
 };
@@ -40,7 +40,7 @@ template <typename T>
 class CRTP {
 public:
     CRTP(int) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
 };
@@ -53,11 +53,11 @@ template <typename T>
 class CRTP {
 public:
     CRTP(int) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
     CRTP(float) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}public:
 };
@@ -70,15 +70,15 @@ template <typename T>
 class CRTP {
 protected:
     CRTP(int) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}protected:
     CRTP() = default;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}protected:
     CRTP(float) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}protected:
 };
@@ -89,7 +89,7 @@ class A : CRTP<A> {};
 namespace struct_implicit_ctor {
 template <typename T>
 struct CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
 
@@ -100,7 +100,7 @@ namespace struct_default_ctor {
 template <typename T>
 struct CRTP {
     CRTP() = default;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-unsafe-crtp]
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
     // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
 };
@@ -111,13 +111,13 @@ class A : CRTP<A> {};
 namespace same_class_multiple_crtps {
 template <typename T>
 struct CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
 
 template <typename T>
 struct CRTP2 {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP2() = default;{{[[:space:]]*}}public:
 
@@ -127,7 +127,7 @@ class A : CRTP<A>, CRTP2<A> {};
 namespace same_crtp_multiple_classes {
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
 // CHECK-FIXES: friend T;
     CRTP() = default;
@@ -140,7 +140,7 @@ class B : CRTP<B> {};
 namespace crtp_template {
 template <typename T, typename U>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
 // CHECK-FIXES: friend U;
     CRTP() = default;
@@ -152,7 +152,7 @@ class A : CRTP<int, A> {};
 namespace crtp_template2 {
 template <typename T, typename U>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
 // CHECK-FIXES: friend T;
     CRTP() = default;
@@ -164,7 +164,7 @@ class A : CRTP<A, A> {};
 namespace template_derived {
 template <typename T>
 class CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
 // CHECK-FIXES: CRTP() = default;
 
@@ -181,7 +181,7 @@ void foo() {
 namespace template_derived_explicit_specialization {
 template <typename T>
 class CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
 // CHECK-FIXES: CRTP() = default;
 
@@ -209,7 +209,7 @@ class A;
 
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-unsafe-crtp]
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
 // CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
 // CHECK-FIXES: friend T;
     CRTP() = default;

>From b28d6dfdca027c579d684d6ff3b1f8f93b74cac9 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 01:16:53 +0100
Subject: [PATCH 17/31] comments

---
 .../bugprone/CrtpConstructorAccessibilityCheck.cpp        | 8 +++++---
 .../checks/bugprone/crtp-constructor-accessibility.rst    | 3 ++-
 .../checkers/bugprone/crtp-constructor-accessibility.cpp  | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 8c78d293239c9e..86f291a83b1875 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -1,4 +1,5 @@
-//===--- CrtpConstructorAccessibilityCheck.cpp - clang-tidy ---------------------------------===//
+//===--- CrtpConstructorAccessibilityCheck.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.
@@ -96,7 +97,8 @@ void CrtpConstructorAccessibilityCheck::registerMatchers(MatchFinder *Finder) {
       this);
 }
 
-void CrtpConstructorAccessibilityCheck::check(const MatchFinder::MatchResult &Result) {
+void CrtpConstructorAccessibilityCheck::check(
+    const MatchFinder::MatchResult &Result) {
   const auto *CRTPInstantiation =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
   const auto *DerivedRecord = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
@@ -156,6 +158,6 @@ void CrtpConstructorAccessibilityCheck::check(const MatchFinder::MatchResult &Re
 
 bool CrtpConstructorAccessibilityCheck::isLanguageVersionSupported(
     const LangOptions &LangOpts) const {
-  return LangOpts.CPlusPlus;
+  return LangOpts.CPlusPlus11;
 }
 } // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
index cbbffeafcf90fc..ecf05ab6b3a421 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
@@ -42,7 +42,8 @@ Example:
   Bad BadInstance;
 
 To ensure that no accidental instantiation happens, the best practice is to make
-the constructor private and declare the derived class as friend.
+the constructor private and declare the derived class as friend. Note that as a tradeoff, 
+this also gives the derived class access to every other private members of the CRTP.
 
 Example:
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index e4efd01007c9dd..fcd21c291eb77c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-crtp-constructor-accessibility %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-crtp-constructor-accessibility %t -- -- -fno-delayed-template-parsing
 
 namespace class_implicit_ctor {
 template <typename T>

>From 7e487de8a852ebbfe29c2b436838f4129e687306 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 01:38:36 +0100
Subject: [PATCH 18/31] format

---
 clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 0e2957ee21a791..e518a64abc52eb 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -20,6 +20,7 @@
 #include "ChainedComparisonCheck.h"
 #include "ComparePointerToMemberVirtualFunctionCheck.h"
 #include "CopyConstructorInitCheck.h"
+#include "CrtpConstructorAccessibilityCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DynamicStaticInitializersCheck.h"
 #include "EasilySwappableParametersCheck.h"
@@ -83,7 +84,6 @@
 #include "UnhandledExceptionAtNewCheck.h"
 #include "UnhandledSelfAssignmentCheck.h"
 #include "UniquePtrArrayMismatchCheck.h"
-#include "CrtpConstructorAccessibilityCheck.h"
 #include "UnsafeFunctionsCheck.h"
 #include "UnusedLocalNonTrivialVariableCheck.h"
 #include "UnusedRaiiCheck.h"
@@ -238,7 +238,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-unhandled-exception-at-new");
     CheckFactories.registerCheck<UniquePtrArrayMismatchCheck>(
         "bugprone-unique-ptr-array-mismatch");
-    CheckFactories.registerCheck<CrtpConstructorAccessibilityCheck>("bugprone-crtp-constructor-accessibility");
+    CheckFactories.registerCheck<CrtpConstructorAccessibilityCheck>(
+        "bugprone-crtp-constructor-accessibility");
     CheckFactories.registerCheck<UnsafeFunctionsCheck>(
         "bugprone-unsafe-functions");
     CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>(

>From 24338d5cc54b820e23d59172498400060b27ee96 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 02:32:44 +0100
Subject: [PATCH 19/31] comments

---
 .../CrtpConstructorAccessibilityCheck.cpp     | 71 ++++++++-----------
 .../crtp-constructor-accessibility.cpp        |  4 +-
 2 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 86f291a83b1875..8520d124ad0c16 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -16,69 +16,59 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::bugprone {
 
 static bool hasPrivateConstructor(const CXXRecordDecl *RD) {
-  for (auto &&Ctor : RD->ctors()) {
-    if (Ctor->getAccess() == AS_private)
-      return true;
-  }
-
-  return false;
+  return llvm::any_of(RD->ctors(), [](const CXXConstructorDecl *Ctor) {
+    return Ctor->getAccess() == AS_private;
+  });
 }
 
 static bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
                                          const NamedDecl *Param) {
-  for (auto &&Friend : CRTP->friends()) {
+  return llvm::any_of(CRTP->friends(), [&](const FriendDecl *Friend) {
     const auto *TTPT =
         dyn_cast<TemplateTypeParmType>(Friend->getFriendType()->getType());
 
-    if (TTPT && TTPT->getDecl() == Param)
-      return true;
-  }
-
-  return false;
+    return TTPT && TTPT->getDecl() == Param;
+  });
 }
 
 static bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
                                      const CXXRecordDecl *Derived) {
-  for (auto &&Friend : CRTP->friends()) {
-    if (Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived)
-      return true;
-  }
-
-  return false;
+  return llvm::any_of(CRTP->friends(), [&](const FriendDecl *Friend) {
+    return Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived;
+  });
 }
 
-static std::optional<const NamedDecl *>
+static const NamedDecl *
 getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
                     const CXXRecordDecl *Derived) {
   size_t Idx = 0;
-  bool Found = false;
-  for (auto &&TemplateArg : CRTP->getTemplateArgs().asArray()) {
-    if (TemplateArg.getKind() == TemplateArgument::Type &&
-        TemplateArg.getAsType()->getAsCXXRecordDecl() == Derived) {
-      Found = true;
-      break;
-    }
-    ++Idx;
-  }
-
-  if (!Found)
-    return std::nullopt;
-
-  return CRTP->getSpecializedTemplate()->getTemplateParameters()->getParam(Idx);
+  bool AnyOf = llvm::any_of(
+      CRTP->getTemplateArgs().asArray(), [&](const TemplateArgument &Arg) {
+        ++Idx;
+        return Arg.getKind() == TemplateArgument::Type &&
+               Arg.getAsType()->getAsCXXRecordDecl() == Derived;
+      });
+
+  return AnyOf ? CRTP->getSpecializedTemplate()
+                     ->getTemplateParameters()
+                     ->getParam(Idx - 1)
+               : nullptr;
 }
 
 static std::vector<FixItHint>
 hintMakeCtorPrivate(const CXXConstructorDecl *Ctor,
-                    const std::string &OriginalAccess, const SourceManager &SM,
-                    const LangOptions &LangOpts) {
+                    const std::string &OriginalAccess) {
   std::vector<FixItHint> Hints;
 
   Hints.emplace_back(FixItHint::CreateInsertion(
       Ctor->getBeginLoc().getLocWithOffset(-1), "private:\n"));
 
+  const ASTContext &ASTCtx = Ctor->getASTContext();
   const SourceLocation CtorEndLoc =
       Ctor->isExplicitlyDefaulted()
-          ? utils::lexer::findNextTerminator(Ctor->getEndLoc(), SM, LangOpts)
+          ? utils::lexer::findNextTerminator(Ctor->getEndLoc(),
+                                             ASTCtx.getSourceManager(),
+                                             ASTCtx.getLangOpts())
           : Ctor->getEndLoc();
   Hints.emplace_back(FixItHint::CreateInsertion(
       CtorEndLoc.getLocWithOffset(1), '\n' + OriginalAccess + ':' + '\n'));
@@ -121,7 +111,10 @@ void CrtpConstructorAccessibilityCheck::check(
   }
 
   const auto *DerivedTemplateParameter =
-      *getDerivedParameter(CRTPInstantiation, DerivedRecord);
+      getDerivedParameter(CRTPInstantiation, DerivedRecord);
+
+  assert(DerivedTemplateParameter &&
+         "No template parameter corresponds to the derived class of the CRTP.");
 
   if (hasPrivateConstructor(CRTPDeclaration) &&
       !isDerivedParameterBefriended(CRTPDeclaration,
@@ -148,9 +141,7 @@ void CrtpConstructorAccessibilityCheck::check(
     diag(Ctor->getLocation(),
          "%0 contructor allows the CRTP to be %select{inherited "
          "from|constructed}1 as a regular template class")
-        << Access << IsPublic << Ctor
-        << hintMakeCtorPrivate(Ctor, Access, *Result.SourceManager,
-                               getLangOpts());
+        << Access << IsPublic << Ctor << hintMakeCtorPrivate(Ctor, Access);
     diag(Ctor->getLocation(), "consider making it private",
          DiagnosticIDs::Note);
   }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index fcd21c291eb77c..df0611c62bd93c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -10,7 +10,7 @@ class CRTP {};
 class A : CRTP<A> {};
 } // namespace class_implicit_ctor
 
-namespace class_uncostructible {
+namespace class_unconstructible {
 template <typename T>
 class CRTP {
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
@@ -20,7 +20,7 @@ class CRTP {
 };
 
 class A : CRTP<A> {};
-} // namespace class_uncostructible 
+} // namespace class_unconstructible
 
 namespace class_public_default_ctor {
 template <typename T>

>From b53538c0b8ee3582d4e65c288c3b100435037098 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 03:22:56 +0100
Subject: [PATCH 20/31] concatenate friend fix with others

---
 .../CrtpConstructorAccessibilityCheck.cpp     | 86 ++++++++++++-------
 .../crtp-constructor-accessibility.cpp        | 44 +++++++---
 2 files changed, 86 insertions(+), 44 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 8520d124ad0c16..506be9c037820c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -42,7 +42,7 @@ static const NamedDecl *
 getDerivedParameter(const ClassTemplateSpecializationDecl *CRTP,
                     const CXXRecordDecl *Derived) {
   size_t Idx = 0;
-  bool AnyOf = llvm::any_of(
+  const bool AnyOf = llvm::any_of(
       CRTP->getTemplateArgs().asArray(), [&](const TemplateArgument &Arg) {
         ++Idx;
         return Arg.getKind() == TemplateArgument::Type &&
@@ -95,38 +95,52 @@ void CrtpConstructorAccessibilityCheck::check(
   const CXXRecordDecl *CRTPDeclaration =
       CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
 
-  if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
-    const bool IsStruct = CRTPDeclaration->isStruct();
-
-    diag(CRTPDeclaration->getLocation(),
-         "the implicit default constructor of the CRTP is publicly accessible")
-        << CRTPDeclaration
-        << FixItHint::CreateInsertion(
-               CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(1),
-               (IsStruct ? "\nprivate:\n" : "\n") +
-                   CRTPDeclaration->getNameAsString() + "() = default;\n" +
-                   (IsStruct ? "public:\n" : ""));
-    diag(CRTPDeclaration->getLocation(), "consider making it private",
-         DiagnosticIDs::Note);
-  }
-
   const auto *DerivedTemplateParameter =
       getDerivedParameter(CRTPInstantiation, DerivedRecord);
 
   assert(DerivedTemplateParameter &&
          "No template parameter corresponds to the derived class of the CRTP.");
 
-  if (hasPrivateConstructor(CRTPDeclaration) &&
-      !isDerivedParameterBefriended(CRTPDeclaration,
-                                    DerivedTemplateParameter) &&
-      !isDerivedClassBefriended(CRTPDeclaration, DerivedRecord)) {
+  bool NeedsFriend = !isDerivedParameterBefriended(CRTPDeclaration,
+                                                   DerivedTemplateParameter) &&
+                     !isDerivedClassBefriended(CRTPDeclaration, DerivedRecord);
+
+  const FixItHint HintFriend = FixItHint::CreateInsertion(
+      CRTPDeclaration->getBraceRange().getEnd(),
+      "friend " + DerivedTemplateParameter->getNameAsString() + ';' + '\n');
+
+  if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
+    const bool IsStruct = CRTPDeclaration->isStruct();
+
+    // FIXME: Clean this up!
+    {
+      DiagnosticBuilder Diag =
+          diag(CRTPDeclaration->getLocation(),
+               "the implicit default constructor of the CRTP is publicly "
+               "accessible")
+          << CRTPDeclaration
+          << FixItHint::CreateInsertion(
+                 CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(
+                     1),
+                 (IsStruct ? "\nprivate:\n" : "\n") +
+                     CRTPDeclaration->getNameAsString() + "() = default;\n" +
+                     (IsStruct ? "public:\n" : ""));
+
+      if (NeedsFriend)
+        Diag << HintFriend;
+    }
+
+    diag(CRTPDeclaration->getLocation(),
+         "consider making it private%select{| and declaring the derived class "
+         "as friend}0",
+         DiagnosticIDs::Note)
+        << NeedsFriend;
+  }
+
+  if (hasPrivateConstructor(CRTPDeclaration) && NeedsFriend) {
     diag(CRTPDeclaration->getLocation(),
          "the CRTP cannot be constructed from the derived class")
-        << CRTPDeclaration
-        << FixItHint::CreateInsertion(
-               CRTPDeclaration->getBraceRange().getEnd(),
-               "friend " + DerivedTemplateParameter->getNameAsString() + ';' +
-                   '\n');
+        << CRTPDeclaration << HintFriend;
     diag(CRTPDeclaration->getLocation(),
          "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
@@ -138,12 +152,24 @@ void CrtpConstructorAccessibilityCheck::check(
     const bool IsPublic = Ctor->getAccess() == AS_public;
     const std::string Access = IsPublic ? "public" : "protected";
 
+    // FIXME: Clean this up!
+    {
+      DiagnosticBuilder Diag =
+          diag(Ctor->getLocation(),
+               "%0 contructor allows the CRTP to be %select{inherited "
+               "from|constructed}1 as a regular template class")
+          << Access << IsPublic << Ctor << hintMakeCtorPrivate(Ctor, Access);
+
+      if (NeedsFriend)
+        Diag << HintFriend;
+    }
+
+    // FIXME: Test the NeedsFriend false case!
     diag(Ctor->getLocation(),
-         "%0 contructor allows the CRTP to be %select{inherited "
-         "from|constructed}1 as a regular template class")
-        << Access << IsPublic << Ctor << hintMakeCtorPrivate(Ctor, Access);
-    diag(Ctor->getLocation(), "consider making it private",
-         DiagnosticIDs::Note);
+         "consider making it private%select{| and declaring the derived class "
+         "as friend}0",
+         DiagnosticIDs::Note)
+        << NeedsFriend;
   }
 }
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index df0611c62bd93c..ab8f3a92299656 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -4,8 +4,9 @@ namespace class_implicit_ctor {
 template <typename T>
 class CRTP {};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private and declaring the derived class as friend
 // CHECK-FIXES: CRTP() = default;
+// CHECK-FIXES: friend T;
 
 class A : CRTP<A> {};
 } // namespace class_implicit_ctor
@@ -28,8 +29,9 @@ class CRTP {
 public:
     CRTP() = default;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+    // CHECK-FIXES: friend T;
 };
 
 class A : CRTP<A> {};
@@ -41,8 +43,9 @@ class CRTP {
 public:
     CRTP(int) {}
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
+    // CHECK-FIXES: friend T;
 };
 
 class A : CRTP<A> {};
@@ -54,12 +57,15 @@ class CRTP {
 public:
     CRTP(int) {}
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
     CRTP(float) {}
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}public:
+    
+    // CHECK-FIXES: friend T;
+    // CHECK-FIXES: friend T;
 };
 
 class A : CRTP<A> {};
@@ -71,16 +77,20 @@ class CRTP {
 protected:
     CRTP(int) {}
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}protected:
     CRTP() = default;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}protected:
     CRTP(float) {}
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}protected:
+    
+    // CHECK-FIXES: friend T;
+    // CHECK-FIXES: friend T;
+    // CHECK-FIXES: friend T;
 };
 
 class A : CRTP<A> {};
@@ -90,8 +100,9 @@ namespace struct_implicit_ctor {
 template <typename T>
 struct CRTP {};
 // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private and declaring the derived class as friend
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+// CHECK-FIXES: friend T;
 
 class A : CRTP<A> {};
 } // namespace struct_implicit_ctor
@@ -101,8 +112,9 @@ template <typename T>
 struct CRTP {
     CRTP() = default;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private
+    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+    // CHECK-FIXES: friend T;
 };
 
 class A : CRTP<A> {};
@@ -112,14 +124,16 @@ namespace same_class_multiple_crtps {
 template <typename T>
 struct CRTP {};
 // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private and declaring the derived class as friend
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
+// CHECK-FIXES: friend T;
 
 template <typename T>
 struct CRTP2 {};
 // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private and declaring the derived class as friend
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP2() = default;{{[[:space:]]*}}public:
+// CHECK-FIXES: friend T;
 
 class A : CRTP<A>, CRTP2<A> {};
 } // namespace same_class_multiple_crtps
@@ -165,8 +179,9 @@ namespace template_derived {
 template <typename T>
 class CRTP {};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private and declaring the derived class as friend
 // CHECK-FIXES: CRTP() = default;
+// CHECK-FIXES: friend T;
 
 template<typename T>
 class A : CRTP<A<T>> {};
@@ -182,8 +197,9 @@ namespace template_derived_explicit_specialization {
 template <typename T>
 class CRTP {};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private and declaring the derived class as friend
 // CHECK-FIXES: CRTP() = default;
+// CHECK-FIXES: friend T;
 
 template<typename T>
 class A : CRTP<A<T>> {};

>From f8309899bd55bb9a6963886b598fc10fadb4b6d0 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 19:59:30 +0100
Subject: [PATCH 21/31] test

---
 .../bugprone/CrtpConstructorAccessibilityCheck.cpp |  1 -
 .../bugprone/crtp-constructor-accessibility.cpp    | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 506be9c037820c..ff860bb51117b0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -164,7 +164,6 @@ void CrtpConstructorAccessibilityCheck::check(
         Diag << HintFriend;
     }
 
-    // FIXME: Test the NeedsFriend false case!
     diag(Ctor->getLocation(),
          "consider making it private%select{| and declaring the derived class "
          "as friend}0",
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index ab8f3a92299656..8b31ed55380e2a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -236,6 +236,20 @@ class A : CRTP<A> {};
 class B : CRTP<B> {};
 } // namespace explicit_derived_friend_multiple
 
+namespace no_need_for_friend {
+class A;
+
+template <typename T>
+class CRTP {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
+// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-FIXES: CRTP() = default;
+    friend A;
+};
+
+class A : CRTP<A> {};
+} // namespace no_need_for_friend
+
 namespace no_warning {
 template <typename T>
 class CRTP

>From d18a72756ca538c4da468ec4516d1a5be82b77c9 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 20:29:50 +0100
Subject: [PATCH 22/31] cleanup

---
 .../CrtpConstructorAccessibilityCheck.cpp     | 88 +++++++++----------
 1 file changed, 43 insertions(+), 45 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index ff860bb51117b0..ffdd53c27ce468 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -109,34 +109,6 @@ void CrtpConstructorAccessibilityCheck::check(
       CRTPDeclaration->getBraceRange().getEnd(),
       "friend " + DerivedTemplateParameter->getNameAsString() + ';' + '\n');
 
-  if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
-    const bool IsStruct = CRTPDeclaration->isStruct();
-
-    // FIXME: Clean this up!
-    {
-      DiagnosticBuilder Diag =
-          diag(CRTPDeclaration->getLocation(),
-               "the implicit default constructor of the CRTP is publicly "
-               "accessible")
-          << CRTPDeclaration
-          << FixItHint::CreateInsertion(
-                 CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(
-                     1),
-                 (IsStruct ? "\nprivate:\n" : "\n") +
-                     CRTPDeclaration->getNameAsString() + "() = default;\n" +
-                     (IsStruct ? "public:\n" : ""));
-
-      if (NeedsFriend)
-        Diag << HintFriend;
-    }
-
-    diag(CRTPDeclaration->getLocation(),
-         "consider making it private%select{| and declaring the derived class "
-         "as friend}0",
-         DiagnosticIDs::Note)
-        << NeedsFriend;
-  }
-
   if (hasPrivateConstructor(CRTPDeclaration) && NeedsFriend) {
     diag(CRTPDeclaration->getLocation(),
          "the CRTP cannot be constructed from the derived class")
@@ -145,6 +117,41 @@ void CrtpConstructorAccessibilityCheck::check(
          "consider declaring the derived class as friend", DiagnosticIDs::Note);
   }
 
+  auto DiagNoteFriendPrivate = [&](const SourceLocation &Loc, bool Friend) {
+    return diag(Loc,
+                "consider making it private%select{| and declaring the "
+                "derived class "
+                "as friend}0",
+                DiagnosticIDs::Note)
+           << Friend;
+  };
+
+  auto WithFriendHintIfNeeded = [&](DiagnosticBuilder Diag, bool NeedsFriend) {
+    if (NeedsFriend)
+      Diag << HintFriend;
+
+    return Diag;
+  };
+
+  if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
+    const bool IsStruct = CRTPDeclaration->isStruct();
+
+    WithFriendHintIfNeeded(
+        diag(CRTPDeclaration->getLocation(),
+             "the implicit default constructor of the CRTP is publicly "
+             "accessible")
+            << CRTPDeclaration
+            << FixItHint::CreateInsertion(
+                   CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(
+                       1),
+                   (IsStruct ? "\nprivate:\n" : "\n") +
+                       CRTPDeclaration->getNameAsString() + "() = default;\n" +
+                       (IsStruct ? "public:\n" : "")),
+        NeedsFriend);
+
+    DiagNoteFriendPrivate(CRTPDeclaration->getLocation(), NeedsFriend);
+  }
+
   for (auto &&Ctor : CRTPDeclaration->ctors()) {
     if (Ctor->getAccess() == AS_private)
       continue;
@@ -152,23 +159,14 @@ void CrtpConstructorAccessibilityCheck::check(
     const bool IsPublic = Ctor->getAccess() == AS_public;
     const std::string Access = IsPublic ? "public" : "protected";
 
-    // FIXME: Clean this up!
-    {
-      DiagnosticBuilder Diag =
-          diag(Ctor->getLocation(),
-               "%0 contructor allows the CRTP to be %select{inherited "
-               "from|constructed}1 as a regular template class")
-          << Access << IsPublic << Ctor << hintMakeCtorPrivate(Ctor, Access);
-
-      if (NeedsFriend)
-        Diag << HintFriend;
-    }
-
-    diag(Ctor->getLocation(),
-         "consider making it private%select{| and declaring the derived class "
-         "as friend}0",
-         DiagnosticIDs::Note)
-        << NeedsFriend;
+    WithFriendHintIfNeeded(
+        diag(Ctor->getLocation(),
+             "%0 contructor allows the CRTP to be %select{inherited "
+             "from|constructed}1 as a regular template class")
+            << Access << IsPublic << Ctor << hintMakeCtorPrivate(Ctor, Access),
+        NeedsFriend);
+
+    DiagNoteFriendPrivate(Ctor->getLocation(), NeedsFriend);
   }
 }
 

>From 64c1fcc68848f675f22498dced8b76c503b76a18 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 20:37:49 +0100
Subject: [PATCH 23/31] test

---
 .../bugprone/crtp-constructor-accessibility.cpp     | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index 8b31ed55380e2a..d2e300389bbe72 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -260,3 +260,16 @@ class CRTP
 
 class A : CRTP<A> {};
 } // namespace no_warning
+
+namespace no_warning_unsupported {
+template<typename... Types>
+class CRTP
+{};
+
+class A : CRTP<A> {};
+
+void foo() {
+    A A;
+    (void) A;
+}
+} // namespace no_warning_unsupported

>From 4ba7b13d96cf552c0a50b3e30ef4434d7889d882 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Fri, 23 Feb 2024 21:14:22 +0100
Subject: [PATCH 24/31] docs

---
 .../crtp-constructor-accessibility.rst        | 21 ++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
index ecf05ab6b3a421..b902b6439a4637 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
@@ -3,7 +3,26 @@
 bugprone-crtp-constructor-accessibility
 =======================================
 
-Finds CRTP used in an error-prone way.
+Finds Curiously Recurring Template Pattern used in an error-prone way.
+
+The CRTP is an idiom, in which a class derives from a template class, where 
+itself is the template argument. It should be ensured that if a class is
+intended to be a base class in this idiom, it can only be instantiated if
+the derived class is it's template argument.
+
+Example:
+
+.. code-block:: c++
+
+  template <typename T> class CRTP {
+  private:
+    CRTP() = default;
+    friend T;
+  };
+
+  class Derived : CRTP<Derived> {};
+
+Below can be seen some common mistakes that will allow the breaking of the idiom.
 
 If the constructor of a class intended to be used in a CRTP is public, then
 it allows users to construct that class on its own.

>From 458f63897a4a817c4bf8722986919e54c166c21d Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sat, 24 Feb 2024 03:15:12 +0100
Subject: [PATCH 25/31] format

---
 .../bugprone/CrtpConstructorAccessibilityCheck.cpp           | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index ffdd53c27ce468..443a4bf0ec928a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -119,9 +119,8 @@ void CrtpConstructorAccessibilityCheck::check(
 
   auto DiagNoteFriendPrivate = [&](const SourceLocation &Loc, bool Friend) {
     return diag(Loc,
-                "consider making it private%select{| and declaring the "
-                "derived class "
-                "as friend}0",
+                "consider making it private%select{| and declaring the derived "
+                "class as friend}0",
                 DiagnosticIDs::Note)
            << Friend;
   };

>From edebaad34451e0d7eb4276ce1b71d2d17ec5b883 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sat, 24 Feb 2024 03:23:49 +0100
Subject: [PATCH 26/31] docs

---
 .../bugprone/CrtpConstructorAccessibilityCheck.h    |  4 ++--
 clang-tools-extra/docs/ReleaseNotes.rst             |  3 ++-
 .../bugprone/crtp-constructor-accessibility.rst     | 13 ++++++++-----
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h
index cf267c756bd041..785116218f4689 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.h
@@ -13,8 +13,8 @@
 
 namespace clang::tidy::bugprone {
 
-/// Detects error-prone CRTP usage, when the CRTP can be constructed outside
-/// itself and the derived class.
+/// Detects error-prone Curiously Recurring Template Pattern usage, when the
+/// CRTP can be constructed outside itself and the derived class.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/crtp-constructor-accessibility.html
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 016c6e81cccbef..20af1d0232edde 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -107,7 +107,8 @@ New checks
 - New :doc:`bugprone-crtp-constructor-accessibility
   <clang-tidy/checks/bugprone/crtp-constructor-accessibility>` check.
 
-  Detects error-prone CRTP usage, when the CRTP can be constructed outside itself and the derived class.
+  Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
+  can be constructed outside itself and the derived class.
 
 - New :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
index b902b6439a4637..c527db1c0b8a66 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
@@ -3,7 +3,8 @@
 bugprone-crtp-constructor-accessibility
 =======================================
 
-Finds Curiously Recurring Template Pattern used in an error-prone way.
+Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
+can be constructed outside itself and the derived class.
 
 The CRTP is an idiom, in which a class derives from a template class, where 
 itself is the template argument. It should be ensured that if a class is
@@ -22,7 +23,8 @@ Example:
 
   class Derived : CRTP<Derived> {};
 
-Below can be seen some common mistakes that will allow the breaking of the idiom.
+Below can be seen some common mistakes that will allow the breaking of the 
+idiom.
 
 If the constructor of a class intended to be used in a CRTP is public, then
 it allows users to construct that class on its own.
@@ -60,9 +62,10 @@ Example:
   class Bad : CRTP<Good> {};
   Bad BadInstance;
 
-To ensure that no accidental instantiation happens, the best practice is to make
-the constructor private and declare the derived class as friend. Note that as a tradeoff, 
-this also gives the derived class access to every other private members of the CRTP.
+To ensure that no accidental instantiation happens, the best practice is to 
+make the constructor private and declare the derived class as friend. Note
+that as a tradeoff, this also gives the derived class access to every other
+private members of the CRTP.
 
 Example:
 

>From 6a5f500c7e237ce03e1b9bebaf5d1ba761077884 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sat, 24 Feb 2024 03:39:04 +0100
Subject: [PATCH 27/31] comment

---
 .../clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 443a4bf0ec928a..2c02ddbb9c1ae4 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -125,7 +125,9 @@ void CrtpConstructorAccessibilityCheck::check(
            << Friend;
   };
 
-  auto WithFriendHintIfNeeded = [&](DiagnosticBuilder Diag, bool NeedsFriend) {
+  auto WithFriendHintIfNeeded =
+      [&](const DiagnosticBuilder &Diag,
+          bool NeedsFriend) -> const DiagnosticBuilder & {
     if (NeedsFriend)
       Diag << HintFriend;
 

>From eaa77d388a154fa19e264ed953bfdb5c00cd77bc Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Sat, 24 Feb 2024 04:00:53 +0100
Subject: [PATCH 28/31] docs

---
 .../bugprone/crtp-constructor-accessibility.rst     | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
index c527db1c0b8a66..afd88764b59673 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/crtp-constructor-accessibility.rst
@@ -83,3 +83,16 @@ Example:
   Bad CompileTimeError;
 
   CRTP<int> AlsoCompileTimeError;
+
+Limitations:
+
+* The check is not supported below C++11
+
+* The check does not handle when the derived class is passed as a variadic
+  template argument
+
+* Accessible functions that can construct the CRTP, like factory functions
+  are not checked
+
+The check also suggests a fix-its in some cases.
+

>From 0ceaa758b536e4e667a7e2be5b27eee39473b820 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Mon, 26 Feb 2024 23:55:19 +0100
Subject: [PATCH 29/31] comments

---
 .../CrtpConstructorAccessibilityCheck.cpp     | 30 ++++------
 .../crtp-constructor-accessibility.cpp        | 60 +++++++------------
 2 files changed, 30 insertions(+), 60 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 2c02ddbb9c1ae4..ac0f66deda8081 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -111,20 +111,11 @@ void CrtpConstructorAccessibilityCheck::check(
 
   if (hasPrivateConstructor(CRTPDeclaration) && NeedsFriend) {
     diag(CRTPDeclaration->getLocation(),
-         "the CRTP cannot be constructed from the derived class")
-        << CRTPDeclaration << HintFriend;
-    diag(CRTPDeclaration->getLocation(),
-         "consider declaring the derived class as friend", DiagnosticIDs::Note);
+         "the CRTP cannot be constructed from the derived class; consider "
+         "declaring the derived class as friend")
+        << HintFriend;
   }
 
-  auto DiagNoteFriendPrivate = [&](const SourceLocation &Loc, bool Friend) {
-    return diag(Loc,
-                "consider making it private%select{| and declaring the derived "
-                "class as friend}0",
-                DiagnosticIDs::Note)
-           << Friend;
-  };
-
   auto WithFriendHintIfNeeded =
       [&](const DiagnosticBuilder &Diag,
           bool NeedsFriend) -> const DiagnosticBuilder & {
@@ -140,8 +131,9 @@ void CrtpConstructorAccessibilityCheck::check(
     WithFriendHintIfNeeded(
         diag(CRTPDeclaration->getLocation(),
              "the implicit default constructor of the CRTP is publicly "
-             "accessible")
-            << CRTPDeclaration
+             "accessible; consider making it private%select{| and declaring "
+             "the derived class as friend}0")
+            << NeedsFriend
             << FixItHint::CreateInsertion(
                    CRTPDeclaration->getBraceRange().getBegin().getLocWithOffset(
                        1),
@@ -149,8 +141,6 @@ void CrtpConstructorAccessibilityCheck::check(
                        CRTPDeclaration->getNameAsString() + "() = default;\n" +
                        (IsStruct ? "public:\n" : "")),
         NeedsFriend);
-
-    DiagNoteFriendPrivate(CRTPDeclaration->getLocation(), NeedsFriend);
   }
 
   for (auto &&Ctor : CRTPDeclaration->ctors()) {
@@ -163,11 +153,11 @@ void CrtpConstructorAccessibilityCheck::check(
     WithFriendHintIfNeeded(
         diag(Ctor->getLocation(),
              "%0 contructor allows the CRTP to be %select{inherited "
-             "from|constructed}1 as a regular template class")
-            << Access << IsPublic << Ctor << hintMakeCtorPrivate(Ctor, Access),
+             "from|constructed}1 as a regular template class; consider making "
+             "it private%select{| and declaring the derived class as friend}2")
+            << Access << IsPublic << NeedsFriend
+            << hintMakeCtorPrivate(Ctor, Access),
         NeedsFriend);
-
-    DiagNoteFriendPrivate(Ctor->getLocation(), NeedsFriend);
   }
 }
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
index d2e300389bbe72..cb41923df157cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/crtp-constructor-accessibility.cpp
@@ -3,8 +3,7 @@
 namespace class_implicit_ctor {
 template <typename T>
 class CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private and declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: CRTP() = default;
 // CHECK-FIXES: friend T;
 
@@ -14,8 +13,7 @@ class A : CRTP<A> {};
 namespace class_unconstructible {
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class; consider declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: friend T;
     CRTP() = default;
 };
@@ -28,8 +26,7 @@ template <typename T>
 class CRTP {
 public:
     CRTP() = default;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
     // CHECK-FIXES: friend T;
 };
@@ -42,8 +39,7 @@ template <typename T>
 class CRTP {
 public:
     CRTP(int) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
     // CHECK-FIXES: friend T;
 };
@@ -56,12 +52,10 @@ template <typename T>
 class CRTP {
 public:
     CRTP(int) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}public:
     CRTP(float) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}public:
     
     // CHECK-FIXES: friend T;
@@ -76,16 +70,13 @@ template <typename T>
 class CRTP {
 protected:
     CRTP(int) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(int) {}{{[[:space:]]*}}protected:
     CRTP() = default;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}protected:
     CRTP(float) {}
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP(float) {}{{[[:space:]]*}}protected:
     
     // CHECK-FIXES: friend T;
@@ -99,8 +90,7 @@ class A : CRTP<A> {};
 namespace struct_implicit_ctor {
 template <typename T>
 struct CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private and declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
 // CHECK-FIXES: friend T;
 
@@ -111,8 +101,7 @@ namespace struct_default_ctor {
 template <typename T>
 struct CRTP {
     CRTP() = default;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class [bugprone-crtp-constructor-accessibility]
-    // CHECK-MESSAGES: :[[@LINE-2]]:5: note: consider making it private and declaring the derived class as friend
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
     // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
     // CHECK-FIXES: friend T;
 };
@@ -123,15 +112,13 @@ class A : CRTP<A> {};
 namespace same_class_multiple_crtps {
 template <typename T>
 struct CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private and declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP() = default;{{[[:space:]]*}}public:
 // CHECK-FIXES: friend T;
 
 template <typename T>
 struct CRTP2 {};
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:8: note: consider making it private and declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: private:{{[[:space:]]*}}CRTP2() = default;{{[[:space:]]*}}public:
 // CHECK-FIXES: friend T;
 
@@ -141,8 +128,7 @@ class A : CRTP<A>, CRTP2<A> {};
 namespace same_crtp_multiple_classes {
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class; consider declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: friend T;
     CRTP() = default;
 };
@@ -154,8 +140,7 @@ class B : CRTP<B> {};
 namespace crtp_template {
 template <typename T, typename U>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class; consider declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: friend U;
     CRTP() = default;
 };
@@ -166,8 +151,7 @@ class A : CRTP<int, A> {};
 namespace crtp_template2 {
 template <typename T, typename U>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class; consider declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: friend T;
     CRTP() = default;
 };
@@ -178,8 +162,7 @@ class A : CRTP<A, A> {};
 namespace template_derived {
 template <typename T>
 class CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private and declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: CRTP() = default;
 // CHECK-FIXES: friend T;
 
@@ -196,8 +179,7 @@ void foo() {
 namespace template_derived_explicit_specialization {
 template <typename T>
 class CRTP {};
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private and declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: CRTP() = default;
 // CHECK-FIXES: friend T;
 
@@ -225,8 +207,7 @@ class A;
 
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider declaring the derived class as friend
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the CRTP cannot be constructed from the derived class; consider declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: friend T;
     CRTP() = default;
     friend A;
@@ -241,8 +222,7 @@ class A;
 
 template <typename T>
 class CRTP {
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible [bugprone-crtp-constructor-accessibility]
-// CHECK-MESSAGES: :[[@LINE-2]]:7: note: consider making it private
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private [bugprone-crtp-constructor-accessibility]
 // CHECK-FIXES: CRTP() = default;
     friend A;
 };

>From 26d00551b0e6f1fd2f891e259a931496761e0fe8 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Tue, 27 Feb 2024 02:14:48 +0100
Subject: [PATCH 30/31] prevent crashes

---
 .../CrtpConstructorAccessibilityCheck.cpp     | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index ac0f66deda8081..e40998f0f91b8f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -24,8 +24,13 @@ static bool hasPrivateConstructor(const CXXRecordDecl *RD) {
 static bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
                                          const NamedDecl *Param) {
   return llvm::any_of(CRTP->friends(), [&](const FriendDecl *Friend) {
-    const auto *TTPT =
-        dyn_cast<TemplateTypeParmType>(Friend->getFriendType()->getType());
+    const TypeSourceInfo *const FriendType = Friend->getFriendType();
+    if (!FriendType) {
+      return false;
+    }
+
+    const auto *const TTPT =
+        dyn_cast<TemplateTypeParmType>(FriendType->getType());
 
     return TTPT && TTPT->getDecl() == Param;
   });
@@ -34,7 +39,12 @@ static bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
 static bool isDerivedClassBefriended(const CXXRecordDecl *CRTP,
                                      const CXXRecordDecl *Derived) {
   return llvm::any_of(CRTP->friends(), [&](const FriendDecl *Friend) {
-    return Friend->getFriendType()->getType()->getAsCXXRecordDecl() == Derived;
+    const TypeSourceInfo *const FriendType = Friend->getFriendType();
+    if (!FriendType) {
+      return false;
+    }
+
+    return FriendType->getType()->getAsCXXRecordDecl() == Derived;
   });
 }
 
@@ -95,6 +105,10 @@ void CrtpConstructorAccessibilityCheck::check(
   const CXXRecordDecl *CRTPDeclaration =
       CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
 
+  if (!CRTPDeclaration->hasDefinition()) {
+    return;
+  }
+
   const auto *DerivedTemplateParameter =
       getDerivedParameter(CRTPInstantiation, DerivedRecord);
 

>From 0049198e28e4b84b321665ccfba8da4e8d34b76c Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs at users.noreply.github.com>
Date: Mon, 4 Mar 2024 18:54:49 +0100
Subject: [PATCH 31/31] comment

---
 .../clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp  | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index e40998f0f91b8f..6175fcdfd229c3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -1,5 +1,4 @@
-//===--- CrtpConstructorAccessibilityCheck.cpp - clang-tidy
-//---------------------------------===//
+//===--- CrtpConstructorAccessibilityCheck.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.



More information about the cfe-commits mailing list