hokein updated this revision to Diff 174760.
hokein marked 2 inline comments as done.
hokein added a comment.
Address review comments, handle move constructor as well.
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,38 @@
struct Empty {};
+struct NoCopyMoveCtor {
+ NoCopyMoveCtor(const NoCopyMoveCtor &) = delete; // implies move ctor is
deleted
+};
+
+struct NoCopyMoveCtorVisible {
+private:
+ NoCopyMoveCtorVisible(const NoCopyMoveCtorVisible&) = default;
+ NoCopyMoveCtorVisible(NoCopyMoveCtorVisible&&) = default;
+};
+
+struct OnlyMoveCtor {
+ OnlyMoveCtor() = default;
+ OnlyMoveCtor(OnlyMoveCtor&&) = default;
+ OnlyMoveCtor(const OnlyMoveCtor &) = delete;
+};
+
+struct OnlyCopyCtor {
+ OnlyCopyCtor(const OnlyCopyCtor&) = default;
+ OnlyCopyCtor(OnlyCopyCtor&&) = delete;
+};
+
+struct OnlyCopyCtorVisible {
+ OnlyCopyCtorVisible(const OnlyCopyCtorVisible &) = default;
+
+private:
+ OnlyCopyCtorVisible(OnlyCopyCtorVisible &&) = default;
+};
+
+struct ImplicitDeletedCopyCtor {
+ const OnlyMoveCtor ctor;
+};
+
struct E {
E(std::initializer_list<int>);
E();
@@ -270,6 +302,33 @@
// 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&move constructors.
+ auto PNoCopyMoveCtor = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: use std::make_unique instead
+ // CHECK-FIXES: auto PNoCopyMoveCtor = std::unique_ptr<NoCopyMoveCtor>(new
NoCopyMoveCtor{});
+
+ auto PNoCopyMoveCtorVisible = std::unique_ptr<NoCopyMoveCtorVisible>(new
NoCopyMoveCtorVisible{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
+ // CHECK-FIXES: auto PNoCopyMoveCtorVisible =
std::unique_ptr<NoCopyMoveCtorVisible>(new NoCopyMoveCtorVisible{});
+
+ auto POnlyMoveCtor = std::unique_ptr<OnlyMoveCtor>(new OnlyMoveCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
+ // CHECK-FIXES: auto POnlyMoveCtor = std::unique_ptr<OnlyMoveCtor>(new
OnlyMoveCtor{});
+
+ // Fix for classes with classes with move constructor.
+ auto POnlyCopyCtor = std::unique_ptr<OnlyCopyCtor>(new OnlyCopyCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
+ // CHECK-FIXES: auto POnlyCopyCtor = std::unique_ptr<OnlyCopyCtor>(new
OnlyCopyCtor{});
+
+ // Fix for classes with classes with move constructor.
+ auto POnlyCopyCtorVisible = std::unique_ptr<OnlyCopyCtorVisible>(new
OnlyCopyCtorVisible{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: use std::make_unique instead
+ // CHECK-FIXES: auto POnlyCopyCtorVisible =
std::unique_ptr<OnlyCopyCtorVisible>(new OnlyCopyCtorVisible{});
+
+ auto PImplicitDeletedCopyCtor = std::unique_ptr<ImplicitDeletedCopyCtor>(new
ImplicitDeletedCopyCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead
+ // CHECK-FIXES: auto PImplicitDeletedCopyCtor =
std::unique_ptr<ImplicitDeletedCopyCtor>(new ImplicitDeletedCopyCtor{});
+
// 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,18 @@
// 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 copy/move constructor of
+ // Pair. If we found any invisible or deleted copy/move constructor, we
+ // stop generating fixes -- as the C++ rule is complicated and we are
less
+ // certain about the correct fixes.
+ if (const auto *RD = New->getType()->getPointeeCXXRecordDecl()) {
+ for (const auto *Ctor : RD->ctors()) {
+ if (Ctor->isCopyOrMoveConstructor() &&
+ (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,38 @@
struct Empty {};
+struct NoCopyMoveCtor {
+ NoCopyMoveCtor(const NoCopyMoveCtor &) = delete; // implies move ctor is deleted
+};
+
+struct NoCopyMoveCtorVisible {
+private:
+ NoCopyMoveCtorVisible(const NoCopyMoveCtorVisible&) = default;
+ NoCopyMoveCtorVisible(NoCopyMoveCtorVisible&&) = default;
+};
+
+struct OnlyMoveCtor {
+ OnlyMoveCtor() = default;
+ OnlyMoveCtor(OnlyMoveCtor&&) = default;
+ OnlyMoveCtor(const OnlyMoveCtor &) = delete;
+};
+
+struct OnlyCopyCtor {
+ OnlyCopyCtor(const OnlyCopyCtor&) = default;
+ OnlyCopyCtor(OnlyCopyCtor&&) = delete;
+};
+
+struct OnlyCopyCtorVisible {
+ OnlyCopyCtorVisible(const OnlyCopyCtorVisible &) = default;
+
+private:
+ OnlyCopyCtorVisible(OnlyCopyCtorVisible &&) = default;
+};
+
+struct ImplicitDeletedCopyCtor {
+ const OnlyMoveCtor ctor;
+};
+
struct E {
E(std::initializer_list<int>);
E();
@@ -270,6 +302,33 @@
// 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&move constructors.
+ auto PNoCopyMoveCtor = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: use std::make_unique instead
+ // CHECK-FIXES: auto PNoCopyMoveCtor = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{});
+
+ auto PNoCopyMoveCtorVisible = std::unique_ptr<NoCopyMoveCtorVisible>(new NoCopyMoveCtorVisible{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
+ // CHECK-FIXES: auto PNoCopyMoveCtorVisible = std::unique_ptr<NoCopyMoveCtorVisible>(new NoCopyMoveCtorVisible{});
+
+ auto POnlyMoveCtor = std::unique_ptr<OnlyMoveCtor>(new OnlyMoveCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
+ // CHECK-FIXES: auto POnlyMoveCtor = std::unique_ptr<OnlyMoveCtor>(new OnlyMoveCtor{});
+
+ // Fix for classes with classes with move constructor.
+ auto POnlyCopyCtor = std::unique_ptr<OnlyCopyCtor>(new OnlyCopyCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
+ // CHECK-FIXES: auto POnlyCopyCtor = std::unique_ptr<OnlyCopyCtor>(new OnlyCopyCtor{});
+
+ // Fix for classes with classes with move constructor.
+ auto POnlyCopyCtorVisible = std::unique_ptr<OnlyCopyCtorVisible>(new OnlyCopyCtorVisible{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: use std::make_unique instead
+ // CHECK-FIXES: auto POnlyCopyCtorVisible = std::unique_ptr<OnlyCopyCtorVisible>(new OnlyCopyCtorVisible{});
+
+ auto PImplicitDeletedCopyCtor = std::unique_ptr<ImplicitDeletedCopyCtor>(new ImplicitDeletedCopyCtor{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead
+ // CHECK-FIXES: auto PImplicitDeletedCopyCtor = std::unique_ptr<ImplicitDeletedCopyCtor>(new ImplicitDeletedCopyCtor{});
+
// 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,18 @@
// 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 copy/move constructor of
+ // Pair. If we found any invisible or deleted copy/move constructor, we
+ // stop generating fixes -- as the C++ rule is complicated and we are less
+ // certain about the correct fixes.
+ if (const auto *RD = New->getType()->getPointeeCXXRecordDecl()) {
+ for (const auto *Ctor : RD->ctors()) {
+ if (Ctor->isCopyOrMoveConstructor() &&
+ (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
