Author: angelgarcia Date: Wed Oct 14 04:22:32 2015 New Revision: 250283 URL: http://llvm.org/viewvc/llvm-project?rev=250283&view=rev Log: Support every kind of initialization.
Summary: modernize-make-unique now correctly supports the different kinds of list initialization. Reviewers: klimek Subscribers: cfe-commits, alexfh Differential Revision: http://reviews.llvm.org/D13590 Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp?rev=250283&r1=250282&r2=250283&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp Wed Oct 14 04:22:32 2015 @@ -54,8 +54,11 @@ void MakeUniqueCheck::check(const MatchF SourceManager &SM = *Result.SourceManager; const auto *Construct = Result.Nodes.getNodeAs<CXXConstructExpr>(ConstructorCall); - const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression); const auto *Type = Result.Nodes.getNodeAs<QualType>(PointerType); + const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression); + + if (New->getNumPlacementArgs() != 0) + return; SourceLocation ConstructCallStart = Construct->getExprLoc(); @@ -86,6 +89,20 @@ void MakeUniqueCheck::check(const MatchF CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd), "std::make_unique"); + // If the unique_ptr is built with brace enclosed direct initialization, use + // parenthesis instead. + if (Construct->isListInitialization()) { + SourceRange BraceRange = Construct->getParenOrBraceRange(); + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange( + BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(1)), + "("); + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(BraceRange.getEnd(), + BraceRange.getEnd().getLocWithOffset(1)), + ")"); + } + SourceLocation NewStart = New->getSourceRange().getBegin(); SourceLocation NewEnd = New->getSourceRange().getEnd(); switch (New->getInitializationStyle()) { @@ -101,9 +118,30 @@ void MakeUniqueCheck::check(const MatchF break; } case CXXNewExpr::ListInit: { - SourceRange InitRange = New->getInitializer()->getSourceRange(); + // Range of the substring that we do not want to remove. + SourceRange InitRange; + if (const auto *NewConstruct = New->getConstructExpr()) { + // Direct initialization with initialization list. + // struct S { S(int x) {} }; + // std::unique_ptr<S>(new S{5}); + // The arguments in the initialization list are going to be forwarded to + // the constructor, so this has to be replaced with: + // struct S { S(int x) {} }; + // std::make_unique<S>(5); + InitRange = SourceRange( + NewConstruct->getParenOrBraceRange().getBegin().getLocWithOffset(1), + NewConstruct->getParenOrBraceRange().getEnd().getLocWithOffset(-1)); + } else { + // Aggregate initialization. + // std::unique_ptr<Pair>(new Pair{first, second}); + // Has to be replaced with: + // std::make_unique<Pair>(Pair{first, second}); + InitRange = SourceRange( + New->getAllocatedTypeSourceInfo()->getTypeLoc().getLocStart(), + New->getInitializer()->getSourceRange().getEnd()); + } Diag << FixItHint::CreateRemoval( - SourceRange(NewStart, InitRange.getBegin().getLocWithOffset(-1))); + CharSourceRange::getCharRange(NewStart, InitRange.getBegin())); Diag << FixItHint::CreateRemoval( SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd)); break; Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp?rev=250283&r1=250282&r2=250283&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Wed Oct 14 04:22:32 2015 @@ -34,12 +34,22 @@ struct Derived : public Base { Derived(int, int); }; -struct Pair { +struct APair { int a, b; }; +struct DPair { + DPair() : a(0), b(0) {} + DPair(int x, int y) : a(y), b(x) {} + int a, b; +}; + +struct Empty {}; + template<class T> using unique_ptr_ = std::unique_ptr<T>; +void *operator new(unsigned long Count, void *Ptr); + int g(std::unique_ptr<int> P); std::unique_ptr<Base> getPointer() { @@ -48,7 +58,7 @@ std::unique_ptr<Base> getPointer() { // CHECK-FIXES: return std::make_unique<Base>(); } -void f() { +void basic() { std::unique_ptr<int> P1 = std::unique_ptr<int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: std::unique_ptr<int> P1 = std::make_unique<int>(); @@ -78,16 +88,6 @@ void f() { // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead // CHECK-FIXES: int T = g(std::make_unique<int>()); - // Arguments are correctly handled. - std::unique_ptr<Base> Pbase = std::unique_ptr<Base>(new Base(5, T)); - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead - // CHECK-FIXES: std::unique_ptr<Base> Pbase = std::make_unique<Base>(5, T); - - // Works with init lists. - std::unique_ptr<Pair> Ppair = std::unique_ptr<Pair>(new Pair{T, 1}); - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead - // CHECK-FIXES: std::unique_ptr<Pair> Ppair = std::make_unique<Pair>({T, 1}); - // Only replace if the type in the template is the same than the type returned // by the new operator. auto Pderived = std::unique_ptr<Base>(new Derived()); @@ -95,7 +95,69 @@ void f() { // The pointer is returned by the function, nothing to do. std::unique_ptr<Base> RetPtr = getPointer(); - // Aliases. + // This emulates std::move. + std::unique_ptr<int> Move = static_cast<std::unique_ptr<int>&&>(P1); + + // Placemenet arguments should not be removed. + int *PInt = new int; + std::unique_ptr<int> Placement = std::unique_ptr<int>(new (PInt) int{3}); +} + +void initialization(int T, Base b) { + // Test different kinds of initialization of the pointee. + + // Direct initialization with parenthesis. + std::unique_ptr<DPair> PDir1 = std::unique_ptr<DPair>(new DPair(1, T)); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<DPair> PDir1 = std::make_unique<DPair>(1, T); + + // Direct initialization with braces. + std::unique_ptr<DPair> PDir2 = std::unique_ptr<DPair>(new DPair{2, T}); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<DPair> PDir2 = std::make_unique<DPair>(2, T); + + // Aggregate initialization. + std::unique_ptr<APair> PAggr = std::unique_ptr<APair>(new APair{T, 1}); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<APair> PAggr = std::make_unique<APair>(APair{T, 1}); + + + // Test different kinds of initialization of the pointee, when the unique_ptr + // is initialized with braces. + + // Direct initialization with parenthesis. + std::unique_ptr<DPair> PDir3 = std::unique_ptr<DPair>{new DPair(3, T)}; + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<DPair> PDir3 = std::make_unique<DPair>(3, T); + + // Direct initialization with braces. + std::unique_ptr<DPair> PDir4 = std::unique_ptr<DPair>{new DPair{4, T}}; + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<DPair> PDir4 = std::make_unique<DPair>(4, T); + + // Aggregate initialization. + std::unique_ptr<APair> PAggr2 = std::unique_ptr<APair>{new APair{T, 2}}; + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<APair> PAggr2 = std::make_unique<APair>(APair{T, 2}); + + + // Direct initialization with parenthesis, without arguments. + std::unique_ptr<DPair> PDir5 = std::unique_ptr<DPair>(new DPair()); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<DPair> PDir5 = std::make_unique<DPair>(); + + // Direct initialization with braces, without arguments. + std::unique_ptr<DPair> PDir6 = std::unique_ptr<DPair>(new DPair{}); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<DPair> PDir6 = std::make_unique<DPair>(); + + // Aggregate initialization without arguments. + std::unique_ptr<Empty> PEmpty = std::unique_ptr<Empty>(new Empty{}); + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr<Empty> PEmpty = std::make_unique<Empty>(Empty{}); +} + +void aliases() { typedef std::unique_ptr<int> IntPtr; IntPtr Typedef = IntPtr(new int); // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead @@ -110,11 +172,9 @@ void f() { std::unique_ptr<int> Using = unique_ptr_<int>(new int); // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr<int> Using = std::make_unique<int>(); +} - // This emulates std::move. - std::unique_ptr<int> Move = static_cast<std::unique_ptr<int>&&>(P1); - - // Adding whitespaces. +void whitespaces() { auto Space = std::unique_ptr <int>(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead // CHECK-FIXES: auto Space = std::make_unique<int>(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
