Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h +++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h @@ -39,10 +39,17 @@ static const char PointerType[]; static const char ConstructorCall[]; + static const char ResetCall[]; static const char NewExpression[]; private: std::string makeSmartPtrFunctionName; + + void checkConstruct(SourceManager &SM, const CXXConstructExpr *Construct, + const QualType *Type, const CXXNewExpr *New); + void checkReset(SourceManager &SM, const CXXMemberCallExpr *Member, + const CXXNewExpr *New); + void replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New); }; } // namespace modernize Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -18,6 +18,7 @@ const char MakeSmartPtrCheck::PointerType[] = "pointerType"; const char MakeSmartPtrCheck::ConstructorCall[] = "constructorCall"; +const char MakeSmartPtrCheck::ResetCall[] = "resetCall"; const char MakeSmartPtrCheck::NewExpression[] = "newExpression"; MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, @@ -31,8 +32,8 @@ // 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()))))))); + auto CanCallCtor = unless(has(ignoringImpCasts( + cxxConstructExpr(hasDeclaration(decl(unless(isPublic()))))))); Finder->addMatcher( cxxBindTemporaryExpr(has(ignoringParenImpCasts( @@ -45,6 +46,14 @@ .bind(NewExpression))) .bind(ConstructorCall)))), this); + + Finder->addMatcher( + cxxMemberCallExpr( + thisPointerType(getSmartPointerTypeMatcher()), + callee(cxxMethodDecl(hasName("reset"))), + hasArgument(0, cxxNewExpr(CanCallCtor).bind(NewExpression))) + .bind(ResetCall), + this); } void MakeSmartPtrCheck::check(const MatchFinder::MatchResult &Result) { @@ -55,12 +64,23 @@ SourceManager &SM = *Result.SourceManager; const auto *Construct = Result.Nodes.getNodeAs(ConstructorCall); + const auto *Reset = Result.Nodes.getNodeAs(ResetCall); const auto *Type = Result.Nodes.getNodeAs(PointerType); const auto *New = Result.Nodes.getNodeAs(NewExpression); if (New->getNumPlacementArgs() != 0) return; + if (Construct) + checkConstruct(SM, Construct, Type, New); + else if (Reset) + checkReset(SM, Reset, New); +} + +void MakeSmartPtrCheck::checkConstruct(SourceManager &SM, + const CXXConstructExpr *Construct, + const QualType *Type, + const CXXNewExpr *New) { SourceLocation ConstructCallStart = Construct->getExprLoc(); bool Invalid = false; @@ -105,6 +125,36 @@ ")"); } + replaceNew(Diag, New); +} + +void MakeSmartPtrCheck::checkReset(SourceManager &SM, + const CXXMemberCallExpr *Reset, + const CXXNewExpr *New) { + const auto *Expr = cast(Reset->getCallee()); + SourceLocation OperatorLoc = Expr->getOperatorLoc(); + SourceLocation ResetCallStart = Reset->getExprLoc(); + SourceLocation ExprStart = Expr->getLocStart(); + SourceLocation ExprEnd = + Lexer::getLocForEndOfToken(Expr->getLocEnd(), 0, SM, getLangOpts()); + + auto Diag = diag(ResetCallStart, "use %0 instead") + << makeSmartPtrFunctionName; + + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(OperatorLoc, ExprEnd), + (llvm::Twine(" = ") + makeSmartPtrFunctionName + "<" + + New->getAllocatedType().getAsString(getLangOpts()) + ">") + .str()); + + if (Expr->isArrow()) + Diag << FixItHint::CreateInsertion(ExprStart, "*"); + + replaceNew(Diag, New); +} + +void MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag, + const CXXNewExpr *New) { SourceLocation NewStart = New->getSourceRange().getBegin(); SourceLocation NewEnd = New->getSourceRange().getEnd(); switch (New->getInitializationStyle()) { Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -81,6 +81,12 @@ Warns if an object is used after it has been moved, without an intervening reinitialization. +- `modernize-make-unique + `_ + and `modernize-make-shared + `_ + now handle calls to the smart pointer's reset method. + - The `modernize-use-auto `_ check now warns about variable declarations that are initialized with a cast. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-shared.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-shared.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-shared.rst @@ -14,3 +14,14 @@ // becomes auto my_ptr = std::make_shared(1, 2); + +This check also finds calls to ``std::shared_ptr::reset()`` with a ``new`` +expression, and replaces it with a call to ``std::make_shared``. + +.. code-block:: c++ + + my_ptr.reset(new MyPair(1, 2)); + + // becomes + + my_ptr = std::make_shared(1, 2); Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst @@ -14,3 +14,14 @@ // becomes auto my_ptr = std::make_unique(1, 2); + +This check also finds calls to ``std::unique_ptr::reset()`` with a ``new`` +expression, and replaces it with a call to ``std::make_unique``. + +.. code-block:: c++ + + my_ptr.reset(new MyPair(1, 2)); + + // becomes + + my_ptr = std::make_unique(1, 2); Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-shared.cpp @@ -5,6 +5,7 @@ template class shared_ptr { public: + shared_ptr(); shared_ptr(type *ptr); shared_ptr(const shared_ptr &t) {} shared_ptr(shared_ptr &&t) {} @@ -14,6 +15,9 @@ type *release(); void reset(); void reset(type *pt); + shared_ptr &operator=(shared_ptr &&); + template + shared_ptr &operator=(shared_ptr &&); private: type *ptr; @@ -60,11 +64,27 @@ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: std::shared_ptr P1 = std::make_shared(); + P1.reset(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: P1 = std::make_shared(); + + P1 = std::shared_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: P1 = std::make_shared(); + // Without parenthesis. std::shared_ptr P2 = std::shared_ptr(new int); // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared] // CHECK-FIXES: std::shared_ptr P2 = std::make_shared(); + P2.reset(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: P2 = std::make_shared(); + + P2 = std::shared_ptr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: P2 = std::make_shared(); + // With auto. auto P3 = std::shared_ptr(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead @@ -76,6 +96,10 @@ shared_ptr Q = shared_ptr(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_shared instead // CHECK-FIXES: shared_ptr Q = std::make_shared(); + + Q = shared_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead + // CHECK-FIXES: Q = std::make_shared(); } std::shared_ptr R(new int()); @@ -85,19 +109,36 @@ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead // CHECK-FIXES: int T = g(std::make_shared()); - // Only replace if the type in the template is the same than the type returned + // Only replace if the type in the template is the same as the type returned // by the new operator. auto Pderived = std::shared_ptr(new Derived()); + // OK to replace for reset and assign + Pderived.reset(new Derived()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_shared instead + // CHECK-FIXES: Pderived = std::make_shared(); + + Pderived = std::shared_ptr(new Derived()); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use std::make_shared instead + // CHECK-FIXES: Pderived = std::make_shared(); + + // FIXME: OK to replace if assigned to shared_ptr + Pderived = std::shared_ptr(new Derived()); + + // FIXME: OK to replace when auto is not used + std::shared_ptr PBase = std::shared_ptr(new Derived()); + // The pointer is returned by the function, nothing to do. std::shared_ptr RetPtr = getPointer(); // This emulates std::move. std::shared_ptr Move = static_cast &&>(P1); - // Placemenet arguments should not be removed. + // Placement arguments should not be removed. int *PInt = new int; std::shared_ptr Placement = std::shared_ptr(new (PInt) int{3}); + Placement.reset(new (PInt) int{3}); + Placement = std::shared_ptr(new (PInt) int{3}); } // Calling make_smart_ptr from within a member function of a type with a @@ -113,6 +154,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_shared instead // CHECK-FIXES: auto callsPublic = std::make_shared(); auto ptr = std::shared_ptr(new Private(42)); + ptr.reset(new Private(42)); + ptr = std::shared_ptr(new Private(42)); } virtual ~Private(); @@ -129,6 +172,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_shared instead // CHECK-FIXES: auto callsPublic = std::make_shared(1, 2); auto ptr = std::shared_ptr(new Protected); + ptr.reset(new Protected); + ptr = std::shared_ptr(new Protected); } }; @@ -139,16 +184,25 @@ std::shared_ptr PDir1 = std::shared_ptr(new DPair(1, T)); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead // CHECK-FIXES: std::shared_ptr PDir1 = std::make_shared(1, T); + PDir1.reset(new DPair(1, T)); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead + // CHECK-FIXES: PDir1 = std::make_shared(1, T); // Direct initialization with braces. std::shared_ptr PDir2 = std::shared_ptr(new DPair{2, T}); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead // CHECK-FIXES: std::shared_ptr PDir2 = std::make_shared(2, T); + PDir2.reset(new DPair{2, T}); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead + // CHECK-FIXES: PDir2 = std::make_shared(2, T); // Aggregate initialization. std::shared_ptr PAggr = std::shared_ptr(new APair{T, 1}); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead // CHECK-FIXES: std::shared_ptr PAggr = std::make_shared(APair{T, 1}); + PAggr.reset(new APair{T, 1}); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead + // CHECK-FIXES: std::make_shared(APair{T, 1}); // Test different kinds of initialization of the pointee, when the shared_ptr // is initialized with braces. @@ -229,4 +283,24 @@ auto Nest = std::shared_ptr>(new std::shared_ptr(new int)); // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use std::make_shared instead // CHECK-FIXES: auto Nest = std::make_shared>(new int); + Nest.reset(new std::shared_ptr(new int)); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead + // CHECK-FIXES: Nest = std::make_shared>(new int); + Nest->reset(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead + // CHECK-FIXES: *Nest = std::make_shared(); +} + +void reset() { + std::shared_ptr P; + P.reset(); + P.reset(nullptr); + P.reset(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_shared instead + // CHECK-FIXES: P = std::make_shared(); + + auto Q = &P; + Q->reset(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead + // CHECK-FIXES: *Q = std::make_shared(); } Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp @@ -8,6 +8,7 @@ template > class unique_ptr { public: + unique_ptr(); unique_ptr(type *ptr); unique_ptr(const unique_ptr &t) = delete; unique_ptr(unique_ptr &&t); @@ -17,11 +18,13 @@ type *release(); void reset(); void reset(type *pt); + unique_ptr &operator=(unique_ptr &&); + template + unique_ptr &operator=(unique_ptr &&); private: type *ptr; }; - } struct Base { @@ -46,7 +49,8 @@ struct Empty {}; -template using unique_ptr_ = std::unique_ptr; +template +using unique_ptr_ = std::unique_ptr; void *operator new(__SIZE_TYPE__ Count, void *Ptr); @@ -63,11 +67,27 @@ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: std::unique_ptr P1 = std::make_unique(); + P1.reset(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: P1 = std::make_unique(); + + P1 = std::unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: P1 = std::make_unique(); + // Without parenthesis. std::unique_ptr P2 = std::unique_ptr(new int); // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] // CHECK-FIXES: std::unique_ptr P2 = std::make_unique(); + P2.reset(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: P2 = std::make_unique(); + + P2 = std::unique_ptr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: P2 = std::make_unique(); + // With auto. auto P3 = std::unique_ptr(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead @@ -79,6 +99,10 @@ unique_ptr Q = unique_ptr(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead // CHECK-FIXES: unique_ptr Q = std::make_unique(); + + Q = unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead + // CHECK-FIXES: Q = std::make_unique(); } std::unique_ptr R(new int()); @@ -88,19 +112,36 @@ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead // CHECK-FIXES: int T = g(std::make_unique()); - // Only replace if the type in the template is the same than the type returned + // Only replace if the type in the template is the same as the type returned // by the new operator. auto Pderived = std::unique_ptr(new Derived()); + // OK to replace for reset and assign + Pderived.reset(new Derived()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_unique instead + // CHECK-FIXES: Pderived = std::make_unique(); + + Pderived = std::unique_ptr(new Derived()); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use std::make_unique instead + // CHECK-FIXES: Pderived = std::make_unique(); + + // FIXME: OK to replace if assigned to unique_ptr + Pderived = std::unique_ptr(new Derived()); + + // FIXME: OK to replace when auto is not used + std::unique_ptr PBase = std::unique_ptr(new Derived()); + // The pointer is returned by the function, nothing to do. std::unique_ptr RetPtr = getPointer(); // This emulates std::move. - std::unique_ptr Move = static_cast&&>(P1); + std::unique_ptr Move = static_cast &&>(P1); - // Placemenet arguments should not be removed. + // Placement arguments should not be removed. int *PInt = new int; std::unique_ptr Placement = std::unique_ptr(new (PInt) int{3}); + Placement.reset(new (PInt) int{3}); + Placement = std::unique_ptr(new (PInt) int{3}); } // Calling make_smart_ptr from within a member function of a type with a @@ -116,6 +157,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead // CHECK-FIXES: auto callsPublic = std::make_unique(); auto ptr = std::unique_ptr(new Private(42)); + ptr.reset(new Private(42)); + ptr = std::unique_ptr(new Private(42)); } virtual ~Private(); @@ -132,6 +175,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead // CHECK-FIXES: auto callsPublic = std::make_unique(1, 2); auto ptr = std::unique_ptr(new Protected); + ptr.reset(new Protected); + ptr = std::unique_ptr(new Protected); } }; @@ -142,17 +187,25 @@ std::unique_ptr PDir1 = std::unique_ptr(new DPair(1, T)); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr PDir1 = std::make_unique(1, T); + PDir1.reset(new DPair(1, T)); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead + // CHECK-FIXES: PDir1 = std::make_unique(1, T); // Direct initialization with braces. std::unique_ptr PDir2 = std::unique_ptr(new DPair{2, T}); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr PDir2 = std::make_unique(2, T); + PDir2.reset(new DPair{2, T}); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead + // CHECK-FIXES: PDir2 = std::make_unique(2, T); // Aggregate initialization. std::unique_ptr PAggr = std::unique_ptr(new APair{T, 1}); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr PAggr = std::make_unique(APair{T, 1}); - + PAggr.reset(new APair{T, 1}); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead + // CHECK-FIXES: std::make_unique(APair{T, 1}); // Test different kinds of initialization of the pointee, when the unique_ptr // is initialized with braces. @@ -172,7 +225,6 @@ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead // CHECK-FIXES: std::unique_ptr PAggr2 = std::make_unique(APair{T, 2}); - // Direct initialization with parenthesis, without arguments. std::unique_ptr PDir5 = std::unique_ptr(new DPair()); // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead @@ -204,13 +256,13 @@ // We use 'Base' instead of 'struct Base'. typedef std::unique_ptr BasePtr; BasePtr StructType = BasePtr(new Base); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead - // CHECK-FIXES: BasePtr StructType = std::make_unique(); +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead +// CHECK-FIXES: BasePtr StructType = std::make_unique(); #define PTR unique_ptr std::unique_ptr Macro = std::PTR(new int); - // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead - // CHECK-FIXES: std::unique_ptr Macro = std::make_unique(); +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead +// CHECK-FIXES: std::unique_ptr Macro = std::make_unique(); #undef PTR std::unique_ptr Using = unique_ptr_(new int); @@ -219,6 +271,7 @@ } void whitespaces() { + // clang-format off auto Space = std::unique_ptr (new int()); // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead // CHECK-FIXES: auto Space = std::make_unique(); @@ -226,10 +279,31 @@ auto Spaces = std :: unique_ptr (new int()); // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead // CHECK-FIXES: auto Spaces = std::make_unique(); + // clang-format on } void nesting() { auto Nest = std::unique_ptr>(new std::unique_ptr(new int)); // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use std::make_unique instead // CHECK-FIXES: auto Nest = std::make_unique>(new int); + Nest.reset(new std::unique_ptr(new int)); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead + // CHECK-FIXES: Nest = std::make_unique>(new int); + Nest->reset(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead + // CHECK-FIXES: *Nest = std::make_unique(); +} + +void reset() { + std::unique_ptr P; + P.reset(); + P.reset(nullptr); + P.reset(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead + // CHECK-FIXES: P = std::make_unique(); + + auto Q = &P; + Q->reset(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead + // CHECK-FIXES: *Q = std::make_unique(); }