Added check uniqueptr-delete-release to replace "delete x.release()" with "x = nullptr"

Reviewers: alexfh

Differential Revision: http://reviews.llvm.org/D13179

llvm-svn: 250742

7 files changed, 185 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 8ed1018e67f..2e4834ecabf 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -13,6 +13,7 @@ add_clang_library(clangTidyReadabilityModule
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   SimplifyBooleanExprCheck.cpp
+  UniqueptrDeleteReleaseCheck.cpp
 
   LINK_LIBS
   clangAST
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 40c00d4f45c..14c24a33130 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/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 @@ 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>(
diff --git a/clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp b/clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.cpp
new file mode 100644
index 00000000000..8a1ca4c16ce
--- /dev/null
+++ b/clang-tools-extra/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<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
+
diff --git a/clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h b/clang-tools-extra/clang-tidy/readability/UniqueptrDeleteReleaseCheck.h
new file mode 100644
index 00000000000..ac55c7690a0
--- /dev/null
+++ b/clang-tools-extra/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 <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
+
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 8b3cfa373e2..d1dda8ecdb2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -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
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
new file mode 100644
index 00000000000..6bfc692a3bd
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
@@ -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.
diff --git a/clang-tools-extra/test/clang-tidy/readability-uniqueptr-delete-release.cpp b/clang-tools-extra/test/clang-tidy/readability-uniqueptr-delete-release.cpp
new file mode 100644
index 00000000000..8fb7562dd41
--- /dev/null
+++ b/clang-tools-extra/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 <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);
+}