diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h @@ -59,8 +59,8 @@ const CXXMemberCallExpr *Member, const CXXNewExpr *New); /// Returns true when the fixes for replacing CXXNewExpr are generated. - bool replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New, - SourceManager &SM, ASTContext *Ctx); + bool replaceNew(const CXXNewExpr *New, SourceManager &SM, ASTContext *Ctx, + std::vector &Fixes); void insertHeader(DiagnosticBuilder &Diag, FileID FD); }; diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -159,17 +159,22 @@ if (Invalid) return; - auto Diag = diag(ConstructCallStart, "use %0 instead") - << MakeSmartPtrFunctionName; + diag(ConstructCallStart, "use %0 instead") << MakeSmartPtrFunctionName; // Disable the fix in macros. if (InMacro) { return; } - if (!replaceNew(Diag, New, SM, Ctx)) { + std::vector Fixes; + if (!replaceNew(New, SM, Ctx, Fixes)) { return; } + auto FixDiag = diag(ConstructCallStart, "change to %0", DiagnosticIDs::Note) + << MakeSmartPtrFunctionName; + + for (const auto &Fix : Fixes) + FixDiag << Fix; // Find the location of the template's left angle. size_t LAngle = ExprStr.find("<"); @@ -178,14 +183,13 @@ // If the template argument is missing (because it is part of the alias) // we have to add it back. ConstructCallEnd = ConstructCallStart.getLocWithOffset(ExprStr.size()); - Diag << FixItHint::CreateInsertion( - ConstructCallEnd, - "<" + GetNewExprName(New, SM, getLangOpts()) + ">"); + FixDiag << FixItHint::CreateInsertion( + ConstructCallEnd, "<" + GetNewExprName(New, SM, getLangOpts()) + ">"); } else { ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle); } - Diag << FixItHint::CreateReplacement( + FixDiag << FixItHint::CreateReplacement( CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd), MakeSmartPtrFunctionName); @@ -193,17 +197,17 @@ // parenthesis instead. if (Construct->isListInitialization()) { SourceRange BraceRange = Construct->getParenOrBraceRange(); - Diag << FixItHint::CreateReplacement( + FixDiag << FixItHint::CreateReplacement( CharSourceRange::getCharRange( BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(1)), "("); - Diag << FixItHint::CreateReplacement( + FixDiag << FixItHint::CreateReplacement( CharSourceRange::getCharRange(BraceRange.getEnd(), BraceRange.getEnd().getLocWithOffset(1)), ")"); } - insertHeader(Diag, SM.getFileID(ConstructCallStart)); + insertHeader(FixDiag, SM.getFileID(ConstructCallStart)); } void MakeSmartPtrCheck::checkReset(SourceManager &SM, ASTContext *Ctx, @@ -229,33 +233,37 @@ return; } - auto Diag = diag(ResetCallStart, "use %0 instead") - << MakeSmartPtrFunctionName; + diag(ResetCallStart, "use %0 instead") << MakeSmartPtrFunctionName; // Disable the fix in macros. if (InMacro) { return; } - if (!replaceNew(Diag, New, SM, Ctx)) { + std::vector Fixes; + if (!replaceNew(New, SM, Ctx, Fixes)) return; - } - Diag << FixItHint::CreateReplacement( + auto FixDiag = diag(ResetCallStart, "change to %0", DiagnosticIDs::Note) + << MakeSmartPtrFunctionName; + for (const auto &Fix : Fixes) + FixDiag << Fix; + + FixDiag << FixItHint::CreateReplacement( CharSourceRange::getCharRange(OperatorLoc, ExprEnd), (llvm::Twine(" = ") + MakeSmartPtrFunctionName + "<" + GetNewExprName(New, SM, getLangOpts()) + ">") .str()); if (Expr->isArrow()) - Diag << FixItHint::CreateInsertion(ExprStart, "*"); + FixDiag << FixItHint::CreateInsertion(ExprStart, "*"); - insertHeader(Diag, SM.getFileID(OperatorLoc)); + insertHeader(FixDiag, SM.getFileID(OperatorLoc)); } -bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag, - const CXXNewExpr *New, SourceManager &SM, - ASTContext *Ctx) { +bool MakeSmartPtrCheck::replaceNew(const CXXNewExpr *New, SourceManager &SM, + ASTContext *Ctx, + std::vector &Fixes) { auto SkipParensParents = [&](const Expr *E) { for (const Expr *OldE = nullptr; E != OldE;) { OldE = E; @@ -318,12 +326,12 @@ switch (New->getInitializationStyle()) { case CXXNewExpr::NoInit: { if (ArraySizeExpr.empty()) { - Diag << FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd)); + Fixes.push_back(FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd))); } else { // New array expression without written initializer: // smart_ptr(new Foo[5]); - Diag << FixItHint::CreateReplacement(SourceRange(NewStart, NewEnd), - ArraySizeExpr); + Fixes.push_back(FixItHint::CreateReplacement( + SourceRange(NewStart, NewEnd), ArraySizeExpr)); } break; } @@ -351,16 +359,17 @@ } if (ArraySizeExpr.empty()) { SourceRange InitRange = New->getDirectInitRange(); - Diag << FixItHint::CreateRemoval( - SourceRange(NewStart, InitRange.getBegin())); - Diag << FixItHint::CreateRemoval(SourceRange(InitRange.getEnd(), NewEnd)); + Fixes.push_back(FixItHint::CreateRemoval( + SourceRange(NewStart, InitRange.getBegin()))); + Fixes.push_back( + FixItHint::CreateRemoval(SourceRange(InitRange.getEnd(), NewEnd))); } else { // New array expression with default/value initialization: // smart_ptr(new int[5]()); // smart_ptr(new Foo[5]()); - Diag << FixItHint::CreateReplacement(SourceRange(NewStart, NewEnd), - ArraySizeExpr); + Fixes.push_back(FixItHint::CreateReplacement( + SourceRange(NewStart, NewEnd), ArraySizeExpr)); } break; } @@ -418,10 +427,10 @@ New->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(), New->getInitializer()->getSourceRange().getEnd()); } - Diag << FixItHint::CreateRemoval( - CharSourceRange::getCharRange(NewStart, InitRange.getBegin())); - Diag << FixItHint::CreateRemoval( - SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd)); + Fixes.push_back(FixItHint::CreateRemoval( + CharSourceRange::getCharRange(NewStart, InitRange.getBegin()))); + Fixes.push_back(FixItHint::CreateRemoval( + SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd))); break; } } diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp --- a/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp @@ -35,6 +35,7 @@ std::shared_ptr getPointer() { return std::shared_ptr(new Base); // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_shared instead + // CHECK-MESSAGES: :[[@LINE-2]]:10: note: change to std::make_shared // CHECK-FIXES: return std::make_shared(); } @@ -45,6 +46,7 @@ P1.reset(new int()); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: change to std::make_shared // CHECK-FIXES: P1 = std::make_shared(); P1 = std::shared_ptr(new int()); diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp --- a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp @@ -80,6 +80,7 @@ std::unique_ptr getPointer() { return std::unique_ptr(new Base); // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead + // CHECK-MESSAGES: :[[@LINE-2]]:10: note: change to std::make_unique // CHECK-FIXES: return std::make_unique(); } @@ -153,6 +154,7 @@ // OK to replace for reset and assign Pderived.reset(new Derived()); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_unique instead + // CHECK-MESSAGES: :[[@LINE-2]]:12: note: change to std::make_unique // CHECK-FIXES: Pderived = std::make_unique(); Pderived = std::unique_ptr(new Derived()); @@ -301,6 +303,7 @@ // Initialization with the initializer-list constructor. std::unique_ptr PE2 = std::unique_ptr(new E{1, 2}); // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead + // CHECK-MESSAGES-NOT: :[[@LINE-2]]:28: note: change to std::make_unique // CHECK-FIXES: std::unique_ptr PE2 = std::unique_ptr(new E{1, 2}); PE2.reset(new E{1, 2}); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead