[clang-tools-extra] 8adfb38 - [clang-tidy] Simplify diagnostics for UniqueptrResetRelease check
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 1 10:40:48 PST 2021
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);
}
More information about the cfe-commits
mailing list