From 26fd0e8b6294e81a983eba9efb06b765e06e3a4e Mon Sep 17 00:00:00 2001 From: Angel Garcia Gomez Date: Tue, 29 Sep 2015 09:36:41 +0000 Subject: [PATCH] Create modernize-make-unique check. Summary: create a check that replaces 'std::unique_ptr(new type(args...))' with 'std::make_unique(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-tidy/modernize/CMakeLists.txt | 1 + .../clang-tidy/modernize/MakeUniqueCheck.cpp | 108 ++++++++++++++++ .../clang-tidy/modernize/MakeUniqueCheck.h | 41 ++++++ .../modernize/ModernizeTidyModule.cpp | 3 + .../test/clang-tidy/modernize-make-unique.cpp | 122 ++++++++++++++++++ 5 files changed, 275 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h create mode 100644 clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 185892426726..2cea862b5741 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp new file mode 100644 index 000000000000..77c684984de8 --- /dev/null +++ b/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(ConstructorCall); + const auto *New = Result.Nodes.getNodeAs(NewExpression); + const auto *Type = Result.Nodes.getNodeAs(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 diff --git a/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h b/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h new file mode 100644 index 000000000000..824204e2162b --- /dev/null +++ b/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(new type(args...)) +/// \endcode +/// +/// With the C++14 version: +/// \code +/// std::make_unique(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 + diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 568abbdac4be..b780cb8aca51 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/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("modernize-loop-convert"); + CheckFactories.registerCheck( + "modernize-make-unique"); CheckFactories.registerCheck("modernize-pass-by-value"); CheckFactories.registerCheck( "modernize-replace-auto-ptr"); diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp new file mode 100644 index 000000000000..e285be5d09ed --- /dev/null +++ b/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 +class unique_ptr { +public: + unique_ptr(type *ptr); + unique_ptr(const unique_ptr &t) = delete; + unique_ptr(unique_ptr &&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 using unique_ptr_ = std::unique_ptr; + +int g(std::unique_ptr P); + +std::unique_ptr getPointer() { + return std::unique_ptr(new Base); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead + // CHECK-FIXES: return std::make_unique(); +} + +void f() { + std::unique_ptr P1 = std::unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: std::unique_ptr P1 = std::make_unique(); + + // Without parenthesis. + std::unique_ptr P2 = std::unique_ptr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique] + // CHECK-FIXES: std::unique_ptr P2 = std::make_unique(); + + // With auto. + auto P3 = std::unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead + // CHECK-FIXES: auto P3 = std::make_unique(); + + { + // No std. + using namespace std; + unique_ptr Q = unique_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead + // CHECK-FIXES: unique_ptr Q = std::make_unique(); + } + + std::unique_ptr R(new int()); + + // Create the unique_ptr as a parameter to a function. + int T = g(std::unique_ptr(new int())); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead + // CHECK-FIXES: int T = g(std::make_unique()); + + // Arguments are correctly handled. + std::unique_ptr Pbase = std::unique_ptr(new Base(5, T)); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr Pbase = std::make_unique(5, T); + + // Works with init lists. + std::unique_ptr Ppair = std::unique_ptr(new Pair{T, 1}); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr Ppair = std::make_unique({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(new Derived()); + + // The pointer is returned by the function, nothing to do. + std::unique_ptr RetPtr = getPointer(); + + // Aliases. + typedef std::unique_ptr IntPtr; + IntPtr Typedef = IntPtr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead + // CHECK-FIXES: IntPtr Typedef = std::make_unique(); + +#define PTR unique_ptr + std::unique_ptr Macro = std::PTR(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr Macro = std::make_unique(); +#undef PTR + + std::unique_ptr Using = unique_ptr_(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead + // CHECK-FIXES: std::unique_ptr Using = std::make_unique(); + + // This emulates std::move. + std::unique_ptr Move = static_cast&&>(P1); + + // Adding whitespaces. + auto Space = std::unique_ptr (new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead + // CHECK-FIXES: auto Space = std::make_unique(); + + auto Spaces = std :: unique_ptr (new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead + // CHECK-FIXES: auto Spaces = std::make_unique(); +}