[clang-tools-extra] [clang-tidy] Unsafe CRTP check (PR #82403)

[via cfe-commits]

================
@@ -0,0 +1,171 @@
+//===--- 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 "../utils/LexerUtils.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;
+      });
+}
+
+bool hasPrivateConstructor(const CXXRecordDecl *RD) {
+  for (auto &&Ctor : RD->ctors()) {
+    if (Ctor->getAccess() == AS_private)
+      return true;
+  }
+
+  return false;
+}
+
+bool isDerivedParameterBefriended(const CXXRecordDecl *CRTP,
+                                  const NamedDecl *Param) {
+  for (auto &&Friend : CRTP->friends()) {
+    const auto *TTPT =
+        dyn_cast<TemplateTypeParmType>(Friend->getFriendType()->getType());
+
+    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;
+  }
+
+  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);
+}
+
+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(
+      Ctor->getBeginLoc().getLocWithOffset(-1), "private:\n"));
+
+  const SourceLocation CtorEndLoc =
+      Ctor->isExplicitlyDefaulted()
+          ? utils::lexer::findNextTerminator(Ctor->getEndLoc(), SM, LangOpts)
+          : Ctor->getEndLoc();
+  Hints.emplace_back(FixItHint::CreateInsertion(
+      CtorEndLoc.getLocWithOffset(1), '\n' + OriginalAccess + ':' + '\n'));
+
+  return Hints;
+}
+} // namespace
+
+void UnsafeCrtpCheck::registerMatchers(MatchFinder *Finder) {
+  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 *CRTPInstantiation =
+      Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("crtp");
+  const auto *DerivedRecord = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
+  const CXXRecordDecl *CRTPDeclaration =
+      CRTPInstantiation->getSpecializedTemplate()->getTemplatedDecl();
+
+  if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
----------------
whisperity wrote:

(Following from 💬...)

How possible it is that these cases appear in conjunction with one another? Based on style, it would look like the returns are missing from the end of the statements.

Perhaps you could create helper functions that create the diagnostics for each individual case with a telling name, and then the `check()` function can be simplified to `if (condition) diagnoseCaseX(); if (condition2) diagnoseCaseY();` etc.

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