njames93 updated this revision to Diff 326977.
njames93 marked an inline comment as done.
njames93 added a comment.
Use single back ticks for option values.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97630/new/
https://reviews.llvm.org/D97630
Files:
clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
@@ -1,5 +1,6 @@
-// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t
-
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=NULLPTR
+// RUN: %check_clang_tidy %s readability-uniqueptr-delete-release %t -check-suffix=RESET -config='{ \
+// RUN: CheckOptions: [{key: readability-uniqueptr-delete-release.PreferResetCall, value: true}]}'
namespace std {
template <typename T>
struct default_delete {};
@@ -13,6 +14,7 @@
template <typename U, typename E>
unique_ptr(unique_ptr<U, E>&&);
T* release();
+ void reset(decltype(nullptr) P = nullptr);
};
} // namespace std
@@ -21,22 +23,30 @@
void Positives() {
std::unique_ptr<int> P;
delete P.release();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
- // CHECK-FIXES: {{^}} P = nullptr;
+ // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+ // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+ // CHECK-FIXES-NULLPTR: {{^}} P = nullptr;
+ // CHECK-FIXES-RESET: {{^}} P.reset();
auto P2 = P;
delete P2.release();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
- // CHECK-FIXES: {{^}} P2 = nullptr;
+ // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' to reset 'unique_ptr<>' objects
+ // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()' to reset 'unique_ptr<>' objects
+ // CHECK-FIXES-NULLPTR: {{^}} P2 = nullptr;
+ // CHECK-FIXES-RESET: {{^}} P2.reset();
std::unique_ptr<int> Array[20];
delete Array[4].release();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
- // CHECK-FIXES: {{^}} Array[4] = nullptr;
+ // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+ // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+ // CHECK-FIXES-NULLPTR: {{^}} Array[4] = nullptr;
+ // CHECK-FIXES-RESET: {{^}} Array[4].reset();
delete ReturnsAUnique().release();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
- // CHECK-FIXES: {{^}} ReturnsAUnique() = nullptr;
+ // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr'
+ // CHECK-MESSAGES-RESET: :[[@LINE-2]]:3: warning: prefer '.reset()'
+ // CHECK-FIXES-NULLPTR: {{^}} ReturnsAUnique() = nullptr;
+ // CHECK-FIXES-RESET: {{^}} ReturnsAUnique().reset();
}
struct NotDefaultDeleter {};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
@@ -15,3 +15,21 @@
std::unique_ptr<int> P;
P = nullptr;
+
+Options
+-------
+
+.. option:: PreferResetCall
+
+ If `true`, refactor by calling the reset member function instead of
+ assigning to ``nullptr``. Default value is `false`.
+
+ .. code-block:: c++
+
+ std::unique_ptr<int> P;
+ delete P.release();
+
+ // becomes
+
+ std::unique_ptr<int> P;
+ P.reset();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
Added an option to choose the set of allowed functions.
+- Improved :doc:`readability-uniqueptr-delete-release
+ <clang-tidy/checks/readability-uniqueptr-delete-release>` check.
+
+ Added an option to choose whether to refactor by calling the ``reset`` member
+ function or assignment to ``nullptr``.
+
Improvements to include-fixer
-----------------------------
Index: clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
+++ clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
@@ -22,10 +22,13 @@
/// http://clang.llvm.org/extra/clang-tidy/checks/readability-uniqueptr-delete-release.html
class UniqueptrDeleteReleaseCheck : public ClangTidyCheck {
public:
- UniqueptrDeleteReleaseCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ UniqueptrDeleteReleaseCheck(StringRef Name, ClangTidyContext *Context);
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const bool PreferResetCall;
};
} // namespace readability
Index: clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
@@ -17,6 +17,16 @@
namespace tidy {
namespace readability {
+void UniqueptrDeleteReleaseCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "PreferResetCall", PreferResetCall);
+}
+
+UniqueptrDeleteReleaseCheck::UniqueptrDeleteReleaseCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ PreferResetCall(Options.get("PreferResetCall", false)) {}
+
void UniqueptrDeleteReleaseCheck::registerMatchers(MatchFinder *Finder) {
auto IsSusbstituted = qualType(anyOf(
substTemplateTypeParmType(), hasDescendant(substTemplateTypeParmType())));
@@ -27,11 +37,13 @@
hasName("std::default_delete")))))));
Finder->addMatcher(
- cxxDeleteExpr(has(ignoringParenImpCasts(cxxMemberCallExpr(
- on(expr(hasType(UniquePtrWithDefaultDelete),
- unless(hasType(IsSusbstituted)))
- .bind("uptr")),
- callee(cxxMethodDecl(hasName("release")))))))
+ cxxDeleteExpr(
+ has(ignoringParenImpCasts(cxxMemberCallExpr(
+ on(expr(hasType(UniquePtrWithDefaultDelete),
+ unless(hasType(IsSusbstituted)))
+ .bind("uptr")),
+ callee(memberExpr(member(cxxMethodDecl(hasName("release"))))
+ .bind("release_expr"))))))
.bind("delete"),
this);
}
@@ -40,6 +52,7 @@
const MatchFinder::MatchResult &Result) {
const auto *PtrExpr = Result.Nodes.getNodeAs<Expr>("uptr");
const auto *DeleteExpr = Result.Nodes.getNodeAs<Expr>("delete");
+ const auto *ReleaseExpr = Result.Nodes.getNodeAs<MemberExpr>("release_expr");
if (PtrExpr->getBeginLoc().isMacroID())
return;
@@ -50,17 +63,18 @@
if (PtrExpr->getType()->isDependentType())
return;
- SourceLocation AfterPtr = Lexer::getLocForEndOfToken(
- PtrExpr->getEndLoc(), 0, *Result.SourceManager, getLangOpts());
-
- diag(DeleteExpr->getBeginLoc(),
- "prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> "
- "objects")
+ diag(DeleteExpr->getBeginLoc(), "prefer '%select{= nullptr|.reset()}0' "
+ "to reset 'unique_ptr<>' objects")
+ << PreferResetCall << DeleteExpr->getSourceRange()
<< FixItHint::CreateRemoval(CharSourceRange::getCharRange(
DeleteExpr->getBeginLoc(), PtrExpr->getBeginLoc()))
- << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(AfterPtr, DeleteExpr->getEndLoc()),
- " = nullptr");
+ << (PreferResetCall
+ ? FixItHint::CreateReplacement(ReleaseExpr->getMemberLoc(),
+ "reset")
+ : FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(
+ ReleaseExpr->getOperatorLoc(), DeleteExpr->getEndLoc()),
+ " = nullptr"));
}
} // namespace readability
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
