[clang-tidy] Add clang-tidy check for unique_ptr's reset+release -> move

Replace x.reset(y.release()); with x = std::move(y);
If y is rvalue, replace with x = y; instead.

http://reviews.llvm.org/D6485

Patch by Alexey Sokolov!

llvm-svn: 223460

diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index ef29603..f9da3a6 100644 (file)
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_library(clangTidyMiscModule
   SwappedArgumentsCheck.cpp
   UndelegatedConstructor.cpp
   UnusedRAII.cpp
+  UniqueptrResetRelease.cpp
   UseOverride.cpp
 
   LINK_LIBS

diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
index d4ba1db..4951bd9 100644 (file)
--- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -15,6 +15,7 @@
 #include "SwappedArgumentsCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UnusedRAII.h"
+#include "UniqueptrResetRelease.h"
 #include "UseOverride.h"
 
 namespace clang {
@@ -30,6 +31,8 @@ public:
         "misc-swapped-arguments");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
+    CheckFactories.registerCheck<UniqueptrResetRelease>(
+        "misc-uniqueptr-reset-release");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
     CheckFactories.registerCheck<UseOverride>("misc-use-override");
   }

diff --git a/clang-tools-extra/clang-tidy/misc/UniqueptrResetRelease.cpp b/clang-tools-extra/clang-tidy/misc/UniqueptrResetRelease.cpp
new file mode 100644 (file)
index 0000000..0a648e2
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/UniqueptrResetRelease.cpp
@@ -0,0 +1,66 @@
+//===--- UniqueptrResetRelease.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 "UniqueptrResetRelease.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void UniqueptrResetRelease::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      memberCallExpr(
+          on(expr().bind("left")), callee(memberExpr().bind("reset_member")),
+          callee(methodDecl(hasName("reset"),
+                            ofClass(hasName("::std::unique_ptr")))),
+          has(memberCallExpr(
+              on(expr().bind("right")),
+              callee(memberExpr().bind("release_member")),
+              callee(methodDecl(hasName("release"),
+                                ofClass(hasName("::std::unique_ptr")))))))
+          .bind("reset_call"),
+      this);
+}
+
+void UniqueptrResetRelease::check(const MatchFinder::MatchResult &Result) {
+  const auto *ResetMember = Result.Nodes.getNodeAs<MemberExpr>("reset_member");
+  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 = clang::Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Left->getSourceRange()),
+      *Result.SourceManager, Result.Context->getLangOpts());
+  std::string RightText = clang::Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Right->getSourceRange()),
+      *Result.SourceManager, Result.Context->getLangOpts());
+
+  if (ResetMember->isArrow())
+    LeftText = "*" + LeftText;
+  if (ReleaseMember->isArrow())
+    RightText = "*" + RightText;
+  // Even if x was rvalue, *x is not rvalue anymore.
+  if (!Right->isRValue() || ReleaseMember->isArrow())
+    RightText = "std::move(" + RightText + ")";
+  std::string NewText = LeftText + " = " + RightText;
+
+  diag(ResetMember->getExprLoc(),
+       "prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release())")
+      << FixItHint::CreateReplacement(
+          CharSourceRange::getTokenRange(ResetCall->getSourceRange()), NewText);
+}
+
+} // namespace tidy
+} // namespace clang

diff --git a/clang-tools-extra/clang-tidy/misc/UniqueptrResetRelease.h b/clang-tools-extra/clang-tidy/misc/UniqueptrResetRelease.h
new file mode 100644 (file)
index 0000000..aacc6a1
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/UniqueptrResetRelease.h
@@ -0,0 +1,38 @@
+//===--- UniqueptrResetRelease.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_MISC_UNIQUEPTR_RESET_RELEASE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNIQUEPTR_RESET_RELEASE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Find and replace unique_ptr::reset(release()) with std::move
+///
+/// Example:
+///   std::unique_ptr<Foo> x, y;
+///   x.reset(y.release()); -> x = std::move(y);
+///
+/// If "y" is already rvalue, std::move is not added.
+/// "x" and "y" can also be std::unique_ptr<Foo>*.
+class UniqueptrResetRelease : public ClangTidyCheck {
+public:
+  UniqueptrResetRelease(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_MISC_UNIQUEPTR_RESET_RELEASE_H

diff --git a/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp b/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp
new file mode 100644 (file)
index 0000000..763a428
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp
@@ -0,0 +1,48 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-uniqueptr-reset-release %t
+// REQUIRES: shell
+
+namespace std {
+template <typename T>
+struct unique_ptr {
+  unique_ptr<T>();
+  explicit unique_ptr<T>(T *);
+  template <typename U>
+  unique_ptr<T>(unique_ptr<U> &&);
+  void reset(T *);
+  T *release();
+};
+} // namespace std
+
+struct Foo {};
+struct Bar : Foo {};
+
+std::unique_ptr<Foo> Create();
+std::unique_ptr<Foo> &Look();
+std::unique_ptr<Foo> *Get();
+
+void f() {
+  std::unique_ptr<Foo> a, b;
+  std::unique_ptr<Bar> c;
+  std::unique_ptr<Foo> *x = &a;
+  std::unique_ptr<Foo> *y = &b;
+
+  a.reset(b.release());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release()) [misc-uniqueptr-reset-release]
+  // CHECK-FIXES: a = std::move(b);
+  a.reset(c.release());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr1 = std::move(ptr2)
+  // CHECK-FIXES: a = std::move(c);
+  a.reset(Create().release());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: prefer ptr1 = std::move(ptr2)
+  // CHECK-FIXES: a = Create();
+  x->reset(y->release());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: prefer ptr1 = std::move(ptr2)
+  // CHECK-FIXES: *x = std::move(*y);
+  Look().reset(Look().release());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2)
+  // CHECK-FIXES: Look() = std::move(Look());
+  Get()->reset(Get()->release());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2)
+  // CHECK-FIXES: *Get() = std::move(*Get());
+}
+