hokein created this revision.
hokein added reviewers: JonasToth, aaron.ballman.
Herald added a subscriber: xazax.hun.

The fix for aggregate initialization (`std::make_unique<Foo>(Foo {1, 2})` needs
to see Foo copy constructor, otherwise we will have a compiler error. So we
only emit the check warning.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54745

Files:
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  test/clang-tidy/modernize-make-unique.cpp


Index: test/clang-tidy/modernize-make-unique.cpp
===================================================================
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -32,6 +32,10 @@
 
 struct Empty {};
 
+struct NoCopyCtor {
+  NoCopyCtor(const NoCopyCtor&) = delete;
+};
+
 struct E {
   E(std::initializer_list<int>);
   E();
@@ -270,6 +274,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<Empty> PEmpty = 
std::make_unique<Empty>(Empty{});
 
+  // No fixes for classes with deleted copy constructor.
+  std::unique_ptr<NoCopyCtor> PNoCopyCtor = std::unique_ptr<NoCopyCtor>(new 
NoCopyCtor{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<NoCopyCtor> PNoCopyCtor = 
std::unique_ptr<NoCopyCtor>(new NoCopyCtor{});
+
   // Initialization with default constructor.
   std::unique_ptr<E> PE1 = std::unique_ptr<E>(new E{});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===================================================================
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -375,6 +375,17 @@
       //   smart_ptr<Pair>(new Pair{first, second});
       // Has to be replaced with:
       //   smart_ptr<Pair>(Pair{first, second});
+      //
+      // The fix (std::make_unique) requires to see the copy constructor of
+      // Pair, so we don't generate fix if the copy consturctor is not visible
+      // or deleted.
+      if (const auto *RD = New->getType()->getPointeeCXXRecordDecl()) {
+        for (const auto *Ctor : RD->ctors()) {
+          if (Ctor->isCopyConstructor() &&
+              (Ctor->isDeleted() || Ctor->getAccess() == AS_private))
+            return false;
+        }
+      }
       InitRange = SourceRange(
           New->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
           New->getInitializer()->getSourceRange().getEnd());


Index: test/clang-tidy/modernize-make-unique.cpp
===================================================================
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -32,6 +32,10 @@
 
 struct Empty {};
 
+struct NoCopyCtor {
+  NoCopyCtor(const NoCopyCtor&) = delete;
+};
+
 struct E {
   E(std::initializer_list<int>);
   E();
@@ -270,6 +274,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<Empty> PEmpty = std::make_unique<Empty>(Empty{});
 
+  // No fixes for classes with deleted copy constructor.
+  std::unique_ptr<NoCopyCtor> PNoCopyCtor = std::unique_ptr<NoCopyCtor>(new NoCopyCtor{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<NoCopyCtor> PNoCopyCtor = std::unique_ptr<NoCopyCtor>(new NoCopyCtor{});
+
   // Initialization with default constructor.
   std::unique_ptr<E> PE1 = std::unique_ptr<E>(new E{});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===================================================================
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -375,6 +375,17 @@
       //   smart_ptr<Pair>(new Pair{first, second});
       // Has to be replaced with:
       //   smart_ptr<Pair>(Pair{first, second});
+      //
+      // The fix (std::make_unique) requires to see the copy constructor of
+      // Pair, so we don't generate fix if the copy consturctor is not visible
+      // or deleted.
+      if (const auto *RD = New->getType()->getPointeeCXXRecordDecl()) {
+        for (const auto *Ctor : RD->ctors()) {
+          if (Ctor->isCopyConstructor() &&
+              (Ctor->isDeleted() || Ctor->getAccess() == AS_private))
+            return false;
+        }
+      }
       InitRange = SourceRange(
           New->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
           New->getInitializer()->getSourceRange().getEnd());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to