[clang-tidy] Add check bugprone-unique-ptr-array-mismatch.

[PiotrZSL]

LGTM, But:

• Align check description before committing.
• Consider adding some test with std::unique_ptr behind typedef.
• Consider adding test with unique_ptr depend on template argument but without specializations (like f_tmpl but without f5 function).

General issue that I got (no need to change that) with this test (and test for shared_ptr) is that actually there is no issue here.
For POD types it actually doesn't mater if you use delete or delete[]. This of course isn't portable and fall into implementation specific.
But can read about this in https://itanium-cxx-abi.github.io/cxx-abi/abi.html#array-cookies
This is why I would like to see NonPOD types instead of POD ones to be used with unique_ptr.
On GCC/Clang memory will be fully released (POD) types or corrupted (non POD types).

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

17–24

this should be same as in release notes ("Finds initializations of C++ unique pointers to non-array type that are initialized with an array.")

[balazske]

Fixed the documentation issue.
Added tests.

[balazske]

The case of fully dependent type (unique_ptr<T>) is not working in this checker and likely not in SharedPtrArrayMismatchCheck. I can fix this in a next patch (for both checkers).
Maybe we can remove the warning in all cases when the type is a POD type (or add a check option)?

[PiotrZSL]

In D151431#4376181, @balazske wrote:

Maybe we can remove the warning in all cases when the type is a POD type (or add a check option)?

Leave it... From a user perspective, it doesn't mater if type is POD or not.
I was mentioning non POD just because this is one of "undefined behavior" that sometimes is well defined.
But you don't know if project isn't using some custom new[]/delete[]

clang-tools-extra/docs/clang-tidy/checks/bugprone/unique-ptr-array-mismatch.rst

24

remove check name from comment to make it shorter

[balazske]

removed check name in documentation code comment

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

 //===--- BugproneTidyModule.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 "TerminatingContinueCheck.h"
 #include "ThrowKeywordMissingCheck.h"
 #include "TooSmallLoopVariableCheck.h"
 #include "UncheckedOptionalAccessCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
 #include "UnhandledExceptionAtNewCheck.h"
 #include "UnhandledSelfAssignmentCheck.h"
+#include "UniquePtrArrayMismatchCheck.h"
 #include "UnsafeFunctionsCheck.h"
 #include "UnusedRaiiCheck.h"
 #include "UnusedReturnValueCheck.h"
 #include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
 
 namespace clang::tidy {
 namespace bugprone {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
         "bugprone-undefined-memory-manipulation");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "bugprone-undelegated-constructor");
     CheckFactories.registerCheck<UnhandledSelfAssignmentCheck>(
         "bugprone-unhandled-self-assignment");
     CheckFactories.registerCheck<UnhandledExceptionAtNewCheck>(
         "bugprone-unhandled-exception-at-new");
+    CheckFactories.registerCheck<UniquePtrArrayMismatchCheck>(
+        "bugprone-unique-ptr-array-mismatch");
     CheckFactories.registerCheck<UnsafeFunctionsCheck>(
         "bugprone-unsafe-functions");
     CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii");
     CheckFactories.registerCheck<UnusedReturnValueCheck>(
         "bugprone-unused-return-value");
     CheckFactories.registerCheck<UseAfterMoveCheck>("bugprone-use-after-move");
     CheckFactories.registerCheck<VirtualNearMissCheck>(
         "bugprone-virtual-near-miss");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   TerminatingContinueCheck.cpp
   ThrowKeywordMissingCheck.cpp
   TooSmallLoopVariableCheck.cpp
   UncheckedOptionalAccessCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
   UnhandledExceptionAtNewCheck.cpp
   UnhandledSelfAssignmentCheck.cpp
+  UniquePtrArrayMismatchCheck.cpp
   UnsafeFunctionsCheck.cpp
   UnusedRaiiCheck.cpp
   UnusedReturnValueCheck.cpp
   UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
   clangTidy

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

