hokein updated this revision to Diff 202940. hokein marked 2 inline comments as done. hokein added a comment.
address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 Files: clang-tools-extra/test/clang-tidy/modernize-make-unique-cxx14.cpp clang-tools-extra/test/clang-tidy/modernize-make-unique-inaccessible-ctors.cpp clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp +++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp @@ -1,5 +1,4 @@ -// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr -// FIXME: Fix the test code in C++2a mode. +// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr #include "unique_ptr.h" #include "initializer_list.h" @@ -32,37 +31,6 @@ 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>); @@ -314,33 +282,6 @@ // 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-tools-extra/test/clang-tidy/modernize-make-unique-inaccessible-ctors.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/modernize-make-unique-inaccessible-ctors.cpp @@ -0,0 +1,113 @@ +// RUN: %check_clang_tidy -std=c++14,c++17 -check-suffix=CXX-14-17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr -D CXX_14_17=1 +// RUN: %check_clang_tidy -std=c++2a -check-suffix=CXX-2A %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr -D CXX_2A=1 + +#include "unique_ptr.h" +// CHECK-FIXES: #include <memory> + +struct NoCopyMoveCtor { +#ifdef CXX_2A + // C++2a requires to see the default constructor, otherwise it is illgal. + NoCopyMoveCtor() = default; +#endif +#ifdef CXX_14_17 + int a, b; +#endif + NoCopyMoveCtor(const NoCopyMoveCtor &) = delete; // implies move ctor is deleted +}; + +struct NoCopyMoveCtorVisible { +#ifdef CXX_2A + NoCopyMoveCtorVisible() = default; +#endif +private: + NoCopyMoveCtorVisible(const NoCopyMoveCtorVisible&) = default; + NoCopyMoveCtorVisible(NoCopyMoveCtorVisible&&) = default; +}; + +struct OnlyMoveCtor { + OnlyMoveCtor() = default; + OnlyMoveCtor(OnlyMoveCtor&&) = default; + OnlyMoveCtor(const OnlyMoveCtor &) = delete; +}; + +struct OnlyCopyCtor { +#ifdef CXX_2A + OnlyCopyCtor() = default; +#endif + OnlyCopyCtor(const OnlyCopyCtor&) = default; + OnlyCopyCtor(OnlyCopyCtor&&) = delete; +}; + +struct OnlyCopyCtorVisible { +#ifdef CXX_2A + OnlyCopyCtorVisible() = default; +#endif + OnlyCopyCtorVisible(const OnlyCopyCtorVisible &) = default; + +private: + OnlyCopyCtorVisible(OnlyCopyCtorVisible &&) = default; +}; + +struct ImplicitDeletedCopyCtor { + const OnlyMoveCtor ctor; +}; + +void f() { + auto my_ptr = std::unique_ptr<int>(new int(1)); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:17: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto my_ptr = std::make_unique<int>(1); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:17: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto my_ptr = std::make_unique<int>(1); + + // "new NoCopyMoveCtor{}" is processed differently in C++14/17 and C++2a: + // * In C++14/17, it is recognized as aggregate initialization, + // no fixes will be generated although the generated fix is compilable. + // * In C++2a, it is is recognized as default constructor initialization ( + // similar to "new NoCopyMoveCtor()"), the check will emit the fix and the + // fix is correct. + auto PNoCopyMoveCtor = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:26: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto PNoCopyMoveCtor = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{}); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:26: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto PNoCopyMoveCtor = std::make_unique<NoCopyMoveCtor>(); + + auto PNoCopyMoveCtorVisible = std::unique_ptr<NoCopyMoveCtorVisible>(new NoCopyMoveCtorVisible{}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:33: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto PNoCopyMoveCtorVisible = std::unique_ptr<NoCopyMoveCtorVisible>(new NoCopyMoveCtorVisible{}); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:33: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto PNoCopyMoveCtorVisible = std::make_unique<NoCopyMoveCtorVisible>(); + + auto POnlyMoveCtor = std::unique_ptr<OnlyMoveCtor>(new OnlyMoveCtor{}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:24: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto POnlyMoveCtor = std::unique_ptr<OnlyMoveCtor>(new OnlyMoveCtor{}); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:24: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto POnlyMoveCtor = std::make_unique<OnlyMoveCtor>(); + + auto POnlyCopyCtor = std::unique_ptr<OnlyCopyCtor>(new OnlyCopyCtor{}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:24: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto POnlyCopyCtor = std::unique_ptr<OnlyCopyCtor>(new OnlyCopyCtor{}); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:24: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto POnlyCopyCtor = std::make_unique<OnlyCopyCtor>(); + + auto POnlyCopyCtorVisible = std::unique_ptr<OnlyCopyCtorVisible>(new OnlyCopyCtorVisible{}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:31: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto POnlyCopyCtorVisible = std::unique_ptr<OnlyCopyCtorVisible>(new OnlyCopyCtorVisible{}); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:31: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto POnlyCopyCtorVisible = std::make_unique<OnlyCopyCtorVisible>(); + + // This is aggregate initialization in C++2a, no fix will be generated. + auto PImplicitDeletedCopyCtor = std::unique_ptr<ImplicitDeletedCopyCtor>(new ImplicitDeletedCopyCtor{}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:35: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto PImplicitDeletedCopyCtor = std::unique_ptr<ImplicitDeletedCopyCtor>(new ImplicitDeletedCopyCtor{}); + // CHECK-MESSAGES-CXX-2A: :[[@LINE-3]]:35: warning: use std::make_unique instead + // CHECK-FIXES-CXX-2A: auto PImplicitDeletedCopyCtor = std::unique_ptr<ImplicitDeletedCopyCtor>(new ImplicitDeletedCopyCtor{}); + + +#ifdef CXX_14_17 + // FIXME: it is impossible to use make_unique for this case, the check should + // stop emitting the message. + auto PNoCopyMoveCtor2 = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{1, 2}); + // CHECK-MESSAGES-CXX-14-17: :[[@LINE-1]]:27: warning: use std::make_unique instead + // CHECK-FIXES-CXX-14-17: auto PNoCopyMoveCtor2 = std::unique_ptr<NoCopyMoveCtor>(new NoCopyMoveCtor{1, 2}); +#endif +} Index: clang-tools-extra/test/clang-tidy/modernize-make-unique-cxx14.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/modernize-make-unique-cxx14.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr - -#include "unique_ptr.h" -// CHECK-FIXES: #include <memory> - -void f() { - auto my_ptr = std::unique_ptr<int>(new int(1)); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead - // CHECK-FIXES: auto my_ptr = std::make_unique<int>(1); -}
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits