balazske created this revision.
Herald added subscribers: carlosgalvezp, steakhal, martong, gamesh411, 
Szelethus, dkrupp, xazax.hun, mgorny.
balazske requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117306

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-shared-ptr-array-mismatch.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-shared-ptr-array-mismatch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-shared-ptr-array-mismatch.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-shared-ptr-array-mismatch.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s misc-shared-ptr-array-mismatch %t
+
+namespace std {
+
+template <typename T>
+struct default_delete {};
+
+template <typename T>
+struct shared_ptr {
+  template <class Y>
+  explicit shared_ptr(Y *) {}
+  template <class Y, class Deleter>
+  shared_ptr(Y *, Deleter) {}
+};
+
+} // namespace std
+
+struct A {};
+
+void f1() {
+  std::shared_ptr<int> P1{new int};
+  std::shared_ptr<int> P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<int[]> P2{new int[10]};
+  std::shared_ptr<  int  > P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<  int[]  > P3{new int[10]};
+  std::shared_ptr<int> P4(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<int[]> P4(new int[10]);
+  new std::shared_ptr<int>(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  std::shared_ptr<int[]> P5(new int[10]);
+  std::shared_ptr<int> P6(new int[10], [](const int *Ptr) {});
+}
+
+void f2() {
+  std::shared_ptr<A> P1(new A);
+  std::shared_ptr<A> P2(new A[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<A[]> P2(new A[10]);
+  std::shared_ptr<A[]> P3(new A[10]);
+}
+
+void f3() {
+  std::shared_ptr<int> P1{new int}, P2{new int[10]}, P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+}
+
+struct S {
+  std::shared_ptr<int> P1;
+  std::shared_ptr<int> P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  std::shared_ptr<int> P3{new int}, P4{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  S() : P1{new int[10]} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+};
+
+void f_parm(std::shared_ptr<int>);
+
+void f4() {
+  f_parm(std::shared_ptr<int>{new int[10]});
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+}
+
+std::shared_ptr<int> f_ret() {
+  return std::shared_ptr<int>(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+}
+
+template <class T>
+void f_tmpl() {
+  std::shared_ptr<T> P1{new T[10]};
+}
+
+void f5() {
+  f_tmpl<char>();
+}
+
+#define CHAR_PTR_TYPE std::shared_ptr<char>
+#define CHAR_PTR_VAR(X) \
+  X { new char[10] }
+#define CHAR_PTR_INIT(X, Y) \
+  std::shared_ptr<char> X { Y }
+
+void f6() {
+  CHAR_PTR_TYPE P1{new char[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  std::shared_ptr<char> CHAR_PTR_VAR(P2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  // CHECK-FIXES: std::shared_ptr<char[]> CHAR_PTR_VAR(P2);
+  CHAR_PTR_INIT(P3, new char[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-shared-ptr-array-mismatch.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-shared-ptr-array-mismatch.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-shared-ptr-array-mismatch
+
+misc-shared-ptr-array-mismatch
+==============================
+
+Find construction of ``std::shared_ptr<T>`` where it is initialized with a new-expression ``new T[]``.
+The check offers replacement of ``shared_ptr<T>`` to ``shared_ptr<T[]>`` if it is used at a single variable declaration.
+
+Reason: A ``shared_ptr<T>`` uses plain ``delete`` to deallocate the target memory, this is incorrect if an array is to be deallocated.
+
+Example:
+
+.. code-block:: c++
+
+  std::shared_ptr<Foo> x(new Foo[10]); // -> std::shared_ptr<Foo[]> x(new Foo[10]);
+  //                     ^ warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  std::shared_ptr<Foo> x1(new Foo), x2(new Foo[10]); // no replacement 
+  //                                   ^ warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+
+  std::shared_ptr<Foo> x3(new Foo[10], [](const Foo *ptr) { delete[] ptr; }); // no warning
+
+  struct S {
+    std::shared_ptr<Foo> x(new Foo[10]); // no replacement in this case
+    //                     ^ warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch]
+  };
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -220,6 +220,7 @@
    `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
    `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `misc-redundant-expression <misc-redundant-expression.html>`_, "Yes"
+   `misc-shared-ptr-array-mismatch <misc-shared-ptr-array-mismatch.html>`_, "Yes"
    `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
    `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,10 @@
 
   Reports identifier with unicode right-to-left characters.
 
+- New :doc:`misc-shared-ptr-array-mismatch <clang-tidy/checks/misc-shared-ptr-array-mismatch>` check.
+
+  Reports if a C++ shared pointer is initialized with an array but declared with non-array type.
+
 - New :doc:`readability-container-data-pointer
   <clang-tidy/checks/readability-container-data-pointer>` check.
 
Index: clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.h
@@ -0,0 +1,42 @@
+//===--- SharedPtrArrayMismatchCheck.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_MISC_SHAREDPTRARRAYMISMATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHAREDPTRARRAYMISMATCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find `std::shared_ptr<T>(new T[...])`, replace it (in some cases) with
+/// `std::shared_ptr<T[]>(new T[...])`.
+///
+/// Example:
+///
+/// \code
+///   std::shared_ptr<int> PtrArr{new int[10]};
+/// \endcode
+class SharedPtrArrayMismatchCheck : public ClangTidyCheck {
+public:
+  SharedPtrArrayMismatchCheck(StringRef Name, ClangTidyContext *Context);
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHAREDPTRARRAYMISMATCHCHECK_H
Index: clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp
@@ -0,0 +1,120 @@
+//===--- SharedPtrArrayMismatchCheck.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 "SharedPtrArrayMismatchCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+SharedPtrArrayMismatchCheck::SharedPtrArrayMismatchCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
+void SharedPtrArrayMismatchCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {}
+
+void SharedPtrArrayMismatchCheck::registerMatchers(MatchFinder *Finder) {
+  auto UsedConstructor =
+      cxxConstructorDecl(
+          ofClass(classTemplateSpecializationDecl(
+              hasName("::std::shared_ptr"), templateArgumentCountIs(1),
+              hasTemplateArgument(
+                  0, templateArgument(refersToType(qualType().bind("T")))))),
+          parameterCountIs(1))
+          .bind("used_constructor");
+  auto ConstructExpr =
+      cxxConstructExpr(
+          hasDeclaration(UsedConstructor), argumentCountIs(1),
+          hasArgument(0, cxxNewExpr(isArray(), hasType(pointerType(pointee(
+                                                   equalsBoundNode("T")))))
+                             .bind("new_expr")))
+          .bind("construct_expr");
+  Finder->addMatcher(ConstructExpr, this);
+}
+
+namespace {
+
+bool isInSingleDeclStmt(const DeclaratorDecl *D) {
+  const DynTypedNodeList Parents =
+      D->getASTContext().getParentMapContext().getParents(*D);
+  for (const DynTypedNode &PNode : Parents)
+    if (const auto *PDecl = PNode.get<DeclStmt>())
+      return PDecl->isSingleDecl();
+  return false;
+}
+
+const DeclaratorDecl *getConstructedVarOrField(const Expr *FoundConstructExpr,
+                                               ASTContext &Ctx) {
+  const DynTypedNodeList ConstructParents =
+      Ctx.getParentMapContext().getParents(*FoundConstructExpr);
+  if (ConstructParents.size() != 1)
+    return nullptr;
+  const Decl *ParentDecl = ConstructParents.begin()->get<Decl>();
+  if (!ParentDecl)
+    return nullptr;
+  if (const auto *ParentVar = dyn_cast<VarDecl>(ParentDecl))
+    return ParentVar;
+  if (const auto *ParentField = dyn_cast<FieldDecl>(ParentDecl))
+    return ParentField;
+
+  return nullptr;
+}
+
+} // namespace
+
+void SharedPtrArrayMismatchCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *FoundNewExpr = Result.Nodes.getNodeAs<CXXNewExpr>("new_expr");
+  const auto *FoundConstructExpr =
+      Result.Nodes.getNodeAs<CXXConstructExpr>("construct_expr");
+  const auto *UsedConstructorDecl =
+      Result.Nodes.getNodeAs<CXXConstructorDecl>("used_constructor");
+
+  ASTContext &Ctx = UsedConstructorDecl->getASTContext();
+  const DeclaratorDecl *VarOrField =
+      getConstructedVarOrField(FoundConstructExpr, Ctx);
+
+  auto D = diag(FoundNewExpr->getBeginLoc(),
+                "shared pointer to non-array is initialized with array");
+  D << FoundNewExpr->getSourceRange();
+
+  if (VarOrField) {
+    auto TSTypeLoc = VarOrField->getTypeSourceInfo()
+                         ->getTypeLoc()
+                         .getAsAdjusted<clang::TemplateSpecializationTypeLoc>();
+    assert(TSTypeLoc.getNumArgs() == 1 &&
+           "Matched type should have 1 template argument.");
+
+    SourceRange TemplateArgumentRange = TSTypeLoc.getArgLoc(0)
+                                            .getTypeSourceInfo()
+                                            ->getTypeLoc()
+                                            .getLocalSourceRange();
+    D << TemplateArgumentRange;
+
+    if (isInSingleDeclStmt(VarOrField)) {
+      const SourceManager &SM = Ctx.getSourceManager();
+      if (!utils::rangeCanBeFixed(TemplateArgumentRange, &SM))
+        return;
+
+      SourceLocation InsertLoc = Lexer::getLocForEndOfToken(
+          TemplateArgumentRange.getEnd(), 0, SM, Ctx.getLangOpts());
+      D << FixItHint::CreateInsertion(InsertLoc, "[]");
+    }
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -17,6 +17,7 @@
 #include "NonCopyableObjects.h"
 #include "NonPrivateMemberVariablesInClassesCheck.h"
 #include "RedundantExpressionCheck.h"
+#include "SharedPtrArrayMismatchCheck.h"
 #include "StaticAssertCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
@@ -46,6 +47,8 @@
         "misc-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<RedundantExpressionCheck>(
         "misc-redundant-expression");
+    CheckFactories.registerCheck<SharedPtrArrayMismatchCheck>(
+        "misc-shared-ptr-array-mismatch");
     CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
     CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
         "misc-throw-by-value-catch-by-reference");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -13,6 +13,7 @@
   NonCopyableObjects.cpp
   NonPrivateMemberVariablesInClassesCheck.cpp
   RedundantExpressionCheck.cpp
+  SharedPtrArrayMismatchCheck.cpp
   StaticAssertCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to