Create modernize-make-unique check.

Summary: create a check that replaces 'std::unique_ptr<type>(new type(args...))' with 'std::make_unique<type>(args...)'. It was on the list of "Ideas for new Tools". It needs to be tested more carefully, but first I wanted to know if you think it is worth the effort.

Reviewers: klimek

Subscribers: cfe-commits

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

llvm-svn: 248785

clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyModernizeModule
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
+  MakeUniqueCheck.cpp
   ModernizeTidyModule.cpp
   PassByValueCheck.cpp
   ReplaceAutoPtrCheck.cpp

clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp

@@ -0,0 +1,108 @@
+//===--- MakeUniqueCheck.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 "MakeUniqueCheck.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 {
+namespace modernize {
+
+const char PointerType[] = "pointerType";
+const char ConstructorCall[] = "constructorCall";
+const char NewExpression[] = "newExpression";
+
+void MakeUniqueCheck::registerMatchers(MatchFinder *Finder) {
+  if (getLangOpts().CPlusPlus11) {
+    Finder->addMatcher(
+        cxxBindTemporaryExpr(has(
+            cxxConstructExpr(
+                hasType(qualType(hasDeclaration(classTemplateSpecializationDecl(
+                    matchesName("::std::unique_ptr"),
+                    templateArgumentCountIs(1),
+                    hasTemplateArgument(
+                        0, templateArgument(
+                               refersToType(qualType().bind(PointerType)))))))),
+                argumentCountIs(1),
+                hasArgument(0, cxxNewExpr(hasType(pointsTo(qualType(
+                                              equalsBoundNode(PointerType)))))
+                                   .bind(NewExpression)))
+                .bind(ConstructorCall))),
+        this);
+  }
+}
+
+void MakeUniqueCheck::check(const MatchFinder::MatchResult &Result) {
+  SourceManager &SM = *Result.SourceManager;
+  const auto *Construct =
+      Result.Nodes.getNodeAs<CXXConstructExpr>(ConstructorCall);
+  const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression);
+  const auto *Type = Result.Nodes.getNodeAs<QualType>(PointerType);
+
+  SourceLocation ConstructCallStart = Construct->getExprLoc();
+
+  bool Invalid = false;
+  StringRef ExprStr = Lexer::getSourceText(
+      CharSourceRange::getCharRange(
+          ConstructCallStart, Construct->getParenOrBraceRange().getBegin()),
+      SM, LangOptions(), &Invalid);
+  if (Invalid)
+    return;
+
+  auto Diag = diag(ConstructCallStart, "use std::make_unique instead");
+
+  // Find the location of the template's left angle.
+  size_t LAngle = ExprStr.find("<");
+  SourceLocation ConstructCallEnd;
+  if (LAngle == StringRef::npos) {
+    // If the template argument is missing (because it is part of the alias)
+    // we have to add it back.
+    ConstructCallEnd = ConstructCallStart.getLocWithOffset(ExprStr.size());
+    Diag << FixItHint::CreateInsertion(ConstructCallEnd,
+                                       "<" + Type->getAsString() + ">");
+  } else {
+    ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle);
+  }
+
+  Diag << FixItHint::CreateReplacement(
+      CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd),
+      "std::make_unique");
+
+  SourceLocation NewStart = New->getSourceRange().getBegin();
+  SourceLocation NewEnd = New->getSourceRange().getEnd();
+  switch (New->getInitializationStyle()) {
+  case CXXNewExpr::NoInit: {
+    Diag << FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd));
+    break;
+  }
+  case CXXNewExpr::CallInit: {
+    SourceRange InitRange = New->getDirectInitRange();
+    Diag << FixItHint::CreateRemoval(
+        SourceRange(NewStart, InitRange.getBegin()));
+    Diag << FixItHint::CreateRemoval(SourceRange(InitRange.getEnd(), NewEnd));
+    break;
+  }
+  case CXXNewExpr::ListInit: {
+    SourceRange InitRange = New->getInitializer()->getSourceRange();
+    Diag << FixItHint::CreateRemoval(
+        SourceRange(NewStart, InitRange.getBegin().getLocWithOffset(-1)));
+    Diag << FixItHint::CreateRemoval(
+        SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd));
+    break;
+  }
+  }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h

