Author: Nathan James Date: 2021-03-01T18:40:37Z New Revision: 8adfb38224697afca205343c0e1a49bd03ecfc09
URL: https://github.com/llvm/llvm-project/commit/8adfb38224697afca205343c0e1a49bd03ecfc09 DIFF: https://github.com/llvm/llvm-project/commit/8adfb38224697afca205343c0e1a49bd03ecfc09.diff LOG: [clang-tidy] Simplify diagnostics for UniqueptrResetRelease check Tweak the diagnostics to create small replacements rather than grabbing source text from the lexer. Also simplified the diagnostic message. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D97632 Added: Modified: clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp index f1e2cbe1fed08..dffb03f76a3d9 100644 --- a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp @@ -19,18 +19,21 @@ namespace misc { void UniqueptrResetReleaseCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( cxxMemberCallExpr( - on(expr().bind("left")), callee(memberExpr().bind("reset_member")), - callee( - cxxMethodDecl(hasName("reset"), - ofClass(cxxRecordDecl(hasName("::std::unique_ptr"), - decl().bind("left_class"))))), - has(ignoringParenImpCasts(cxxMemberCallExpr( - on(expr().bind("right")), - callee(memberExpr().bind("release_member")), - callee(cxxMethodDecl( - hasName("release"), - ofClass(cxxRecordDecl(hasName("::std::unique_ptr"), - decl().bind("right_class"))))))))) + callee(memberExpr( + member(cxxMethodDecl( + hasName("reset"), + ofClass(cxxRecordDecl(hasName("::std::unique_ptr"), + decl().bind("left_class")))))) + .bind("reset_member")), + hasArgument( + 0, ignoringParenImpCasts(cxxMemberCallExpr( + on(expr().bind("right")), + callee(memberExpr(member(cxxMethodDecl( + hasName("release"), + ofClass(cxxRecordDecl( + hasName("::std::unique_ptr"), + decl().bind("right_class")))))) + .bind("release_member")))))) .bind("reset_call"), this); } @@ -95,37 +98,31 @@ void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult &Result) { const auto *ReleaseMember = Result.Nodes.getNodeAs<MemberExpr>("release_member"); const auto *Right = Result.Nodes.getNodeAs<Expr>("right"); - const auto *Left = Result.Nodes.getNodeAs<Expr>("left"); const auto *ResetCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset_call"); - std::string LeftText = std::string(clang::Lexer::getSourceText( - CharSourceRange::getTokenRange(Left->getSourceRange()), - *Result.SourceManager, getLangOpts())); - std::string RightText = std::string(clang::Lexer::getSourceText( - CharSourceRange::getTokenRange(Right->getSourceRange()), - *Result.SourceManager, getLangOpts())); - - if (ResetMember->isArrow()) - LeftText = "*" + LeftText; - if (ReleaseMember->isArrow()) - RightText = "*" + RightText; - bool IsMove = false; - // Even if x was rvalue, *x is not rvalue anymore. - if (!Right->isRValue() || ReleaseMember->isArrow()) { - RightText = "std::move(" + RightText + ")"; - IsMove = true; + StringRef AssignmentText = " = "; + StringRef TrailingText = ""; + if (ReleaseMember->isArrow()) { + AssignmentText = " = std::move(*"; + TrailingText = ")"; + } else if (!Right->isRValue()) { + AssignmentText = " = std::move("; + TrailingText = ")"; } - std::string NewText = LeftText + " = " + RightText; - - diag(ResetMember->getExprLoc(), - "prefer ptr = %select{std::move(ptr2)|ReturnUnique()}0 over " - "ptr.reset(%select{ptr2|ReturnUnique()}0.release())") - << !IsMove - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(ResetCall->getSourceRange()), - NewText); + auto D = diag(ResetMember->getExprLoc(), + "prefer 'unique_ptr<>' assignment over 'release' and 'reset'"); + if (ResetMember->isArrow()) + D << FixItHint::CreateInsertion(ResetMember->getBeginLoc(), "*"); + D << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(ResetMember->getOperatorLoc(), + Right->getBeginLoc()), + AssignmentText) + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(ReleaseMember->getOperatorLoc(), + ResetCall->getEndLoc()), + TrailingText); } } // namespace misc diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp index c97b9b3f4a968..befd2a0576d2d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-uniqueptr-reset-release.cpp @@ -33,27 +33,27 @@ void f() { std::unique_ptr<Foo> *y = &b; a.reset(b.release()); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2) over ptr.reset(ptr2.release()) [misc-uniqueptr-reset-release] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment over 'release' and 'reset' [misc-uniqueptr-reset-release] // CHECK-FIXES: a = std::move(b); a.reset(c.release()); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment // CHECK-FIXES: a = std::move(c); a.reset(Create().release()); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release()) [misc-uniqueptr-reset-release] + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer 'unique_ptr<>' assignment // CHECK-FIXES: a = Create(); x->reset(y->release()); - // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer ptr = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer 'unique_ptr<>' assignment // CHECK-FIXES: *x = std::move(*y); Look().reset(Look().release()); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment // CHECK-FIXES: Look() = std::move(Look()); Get()->reset(Get()->release()); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment // CHECK-FIXES: *Get() = std::move(*Get()); std::unique_ptr<Bar, FooFunc> func_a, func_b; func_a.reset(func_b.release()); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr = std::move(ptr2) + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer 'unique_ptr<>' assignment // CHECK-FIXES: func_a = std::move(func_b); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
