Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt @@ -13,6 +13,7 @@ RedundantStringCStrCheck.cpp RedundantSmartptrGetCheck.cpp SimplifyBooleanExprCheck.cpp + UniqueptrDeleteReleaseCheck.cpp LINK_LIBS clangAST Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -20,6 +20,7 @@ #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "SimplifyBooleanExprCheck.h" +#include "UniqueptrDeleteReleaseCheck.h" namespace clang { namespace tidy { @@ -40,6 +41,8 @@ "readability-identifier-naming"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-uniqueptr-delete-release"); CheckFactories.registerCheck( "readability-named-parameter"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h +++ clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h @@ -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 .release() +/// and replace them with: = 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 + Index: clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp @@ -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("uptr"); + const auto *DeleteExpr = Result.Nodes.getNodeAs("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 + Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -67,3 +67,4 @@ readability-redundant-smartptr-get readability-redundant-string-cstr readability-simplify-boolean-expr + readability-uniqueptr-delete-release Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst @@ -0,0 +1,5 @@ +readability-uniqueptr-delete-release +==================================== + +Replace ``delete .release()`` with `` = nullptr``. +The latter is shorter, simpler and does not require use of raw pointer APIs. Index: clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-uniqueptr-delete-release.cpp @@ -0,0 +1,71 @@ +// RUN: %python %S/check_clang_tidy.py %s readability-uniqueptr-delete-release %t + +namespace std { +template +struct default_delete {}; + +template > +class unique_ptr { + public: + unique_ptr(); + ~unique_ptr(); + explicit unique_ptr(T*); + template + unique_ptr(unique_ptr&&); + T* release(); +}; +} // namespace std + +std::unique_ptr& ReturnsAUnique(); + +void Positives() { + std::unique_ptr 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 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 P; + delete P.release(); + + NotUniquePtr P2; + delete P2.release(); +} + +template +void NegativeDeleterT() { + // Ideally this would trigger a warning, but we have all dependent types + // disabled for now. + std::unique_ptr 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 P2; + delete P2.release(); +} +template void NegativeDeleterT>(); + +// Test some macros + +#define DELETE_RELEASE(x) delete (x).release() +void NegativesWithTemplate() { + std::unique_ptr P; + DELETE_RELEASE(P); +}