@@ -0,0 +1,41 @@
+//===--- MakeUniqueCheck.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_MODERNIZE_MAKE_UNIQUE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_UNIQUE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the pattern:
+/// \code
+///   std::unique_ptr<type>(new type(args...))
+/// \endcode
+///
+/// With the C++14 version:
+/// \code
+///   std::make_unique<type>(args...)
+/// \endcode
+class MakeUniqueCheck : public ClangTidyCheck {
+public:
+  MakeUniqueCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_UNIQUE_H
+

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "LoopConvertCheck.h"
+#include "MakeUniqueCheck.h"
 #include "PassByValueCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ShrinkToFitCheck.h"
@@ -28,6 +29,8 @@ class ModernizeModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");
+    CheckFactories.registerCheck<MakeUniqueCheck>(
+        "modernize-make-unique");
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");

clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp

@@ -0,0 +1,122 @@
+// RUN: %python %S/check_clang_tidy.py %s modernize-make-unique %t
+
+namespace std {
+
+template <typename type>
+class unique_ptr {
+public:
+  unique_ptr(type *ptr);
+  unique_ptr(const unique_ptr<type> &t) = delete;
+  unique_ptr(unique_ptr<type> &&t);
+  ~unique_ptr();
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release();
+  void reset();
+  void reset(type *pt);
+
+private:
+  type *ptr;
+};
+
+}
+
+struct Base {
+  Base();
+  Base(int, int);
+};
+
+struct Derived : public Base {
+  Derived();
+  Derived(int, int);
+};
+
+struct Pair {
+  int a, b;
+};
+
+template<class T> using unique_ptr_ = std::unique_ptr<T>;
+
+int g(std::unique_ptr<int> P);
+
+std::unique_ptr<Base> getPointer() {
+  return std::unique_ptr<Base>(new Base);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique<Base>();
+}
+
+void f() {
+  std::unique_ptr<int> P1 = std::unique_ptr<int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: std::unique_ptr<int> P1 = std::make_unique<int>();
+
+  // Without parenthesis.
+  std::unique_ptr<int> P2 = std::unique_ptr<int>(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: std::unique_ptr<int> P2 = std::make_unique<int>();
+
+  // With auto.
+  auto P3 = std::unique_ptr<int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+  // CHECK-FIXES: auto P3 = std::make_unique<int>();
+
+  {
+    // No std.
+    using namespace std;
+    unique_ptr<int> Q = unique_ptr<int>(new int());
+    // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
+    // CHECK-FIXES: unique_ptr<int> Q = std::make_unique<int>();
+  }
+
+  std::unique_ptr<int> R(new int());
+
+  // Create the unique_ptr as a parameter to a function.
+  int T = g(std::unique_ptr<int>(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+  // CHECK-FIXES: int T = g(std::make_unique<int>());
+
+  // Arguments are correctly handled.
+  std::unique_ptr<Base> Pbase = std::unique_ptr<Base>(new Base(5, T));
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<Base> Pbase = std::make_unique<Base>(5, T);
+
+  // Works with init lists.
+  std::unique_ptr<Pair> Ppair = std::unique_ptr<Pair>(new Pair{T, 1});
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<Pair> Ppair = std::make_unique<Pair>({T, 1});
+
+  // Only replace if the type in the template is the same than the type returned
+  // by the new operator.
+  auto Pderived = std::unique_ptr<Base>(new Derived());
+
+  // The pointer is returned by the function, nothing to do.
+  std::unique_ptr<Base> RetPtr = getPointer();
+
+  // Aliases.
+  typedef std::unique_ptr<int> IntPtr;
+  IntPtr Typedef = IntPtr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead
+  // CHECK-FIXES: IntPtr Typedef = std::make_unique<int>();
+
+#define PTR unique_ptr<int>
+  std::unique_ptr<int> Macro = std::PTR(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<int> Macro = std::make_unique<int>();
+#undef PTR
+
+  std::unique_ptr<int> Using = unique_ptr_<int>(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<int> Using = std::make_unique<int>();
+
+  // This emulates std::move.
+  std::unique_ptr<int> Move = static_cast<std::unique_ptr<int>&&>(P1);
+
+  // Adding whitespaces.
+  auto Space = std::unique_ptr <int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead
+  // CHECK-FIXES: auto Space = std::make_unique<int>();
+
+  auto Spaces = std  ::    unique_ptr  <int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead
+  // CHECK-FIXES: auto Spaces = std::make_unique<int>();
+}