+//===--- UniquePtrArrayMismatchCheck.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_UNIQUEPTRARRAYMISMATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNIQUEPTRARRAYMISMATCHCHECK_H
+
+#include "SmartPtrArrayMismatchCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds initializations of C++ unique pointers to non-array type that are
+/// initialized with an array.
+///
+/// Example:
+///
+/// \code
+///   std::unique_ptr<int> PtrArr{new int[10]};
+/// \endcode
+class UniquePtrArrayMismatchCheck : public SmartPtrArrayMismatchCheck {
+public:
+  UniquePtrArrayMismatchCheck(StringRef Name, ClangTidyContext *Context);
+
+protected:
+  SmartPtrClassMatcher getSmartPointerClassMatcher() const override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNIQUEPTRARRAYMISMATCHCHECK_H

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

+//===--- UniquePtrArrayMismatchCheck.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 "UniquePtrArrayMismatchCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+UniquePtrArrayMismatchCheck::UniquePtrArrayMismatchCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : SmartPtrArrayMismatchCheck(Name, Context, "unique") {}
+
+UniquePtrArrayMismatchCheck::SmartPtrClassMatcher
+UniquePtrArrayMismatchCheck::getSmartPointerClassMatcher() const {
+  auto DeleterDecl = classTemplateSpecializationDecl(
+      hasName("::std::default_delete"), templateArgumentCountIs(1),
+      hasTemplateArgument(0, templateArgument(refersToType(
+                                 qualType(equalsBoundNode(PointerTypeN))))));
+  return classTemplateSpecializationDecl(
+      hasName("::std::unique_ptr"), templateArgumentCountIs(2),
+      hasTemplateArgument(
+          0, templateArgument(refersToType(qualType().bind(PointerTypeN)))),
+      hasTemplateArgument(1, templateArgument(refersToType(
+                                 qualType(hasDeclaration(DeleterDecl))))));
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-multiple-new-in-one-expression
   memory by the first ``new`` may leak if the second allocation fails and throws exception.
 
 - New :doc:`bugprone-non-zero-enum-to-bool-conversion
   <clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion>` check.
 
   Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum`` type
   doesn't have a zero-value enumerator.
 
+- New :doc:`bugprone-unique-ptr-array-mismatch
+  <clang-tidy/checks/bugprone/unique-ptr-array-mismatch>` check.
+
+  Finds initializations of C++ unique pointers to non-array type that are
+  initialized with an array.
+
 - New :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` check.
 
   Checks for functions that have safer, more secure replacements available, or
   are considered deprecated due to design flaws.
   This check relies heavily on, but is not exclusive to, the functions from
   the *Annex K. "Bounds-checking interfaces"* of C11.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/unique-ptr-array-mismatch.rst

+.. title:: clang-tidy - bugprone-unique-ptr-array-mismatch
+
+bugprone-unique-ptr-array-mismatch
+==================================
+
+Finds initializations of C++ unique pointers to non-array type that are
+initialized with an array.
+
+If a pointer ``std::unique_ptr<T>`` is initialized with a new-expression
+``new T[]`` the memory is not deallocated correctly. A plain ``delete`` is used
+in this case to deallocate the target memory. Instead a ``delete[]`` call is
+needed. A ``std::unique_ptr<T[]>`` uses the correct delete operator. The check
+does not emit warning if an ``unique_ptr`` with user-specified deleter type is
+used.
+
+The check offers replacement of ``unique_ptr<T>`` to ``unique_ptr<T[]>`` if it
+is used at a single variable declaration (one variable in one statement).
+
+Example:
+
+.. code-block:: c++
+
+  std::unique_ptr<Foo> x(new Foo[10]); // -> std::unique_ptr<Foo[]> x(new Foo[10]);
+  //                     ^ warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  std::unique_ptr<Foo> x1(new Foo), x2(new Foo[10]); // no replacement
+  //                                   ^ warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+
+  D d;
+  std::unique_ptr<Foo, D> x3(new Foo[10], d); // no warning (custom deleter used)
+
+  struct S {
+    std::unique_ptr<Foo> x(new Foo[10]); // no replacement in this case
+    //                     ^ warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  };
+
+This check partially covers the CERT C++ Coding Standard rule
+`MEM51-CPP. Properly deallocate dynamically allocated resources
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM51-CPP.+Properly+deallocate+dynamically+allocated+resources>`_
+However, only the ``std::unique_ptr`` case is detected by this check.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-terminating-continue <bugprone/terminating-continue.html>`_, "Yes"
    `bugprone-throw-keyword-missing <bugprone/throw-keyword-missing.html>`_,
    `bugprone-too-small-loop-variable <bugprone/too-small-loop-variable.html>`_,
    `bugprone-unchecked-optional-access <bugprone/unchecked-optional-access.html>`_,
    `bugprone-undefined-memory-manipulation <bugprone/undefined-memory-manipulation.html>`_,
    `bugprone-undelegated-constructor <bugprone/undelegated-constructor.html>`_,
    `bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new.html>`_,
    `bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment.html>`_,
+   `bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch.html>`_, "Yes"
    `bugprone-unsafe-functions <bugprone/unsafe-functions.html>`_,
    `bugprone-unused-raii <bugprone/unused-raii.html>`_, "Yes"
    `bugprone-unused-return-value <bugprone/unused-return-value.html>`_,
    `bugprone-use-after-move <bugprone/use-after-move.html>`_,
    `bugprone-virtual-near-miss <bugprone/virtual-near-miss.html>`_, "Yes"
    `cert-dcl21-cpp <cert/dcl21-cpp.html>`_, "Yes"
    `cert-dcl50-cpp <cert/dcl50-cpp.html>`_,
    `cert-dcl58-cpp <cert/dcl58-cpp.html>`_,

clang-tools-extra/test/clang-tidy/checkers/bugprone/unique-ptr-array-mismatch.cpp

+// RUN: %check_clang_tidy %s bugprone-unique-ptr-array-mismatch %t
+
+namespace std {
+
+template<class T> struct default_delete {};
+template<class T> struct default_delete<T[]> {};
+
+template<class T, class Deleter = std::default_delete<T>>
+class unique_ptr {
+public:
+  explicit unique_ptr(T* p) noexcept;
+  unique_ptr(T* p, Deleter d1 ) noexcept;
+};
+
+template <class T, class Deleter>
+class unique_ptr<T[], Deleter> {
+public:
+  template<class U>
+  explicit unique_ptr(U p) noexcept;
+  template<class U>
+  unique_ptr(U p, Deleter d1) noexcept;
+};
+
+} // namespace std
+
+struct A {};
+
+using PtrT = std::unique_ptr<A>;
+using PtrTArr = std::unique_ptr<A[]>;
+
+void f1() {
+  std::unique_ptr<int> P1{new int};
+  std::unique_ptr<int> P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<int[]> P2{new int[10]};
+  // clang-format off
+  std::unique_ptr<  int  > P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<  int[]  > P3{new int[10]};
+  // clang-format on
+  std::unique_ptr<int> P4(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<int[]> P4(new int[10]);
+  new std::unique_ptr<int>(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  std::unique_ptr<int[]> P5(new int[10]);
+
+  A deleter;
+  std::unique_ptr<int, A> P6(new int[10], deleter);
+  std::unique_ptr<int, A> P7(new int[10]);
+  std::default_delete<int[]> def_del;
+  std::unique_ptr<int, std::default_delete<int[]>> P8(new int[10], def_del);
+
+  new PtrT(new A[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  new PtrTArr(new A[10]);
+}
+
+void f2() {
+  std::unique_ptr<A> P1(new A);
+  std::unique_ptr<A> P2(new A[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<A[]> P2(new A[10]);
+  std::unique_ptr<A[]> P3(new A[10]);
+}
+
+void f3() {
+  std::unique_ptr<int> P1{new int}, P2{new int[10]}, P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}
+
+struct S {
+  std::unique_ptr<int> P1;
+  std::unique_ptr<int> P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  std::unique_ptr<int> P3{new int}, P4{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  S() : P1{new int[10]} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+};
+
+void f_parm(std::unique_ptr<int>);
+
+void f4() {
+  f_parm(std::unique_ptr<int>{new int[10]});
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}
+
+std::unique_ptr<int> f_ret() {
+  return std::unique_ptr<int>(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}
+
+template <class T>
+void f_tmpl() {
+  std::unique_ptr<T> P1{new T[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<T[]> P1{new T[10]};
+}
+
+void f5() {
+  f_tmpl<char>();
+}
+
+template <class T>
+void f_tmpl_1() {
+  std::unique_ptr<T> P1{new T[10]};
+  // FIXME_CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // FIXME_CHECK-FIXES: std::unique_ptr<T[]> P1{new T[10]};
+}
+
+#define CHAR_PTR_TYPE std::unique_ptr<char>
+#define CHAR_PTR_VAR(X) \
+  X { new char[10] }
+#define CHAR_PTR_INIT(X, Y) \
+  std::unique_ptr<char> X { Y }
+
+void f6() {
+  CHAR_PTR_TYPE P1{new char[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  std::unique_ptr<char> CHAR_PTR_VAR(P2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<char[]> CHAR_PTR_VAR(P2);
+  CHAR_PTR_INIT(P3, new char[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}