From ce18ade406a58aac9cfdea74479a6412f30d920f Mon Sep 17 00:00:00 2001 From: Piotr Padlewski Date: Mon, 2 May 2016 16:56:39 +0000 Subject: [PATCH] [clang-tidy] Add modernize-make-shared check Because modernize-make-shared do almost the same job as modernize-make-unique, I refactored common code to MakeSmartPtrCheck. http://reviews.llvm.org/D19183 llvm-svn: 268253 --- .../clang-tidy/modernize/CMakeLists.txt | 2 + .../clang-tidy/modernize/MakeSharedCheck.cpp | 31 ++++ .../clang-tidy/modernize/MakeSharedCheck.h | 43 +++++ .../clang-tidy/modernize/MakeSmartPtrCheck.cpp | 150 ++++++++++++++++ .../clang-tidy/modernize/MakeSmartPtrCheck.h | 50 ++++++ .../clang-tidy/modernize/MakeUniqueCheck.cpp | 151 ++-------------- .../clang-tidy/modernize/MakeUniqueCheck.h | 12 +- .../clang-tidy/modernize/ModernizeTidyModule.cpp | 2 + clang-tools-extra/docs/ReleaseNotes.rst | 5 + clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 + .../clang-tidy/checks/modernize-make-shared.rst | 16 ++ .../test/clang-tidy/modernize-make-shared.cpp | 200 +++++++++++++++++++++ 12 files changed, 524 insertions(+), 139 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.h create mode 100644 clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst create mode 100644 clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index b43b2a9..3c8add4 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -4,6 +4,8 @@ add_clang_library(clangTidyModernizeModule DeprecatedHeadersCheck.cpp LoopConvertCheck.cpp LoopConvertUtils.cpp + MakeSmartPtrCheck.cpp + MakeSharedCheck.cpp MakeUniqueCheck.cpp ModernizeTidyModule.cpp PassByValueCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.cpp new file mode 100644 index 0000000..8d3020c --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.cpp @@ -0,0 +1,31 @@ +//===--- MakeSharedCheck.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 "MakeSharedCheck.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { + +MakeSharedCheck::MakeSharedCheck(StringRef Name, ClangTidyContext *Context) + : MakeSmartPtrCheck(Name, Context, "std::make_shared") {} + +MakeSharedCheck::SmartPtrTypeMatcher +MakeSharedCheck::getSmartPointerTypeMatcher() const { + return qualType(hasDeclaration(classTemplateSpecializationDecl( + matchesName("::std::shared_ptr"), templateArgumentCountIs(1), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType().bind(PointerType))))))); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.h b/clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.h new file mode 100644 index 0000000..cf01446 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MakeSharedCheck.h @@ -0,0 +1,43 @@ +//===--- MakeSharedCheck.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_SHARED_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_SHARED_H + +#include "MakeSmartPtrCheck.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// Replace the pattern: +/// \code +/// std::shared_ptr(new type(args...)) +/// \endcode +/// +/// With the safer version: +/// \code +/// std::make_shared(args...) +/// \endcode +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-shared.html +class MakeSharedCheck : public MakeSmartPtrCheck { +public: + MakeSharedCheck(StringRef Name, ClangTidyContext *Context); + +protected: + SmartPtrTypeMatcher getSmartPointerTypeMatcher() const override; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_SHARED_H diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp new file mode 100644 index 0000000..d1a5612 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -0,0 +1,150 @@ +//===--- MakeSmartPtrCheck.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 "MakeSharedCheck.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { + +const char MakeSmartPtrCheck::PointerType[] = "pointerType"; +const char MakeSmartPtrCheck::ConstructorCall[] = "constructorCall"; +const char MakeSmartPtrCheck::NewExpression[] = "newExpression"; + +MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, + std::string makeSmartPtrFunctionName) + : ClangTidyCheck(Name, Context), + makeSmartPtrFunctionName(std::move(makeSmartPtrFunctionName)) {} + +void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus11) + return; + + Finder->addMatcher( + cxxBindTemporaryExpr( + has(cxxConstructExpr( + hasType(getSmartPointerTypeMatcher()), argumentCountIs(1), + hasArgument( + 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( + equalsBoundNode(PointerType)))))) + .bind(NewExpression))) + .bind(ConstructorCall))), + this); +} + +void MakeSmartPtrCheck::check(const MatchFinder::MatchResult &Result) { + // 'smart_ptr' refers to 'std::shared_ptr' or 'std::unique_ptr' or other + // pointer, 'make_smart_ptr' refers to 'std::make_shared' or + // 'std::make_unique' or other function that creates smart_ptr. + + SourceManager &SM = *Result.SourceManager; + const auto *Construct = + Result.Nodes.getNodeAs(ConstructorCall); + const auto *Type = Result.Nodes.getNodeAs(PointerType); + const auto *New = Result.Nodes.getNodeAs(NewExpression); + + if (New->getNumPlacementArgs() != 0) + return; + + 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 %0 instead") + << makeSmartPtrFunctionName; + + // 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(getLangOpts()) + ">"); + } else { + ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle); + } + + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd), + makeSmartPtrFunctionName); + + // If the smart_ptr is built with brace enclosed direct initialization, use + // parenthesis instead. + if (Construct->isListInitialization()) { + SourceRange BraceRange = Construct->getParenOrBraceRange(); + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange( + BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(1)), + "("); + Diag << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(BraceRange.getEnd(), + BraceRange.getEnd().getLocWithOffset(1)), + ")"); + } + + 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: { + // Range of the substring that we do not want to remove. + SourceRange InitRange; + if (const auto *NewConstruct = New->getConstructExpr()) { + // Direct initialization with initialization list. + // struct S { S(int x) {} }; + // smart_ptr(new S{5}); + // The arguments in the initialization list are going to be forwarded to + // the constructor, so this has to be replaced with: + // struct S { S(int x) {} }; + // std::make_smart_ptr(5); + InitRange = SourceRange( + NewConstruct->getParenOrBraceRange().getBegin().getLocWithOffset(1), + NewConstruct->getParenOrBraceRange().getEnd().getLocWithOffset(-1)); + } else { + // Aggregate initialization. + // smart_ptr(new Pair{first, second}); + // Has to be replaced with: + // smart_ptr(Pair{first, second}); + InitRange = SourceRange( + New->getAllocatedTypeSourceInfo()->getTypeLoc().getLocStart(), + New->getInitializer()->getSourceRange().getEnd()); + } + Diag << FixItHint::CreateRemoval( + CharSourceRange::getCharRange(NewStart, InitRange.getBegin())); + 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/MakeSmartPtrCheck.h b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h new file mode 100644 index 0000000..38a8b18 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h @@ -0,0 +1,50 @@ +//===--- MakeSmartPtrCheck.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_SMART_PTR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_SMART_PTR_H + +#include "../ClangTidy.h" +#include + +namespace clang { +namespace tidy { +namespace modernize { + +/// Base class for MakeSharedCheck and MakeUniqueCheck. +class MakeSmartPtrCheck : public ClangTidyCheck { +public: + MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, + std::string makeSmartPtrFunctionName); + void registerMatchers(ast_matchers::MatchFinder *Finder) override final; + void + check(const ast_matchers::MatchFinder::MatchResult &Result) override final; + +protected: + using SmartPtrTypeMatcher = ast_matchers::internal::BindableMatcher; + + /// Returns matcher that match with different smart pointer types. + /// + /// Requires to bind pointer type (qualType) with PointerType string declared + /// in this class. + virtual SmartPtrTypeMatcher getSmartPointerTypeMatcher() const = 0; + + static const char PointerType[]; + static const char ConstructorCall[]; + static const char NewExpression[]; + +private: + std::string makeSmartPtrFunctionName; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_SMART_PTR_H diff --git a/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp index a585fe3..cef8e9d 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.cpp @@ -8,9 +8,6 @@ //===----------------------------------------------------------------------===// #include "MakeUniqueCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; @@ -18,136 +15,24 @@ namespace clang { namespace tidy { namespace modernize { -static const char PointerType[] = "pointerType"; -static const char ConstructorCall[] = "constructorCall"; -static 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(2), - hasTemplateArgument(0, templateArgument(refersToType( - qualType().bind(PointerType)))), - hasTemplateArgument( - 1, templateArgument(refersToType(qualType( - hasDeclaration(classTemplateSpecializationDecl( - matchesName("::std::default_delete"), - templateArgumentCountIs(1), - hasTemplateArgument( - 0, templateArgument(refersToType( - qualType(equalsBoundNode( - PointerType))))))))))))))), - argumentCountIs(1), - hasArgument( - 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( - 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 *Type = Result.Nodes.getNodeAs(PointerType); - const auto *New = Result.Nodes.getNodeAs(NewExpression); - - if (New->getNumPlacementArgs() != 0) - return; - - 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(getLangOpts()) + ">"); - } else { - ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle); - } - - Diag << FixItHint::CreateReplacement( - CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd), - "std::make_unique"); - - // If the unique_ptr is built with brace enclosed direct initialization, use - // parenthesis instead. - if (Construct->isListInitialization()) { - SourceRange BraceRange = Construct->getParenOrBraceRange(); - Diag << FixItHint::CreateReplacement( - CharSourceRange::getCharRange( - BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(1)), - "("); - Diag << FixItHint::CreateReplacement( - CharSourceRange::getCharRange(BraceRange.getEnd(), - BraceRange.getEnd().getLocWithOffset(1)), - ")"); - } - - 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: { - // Range of the substring that we do not want to remove. - SourceRange InitRange; - if (const auto *NewConstruct = New->getConstructExpr()) { - // Direct initialization with initialization list. - // struct S { S(int x) {} }; - // std::unique_ptr(new S{5}); - // The arguments in the initialization list are going to be forwarded to - // the constructor, so this has to be replaced with: - // struct S { S(int x) {} }; - // std::make_unique(5); - InitRange = SourceRange( - NewConstruct->getParenOrBraceRange().getBegin().getLocWithOffset(1), - NewConstruct->getParenOrBraceRange().getEnd().getLocWithOffset(-1)); - } else { - // Aggregate initialization. - // std::unique_ptr(new Pair{first, second}); - // Has to be replaced with: - // std::make_unique(Pair{first, second}); - InitRange = SourceRange( - New->getAllocatedTypeSourceInfo()->getTypeLoc().getLocStart(), - New->getInitializer()->getSourceRange().getEnd()); - } - Diag << FixItHint::CreateRemoval( - CharSourceRange::getCharRange(NewStart, InitRange.getBegin())); - Diag << FixItHint::CreateRemoval( - SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd)); - break; - } - } +MakeUniqueCheck::MakeUniqueCheck(StringRef Name, + clang::tidy::ClangTidyContext *Context) + : MakeSmartPtrCheck(Name, Context, "std::make_unique") {} + +MakeUniqueCheck::SmartPtrTypeMatcher +MakeUniqueCheck::getSmartPointerTypeMatcher() const { + return qualType(hasDeclaration(classTemplateSpecializationDecl( + matchesName("::std::unique_ptr"), templateArgumentCountIs(2), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType().bind(PointerType)))), + hasTemplateArgument( + 1, templateArgument(refersToType( + qualType(hasDeclaration(classTemplateSpecializationDecl( + matchesName("::std::default_delete"), + templateArgumentCountIs(1), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType( + equalsBoundNode(PointerType)))))))))))))); } } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h b/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h index 824204e..b939633 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/MakeUniqueCheck.h @@ -10,7 +10,7 @@ #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" +#include "MakeSmartPtrCheck.h" namespace clang { namespace tidy { @@ -25,12 +25,12 @@ namespace modernize { /// \code /// std::make_unique(args...) /// \endcode -class MakeUniqueCheck : public ClangTidyCheck { +class MakeUniqueCheck : public MakeSmartPtrCheck { 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; + MakeUniqueCheck(StringRef Name, ClangTidyContext *Context); + +protected: + SmartPtrTypeMatcher getSmartPointerTypeMatcher() const override; }; } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index d7bda45..93866ba 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "DeprecatedHeadersCheck.h" #include "LoopConvertCheck.h" +#include "MakeSharedCheck.h" #include "MakeUniqueCheck.h" #include "PassByValueCheck.h" #include "RawStringLiteralCheck.h" @@ -35,6 +36,7 @@ public: CheckFactories.registerCheck( "modernize-deprecated-headers"); CheckFactories.registerCheck("modernize-loop-convert"); + CheckFactories.registerCheck("modernize-make-shared"); CheckFactories.registerCheck("modernize-make-unique"); CheckFactories.registerCheck("modernize-pass-by-value"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 219b63f..e4a88b2 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -180,6 +180,11 @@ identified. The improvements since the 3.8 release include: Replaces C standard library headers with their C++ alternatives. +- New `modernize-make-shared + `_ check + + Replaces creation of ``std::shared_ptr`` from new expression with call to ``std::make_shared``. + - New `modernize-raw-string-literal `_ check diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index cff2612..d5c680c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -92,6 +92,7 @@ Clang-Tidy Checks misc-virtual-near-miss modernize-deprecated-headers modernize-loop-convert + modernize-make-shared modernize-make-unique modernize-pass-by-value modernize-raw-string-literal diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst new file mode 100644 index 0000000..e7c690e --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst @@ -0,0 +1,16 @@ +.. title:: clang-tidy - modernize-make-shared + +modernize-make-shared +===================== + +This check finds the creation of ``std::shared_ptr`` objects by explicitly +calling the constructor and a ``new`` expression, and replaces it with a call +to ``std::make_shared``. + +.. code-block:: c++ + + auto my_ptr = std::shared_ptr(new MyPair(1, 2)); + + // becomes + + auto my_ptr = std::make_shared(1, 2); diff --git a/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp b/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp new file mode 100644 index 0000000..ee4c686 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/modernize-make-shared.cpp @@ -0,0 +1,200 @@ +// RUN: %check_clang_tidy %s modernize-make-shared %t + +namespace std { + +template +class shared_ptr { +public: + shared_ptr(type *ptr); + shared_ptr(const shared_ptr &t) {} + shared_ptr(shared_ptr &&t) {} + ~shared_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 APair { + int a, b; +}; + +struct DPair { + DPair() : a(0), b(0) {} + DPair(int x, int y) : a(y), b(x) {} + int a, b; +}; + +struct Empty {}; + +template +using shared_ptr_ = std::shared_ptr; + +void *operator new(__SIZE_TYPE__ Count, void *Ptr); + +int g(std::shared_ptr P); + +std::shared_ptr getPointer() { + return std::shared_ptr(new Base); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_shared instead + // CHECK-FIXES: return std::make_shared(); +} + +void basic() { + std::shared_ptr P1 = std::shared_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: std::shared_ptr P1 = std::make_shared(); + + // Without parenthesis. + std::shared_ptr P2 = std::shared_ptr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared] + // CHECK-FIXES: std::shared_ptr P2 = std::make_shared(); + + // With auto. + auto P3 = std::shared_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead + // CHECK-FIXES: auto P3 = std::make_shared(); + + { + // No std. + using namespace std; + shared_ptr Q = shared_ptr(new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_shared instead + // CHECK-FIXES: shared_ptr Q = std::make_shared(); + } + + std::shared_ptr R(new int()); + + // Create the shared_ptr as a parameter to a function. + int T = g(std::shared_ptr(new int())); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead + // CHECK-FIXES: int T = g(std::make_shared()); + + // Only replace if the type in the template is the same than the type returned + // by the new operator. + auto Pderived = std::shared_ptr(new Derived()); + + // The pointer is returned by the function, nothing to do. + std::shared_ptr RetPtr = getPointer(); + + // This emulates std::move. + std::shared_ptr Move = static_cast &&>(P1); + + // Placemenet arguments should not be removed. + int *PInt = new int; + std::shared_ptr Placement = std::shared_ptr(new (PInt) int{3}); +} + +void initialization(int T, Base b) { + // Test different kinds of initialization of the pointee. + + // Direct initialization with parenthesis. + std::shared_ptr PDir1 = std::shared_ptr(new DPair(1, T)); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PDir1 = std::make_shared(1, T); + + // Direct initialization with braces. + std::shared_ptr PDir2 = std::shared_ptr(new DPair{2, T}); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PDir2 = std::make_shared(2, T); + + // Aggregate initialization. + std::shared_ptr PAggr = std::shared_ptr(new APair{T, 1}); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PAggr = std::make_shared(APair{T, 1}); + + // Test different kinds of initialization of the pointee, when the shared_ptr + // is initialized with braces. + + // Direct initialization with parenthesis. + std::shared_ptr PDir3 = std::shared_ptr{new DPair(3, T)}; + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PDir3 = std::make_shared(3, T); + + // Direct initialization with braces. + std::shared_ptr PDir4 = std::shared_ptr{new DPair{4, T}}; + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PDir4 = std::make_shared(4, T); + + // Aggregate initialization. + std::shared_ptr PAggr2 = std::shared_ptr{new APair{T, 2}}; + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PAggr2 = std::make_shared(APair{T, 2}); + + // Direct initialization with parenthesis, without arguments. + std::shared_ptr PDir5 = std::shared_ptr(new DPair()); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PDir5 = std::make_shared(); + + // Direct initialization with braces, without arguments. + std::shared_ptr PDir6 = std::shared_ptr(new DPair{}); + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PDir6 = std::make_shared(); + + // Aggregate initialization without arguments. + std::shared_ptr PEmpty = std::shared_ptr(new Empty{}); + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr PEmpty = std::make_shared(Empty{}); +} + +void aliases() { + typedef std::shared_ptr IntPtr; + IntPtr Typedef = IntPtr(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_shared instead + // CHECK-FIXES: IntPtr Typedef = std::make_shared(); + + // We use 'bool' instead of '_Bool'. + typedef std::shared_ptr BoolPtr; + BoolPtr BoolType = BoolPtr(new bool); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use std::make_shared instead + // CHECK-FIXES: BoolPtr BoolType = std::make_shared(); + + // We use 'Base' instead of 'struct Base'. + typedef std::shared_ptr BasePtr; + BasePtr StructType = BasePtr(new Base); +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_shared instead +// CHECK-FIXES: BasePtr StructType = std::make_shared(); + +#define PTR shared_ptr + std::shared_ptr Macro = std::PTR(new int); +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_shared instead +// CHECK-FIXES: std::shared_ptr Macro = std::make_shared(); +#undef PTR + + std::shared_ptr Using = shared_ptr_(new int); + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_shared instead + // CHECK-FIXES: std::shared_ptr Using = std::make_shared(); +} + +void whitespaces() { + // clang-format off + auto Space = std::shared_ptr (new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_shared instead + // CHECK-FIXES: auto Space = std::make_shared(); + + auto Spaces = std :: shared_ptr (new int()); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_shared instead + // CHECK-FIXES: auto Spaces = std::make_shared(); + // clang-format on +} + +void nesting() { + auto Nest = std::shared_ptr>(new std::shared_ptr(new int)); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use std::make_shared instead + // CHECK-FIXES: auto Nest = std::make_shared>(new int); +} -- 2.7.4