From d57be707b88beb31bcf75b51ca6c381b17dd8249 Mon Sep 17 00:00:00 2001
From: Piotr Padlewski <piotr.padlewski@gmail.com>
Date: Wed, 31 Aug 2016 00:06:55 +0000
Subject: [PATCH] [clang-tidy] modernize-make-{smart_ptr} private ctor bugfix

Summary:
Bugfix for 27321. When the constructor of stored pointer
type is private then it is invalid to change it to
make_shared or make_unique.

Reviewers: alexfh, aaron.ballman, hokein

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D23343

llvm-svn: 280180
---
 .../modernize/MakeSmartPtrCheck.cpp           |  8 ++++-
 clang-tools-extra/docs/ReleaseNotes.rst       |  8 +++++
 .../test/clang-tidy/modernize-make-shared.cpp | 32 +++++++++++++++++++
 .../test/clang-tidy/modernize-make-unique.cpp | 32 +++++++++++++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
index 9b1ea3c28f71..80dd10358ad2 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -29,13 +29,19 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return;
 
+  // Calling make_smart_ptr from within a member function of a type with a
+  // private or protected constructor would be ill-formed.
+  auto CanCallCtor = unless(has(ignoringImpCasts(cxxConstructExpr(
+      hasDeclaration(decl(unless(isPublic())))))));
+
   Finder->addMatcher(
       cxxBindTemporaryExpr(has(ignoringParenImpCasts(
           cxxConstructExpr(
               hasType(getSmartPointerTypeMatcher()), argumentCountIs(1),
               hasArgument(0,
                           cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType(
-                                         equalsBoundNode(PointerType))))))
+                                         equalsBoundNode(PointerType))))),
+                                     CanCallCtor)
                               .bind(NewExpression)))
               .bind(ConstructorCall)))),
       this);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d218ac956151..658a8fce3173 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,14 @@ Improvements to clang-tidy
   Flags function parameters of a pointer type that could be changed to point to
   a constant type instead.
 
+Fixed bugs:
+- `modernize-make-unique
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-unique.html>`_
+  and `modernize-make-shared
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-shared.html>`_
+  Calling ``make_{unique|shared}`` from within a member function of a type
+  with a private or protected constructor would be ill-formed.
+
 Improvements to include-fixer
 -----------------------------
 
diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
index ee4c68695fa8..188ef5ee196e 100644
--- a/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
+++ b/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp
@@ -100,6 +100,38 @@ void basic() {
   std::shared_ptr<int> Placement = std::shared_ptr<int>(new (PInt) int{3});
 }
 
+// Calling make_smart_ptr from within a member function of a type with a
+// private or protected constructor would be ill-formed.
+class Private {
+private:
+  Private(int z) {}
+
+public:
+  Private() {}
+  void create() {
+    auto callsPublic = std::shared_ptr<Private>(new Private);
+    // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_shared instead
+    // CHECK-FIXES: auto callsPublic = std::make_shared<Private>();
+    auto ptr = std::shared_ptr<Private>(new Private(42));
+  }
+
+  virtual ~Private();
+};
+
+class Protected {
+protected:
+  Protected() {}
+
+public:
+  Protected(int, int) {}
+  void create() {
+    auto callsPublic = std::shared_ptr<Protected>(new Protected(1, 2));
+    // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_shared instead
+    // CHECK-FIXES: auto callsPublic = std::make_shared<Protected>(1, 2);
+    auto ptr = std::shared_ptr<Protected>(new Protected);
+  }
+};
+
 void initialization(int T, Base b) {
   // Test different kinds of initialization of the pointee.
 
diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
index 842955486a1e..31157347736c 100644
--- a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -103,6 +103,38 @@ void basic() {
   std::unique_ptr<int> Placement = std::unique_ptr<int>(new (PInt) int{3});
 }
 
+// Calling make_smart_ptr from within a member function of a type with a
+// private or protected constructor would be ill-formed.
+class Private {
+private:
+  Private(int z) {}
+
+public:
+  Private() {}
+  void create() {
+    auto callsPublic = std::unique_ptr<Private>(new Private);
+    // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
+    // CHECK-FIXES: auto callsPublic = std::make_unique<Private>();
+    auto ptr = std::unique_ptr<Private>(new Private(42));
+  }
+
+  virtual ~Private();
+};
+
+class Protected {
+protected:
+  Protected() {}
+
+public:
+  Protected(int, int) {}
+  void create() {
+    auto callsPublic = std::unique_ptr<Protected>(new Protected(1, 2));
+    // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
+    // CHECK-FIXES: auto callsPublic = std::make_unique<Protected>(1, 2);
+    auto ptr = std::unique_ptr<Protected>(new Protected);
+  }
+};
+
 void initialization(int T, Base b) {
   // Test different kinds of initialization of the pointee.
 
-- 
2.34.1