[clang-tools-extra] r250742 - Added check uniqueptr-delete-release to replace "delete x.release()" with "x = nullptr"
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 19 15:00:13 PDT 2015
On Mon, Oct 19, 2015 at 2:49 PM, Samuel Benzaquen via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: sbenza
> Date: Mon Oct 19 16:49:51 2015
> New Revision: 250742
>
> URL: http://llvm.org/viewvc/llvm-project?rev=250742&view=rev
> Log:
> Added check uniqueptr-delete-release to replace "delete x.release()" with
> "x = nullptr"
>
Any stats on this? Have we seen many instances of "delete x.release()"
Also, an interesting question: should the fixit be "x = nullptr" or
"x.reset()" ? I remember having this discussion with at least Lang Hames &
he preferred the latter, which I can see, though my initial reaction is to
use the former.
>
> Reviewers: alexfh
>
> Differential Revision: http://reviews.llvm.org/D13179
>
> Added:
>
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
>
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
>
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
>
> clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
>
> clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>
> Modified: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt?rev=250742&r1=250741&r2=250742&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Mon Oct
> 19 16:49:51 2015
> @@ -13,6 +13,7 @@ add_clang_library(clangTidyReadabilityMo
> RedundantStringCStrCheck.cpp
> RedundantSmartptrGetCheck.cpp
> SimplifyBooleanExprCheck.cpp
> + UniqueptrDeleteReleaseCheck.cpp
>
> LINK_LIBS
> clangAST
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp?rev=250742&r1=250741&r2=250742&view=diff
>
> ==============================================================================
> ---
> clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
> (original)
> +++
> clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
> Mon Oct 19 16:49:51 2015
> @@ -20,6 +20,7 @@
> #include "RedundantSmartptrGetCheck.h"
> #include "RedundantStringCStrCheck.h"
> #include "SimplifyBooleanExprCheck.h"
> +#include "UniqueptrDeleteReleaseCheck.h"
>
> namespace clang {
> namespace tidy {
> @@ -40,6 +41,8 @@ public:
> "readability-identifier-naming");
>
> CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
> "readability-inconsistent-declaration-parameter-name");
> + CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
> + "readability-uniqueptr-delete-release");
> CheckFactories.registerCheck<readability::NamedParameterCheck>(
> "readability-named-parameter");
> CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
>
> Added:
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp?rev=250742&view=auto
>
> ==============================================================================
> ---
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
> (added)
> +++
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
> Mon Oct 19 16:49:51 2015
> @@ -0,0 +1,69 @@
> +//===--- UniqueptrDeleteReleaseCheck.cpp -
> clang-tidy----------------------===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===----------------------------------------------------------------------===//
> +
> +#include "UniqueptrDeleteReleaseCheck.h"
> +#include "clang/AST/ASTContext.h"
> +#include "clang/ASTMatchers/ASTMatchFinder.h"
> +#include "clang/Lex/Lexer.h"
> +
> +using namespace clang::ast_matchers;
> +
> +namespace clang {
> +namespace tidy {
> +
> +void UniqueptrDeleteReleaseCheck::registerMatchers(MatchFinder *Finder) {
> + auto IsSusbstituted = qualType(anyOf(
> + substTemplateTypeParmType(),
> hasDescendant(substTemplateTypeParmType())));
> +
> + auto UniquePtrWithDefaultDelete = classTemplateSpecializationDecl(
> + hasName("std::unique_ptr"),
> + hasTemplateArgument(1,
> refersToType(qualType(hasDeclaration(cxxRecordDecl(
> + hasName("std::default_delete")))))));
> +
> + Finder->addMatcher(
> + cxxDeleteExpr(
> +
> has(cxxMemberCallExpr(on(expr(hasType(UniquePtrWithDefaultDelete),
> + unless(hasType(IsSusbstituted)))
> + .bind("uptr")),
> +
> callee(cxxMethodDecl(hasName("release"))))))
> + .bind("delete"),
> + this);
> +}
> +
> +void UniqueptrDeleteReleaseCheck::check(
> + const MatchFinder::MatchResult &Result) {
> + const auto *PtrExpr = Result.Nodes.getNodeAs<Expr>("uptr");
> + const auto *DeleteExpr = Result.Nodes.getNodeAs<Expr>("delete");
> +
> + if (PtrExpr->getLocStart().isMacroID())
> + return;
> +
> + // Ignore dependent types.
> + // It can give us false positives, so we go with false negatives
> instead to
> + // be safe.
> + if (PtrExpr->getType()->isDependentType())
> + return;
> +
> + SourceLocation AfterPtr =
> + Lexer::getLocForEndOfToken(PtrExpr->getLocEnd(), 0,
> *Result.SourceManager,
> + Result.Context->getLangOpts());
> +
> + diag(DeleteExpr->getLocStart(),
> + "prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> "
> + "objects")
> + << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
> + DeleteExpr->getLocStart(), PtrExpr->getLocStart()))
> + << FixItHint::CreateReplacement(
> + CharSourceRange::getTokenRange(AfterPtr,
> DeleteExpr->getLocEnd()),
> + " = nullptr");
> +}
> +
> +} // namespace tidy
> +} // namespace clang
> +
>
> Added:
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h?rev=250742&view=auto
>
> ==============================================================================
> ---
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
> (added)
> +++
> clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
> Mon Oct 19 16:49:51 2015
> @@ -0,0 +1,35 @@
> +//===--- UniqueptrDeleteReleaseCheck.h - clang-tidy--------------*- C++
> -*-===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef
> LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNIQUEPTR_DELETE_RELEASE_H
> +#define
> LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNIQUEPTR_DELETE_RELEASE_H
> +
> +#include "../ClangTidy.h"
> +
> +namespace clang {
> +namespace tidy {
> +
> +/// Flag statements of the form: delete <unique_ptr expr>.release()
> +/// and replace them with: <unique_ptr expr> = nullptr
> +///
> +/// For the user-facing documentation see:
> +///
> 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) {}
> + void registerMatchers(ast_matchers::MatchFinder *Finder) override;
> + void check(const ast_matchers::MatchFinder::MatchResult &Result)
> override;
> +};
> +
> +} // namespace tidy
> +} // namespace clang
> +
> +#endif //
> LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNIQUEPTR_DELETE_RELEASE_H
> +
>
> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=250742&r1=250741&r2=250742&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Mon Oct 19
> 16:49:51 2015
> @@ -67,3 +67,4 @@ List of clang-tidy Checks
> readability-redundant-smartptr-get
> readability-redundant-string-cstr
> readability-simplify-boolean-expr
> + readability-uniqueptr-delete-release
>
> Added:
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst?rev=250742&view=auto
>
> ==============================================================================
> ---
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
> (added)
> +++
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
> Mon Oct 19 16:49:51 2015
> @@ -0,0 +1,5 @@
> +readability-uniqueptr-delete-release
> +====================================
> +
> +Replace ``delete <unique_ptr>.release()`` with ``<unique_ptr> = nullptr``.
> +The latter is shorter, simpler and does not require use of raw pointer
> APIs.
>
> Added:
> clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp?rev=250742&view=auto
>
> ==============================================================================
> ---
> clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp
> (added)
> +++
> clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp
> Mon Oct 19 16:49:51 2015
> @@ -0,0 +1,71 @@
> +// RUN: %python %S/check_clang_tidy.py %s
> readability-uniqueptr-delete-release %t
> +
> +namespace std {
> +template <typename T>
> +struct default_delete {};
> +
> +template <typename T, typename D = default_delete<T>>
> +class unique_ptr {
> + public:
> + unique_ptr();
> + ~unique_ptr();
> + explicit unique_ptr(T*);
> + template <typename U, typename E>
> + unique_ptr(unique_ptr<U, E>&&);
> + T* release();
> +};
> +} // namespace std
> +
> +std::unique_ptr<int>& ReturnsAUnique();
> +
> +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;
> +
> + 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;
> +
> + delete ReturnsAUnique().release();
> + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to
> 'delete
> + // CHECK-FIXES: {{^}} ReturnsAUnique() = nullptr;
> +}
> +
> +struct NotDefaultDeleter {};
> +
> +struct NotUniquePtr {
> + int* release();
> +};
> +
> +void Negatives() {
> + std::unique_ptr<int, NotDefaultDeleter> P;
> + delete P.release();
> +
> + NotUniquePtr P2;
> + delete P2.release();
> +}
> +
> +template <typename T, typename D>
> +void NegativeDeleterT() {
> + // Ideally this would trigger a warning, but we have all dependent types
> + // disabled for now.
> + std::unique_ptr<T> P;
> + delete P.release();
> +
> + // We ignore this one because the deleter is a template argument.
> + // Not all instantiations will use the default deleter.
> + std::unique_ptr<int, D> P2;
> + delete P2.release();
> +}
> +template void NegativeDeleterT<int, std::default_delete<int>>();
> +
> +// Test some macros
> +
> +#define DELETE_RELEASE(x) delete (x).release()
> +void NegativesWithTemplate() {
> + std::unique_ptr<int> P;
> + DELETE_RELEASE(P);
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151019/8d5759e7/attachment-0001.html>
More information about the cfe-commits
mailing list