[clang-tidy] Add "MakeSmartPtrFunction" option to modernize-make-shared/unique checks.

[Eugene.Zelenko]

It'll be good idea to run modernize-make-unique on LLVM/Clang/etc for llvm::make_unique.

[hokein]

More improvements.

[hokein]

In D34206#780455, @Eugene.Zelenko wrote:

It'll be good idea to run modernize-make-unique on LLVM/Clang/etc for llvm::make_unique.

+1. See https://reviews.llvm.org/D34334, https://reviews.llvm.org/D34333. And found a few bugs in the check, will work on fixes on them.

[alexfh]

In D34206#783673, @hokein wrote:

In D34206#780455, @Eugene.Zelenko wrote:

It'll be good idea to run modernize-make-unique on LLVM/Clang/etc for llvm::make_unique.

+1. See https://reviews.llvm.org/D34334, https://reviews.llvm.org/D34333. And found a few bugs in the check, will work on fixes on them.

Ping me once you're done fixing the bugs.

[hokein]

In D34206#790406, @alexfh wrote:

In D34206#783673, @hokein wrote:

In D34206#780455, @Eugene.Zelenko wrote:

It'll be good idea to run modernize-make-unique on LLVM/Clang/etc for llvm::make_unique.

+1. See https://reviews.llvm.org/D34334, https://reviews.llvm.org/D34333. And found a few bugs in the check, will work on fixes on them.

Ping me once you're done fixing the bugs.

I have a fix patch under review: D34286, and the other will come afterwards.

I think it is fine to check in this patch before these two bugs are fixed.

[hokein]

In D34206#790406, @alexfh wrote:

Ping me once you're done fixing the bugs.

The bugs are fixed. The patch is ready for review ;)

[alexfh]

clang-tidy/modernize/MakeSmartPtrCheck.h

28

s/makeSmartPtr/MakeSmartPtr/ here and below.

test/clang-tidy/modernize-make-shared-header.cpp

2

Maybe just add "-I%S/Inputs/modernize-smart-ptr/" flag to the clang-tidy invocation?

[hokein]

Rebase and address review comments.

[hokein]

test/clang-tidy/modernize-make-shared-header.cpp

2

Good idea!

[alexfh]

LG with a couple of nits.

test/clang-tidy/modernize-make-shared-header.cpp

9

I'd put this on the previous line.

test/clang-tidy/modernize-make-unique-header.cpp

9

I'd put this on the previous line.

[hokein]

Fix small nits.

clang-tidy/modernize/MakeSmartPtrCheck.h

 //===--- 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 "../utils/IncludeInserter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "llvm/ADT/StringRef.h"
 #include <string>
 
 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);
+                    StringRef MakeSmartPtrFunctionName);
   void registerMatchers(ast_matchers::MatchFinder *Finder) final;
+  void registerPPCallbacks(clang::CompilerInstance &Compiler) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 protected:
   using SmartPtrTypeMatcher = ast_matchers::internal::BindableMatcher<QualType>;
 
   /// 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 ResetCall[];
   static const char NewExpression[];
 
 private:
-  std::string makeSmartPtrFunctionName;
+  std::unique_ptr<utils::IncludeInserter> Inserter;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  const std::string MakeSmartPtrFunctionHeader;
+  const std::string MakeSmartPtrFunctionName;
 
   void checkConstruct(SourceManager &SM, const CXXConstructExpr *Construct,
                       const QualType *Type, const CXXNewExpr *New);
   void checkReset(SourceManager &SM, const CXXMemberCallExpr *Member,
                   const CXXNewExpr *New);
+
   void replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New,
                   SourceManager &SM);
+  void insertHeader(DiagnosticBuilder &Diag, FileID FD);
 };
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_SMART_PTR_H

clang-tidy/modernize/MakeSmartPtrCheck.cpp

 //===--- 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/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 namespace {
+
+constexpr char StdMemoryHeader[] = "memory";
+
 std::string GetNewExprName(const CXXNewExpr *NewExpr,
                            const SourceManager &SM,
                            const LangOptions &Lang) {
   StringRef WrittenName = Lexer::getSourceText(
       CharSourceRange::getTokenRange(
           NewExpr->getAllocatedTypeSourceInfo()->getTypeLoc().getSourceRange()),
       SM, Lang);
   if (NewExpr->isArray()) {
     return WrittenName.str() + "[]";
   }
   return WrittenName.str();
 }
+
 } // namespace
 
 const char MakeSmartPtrCheck::PointerType[] = "pointerType";
 const char MakeSmartPtrCheck::ConstructorCall[] = "constructorCall";
 const char MakeSmartPtrCheck::ResetCall[] = "resetCall";
 const char MakeSmartPtrCheck::NewExpression[] = "newExpression";
 
 MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
-                                     std::string makeSmartPtrFunctionName)
+                                     StringRef MakeSmartPtrFunctionName)
     : ClangTidyCheck(Name, Context),
-      makeSmartPtrFunctionName(std::move(makeSmartPtrFunctionName)) {}
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.get("IncludeStyle", "llvm"))),
+      MakeSmartPtrFunctionHeader(
+          Options.get("MakeSmartPtrFunctionHeader", StdMemoryHeader)),
+      MakeSmartPtrFunctionName(
+          Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)) {}
+
+void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "MakeSmartPtrFunctionHeader", MakeSmartPtrFunctionHeader);
+  Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
+}
+
+void MakeSmartPtrCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+  if (getLangOpts().CPlusPlus11) {
+    Inserter.reset(new utils::IncludeInserter(
+        Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
+    Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
+  }
+}
 
 void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return;
 
   // Calling make_smart_ptr from within a member function of a type with a
   // private or protected constructor would be ill-formed.
   auto CanCallCtor = unless(has(ignoringImpCasts(
void MakeSmartPtrCheck::checkConstruct(SourceManager &SM,
   StringRef ExprStr = Lexer::getSourceText(
       CharSourceRange::getCharRange(
           ConstructCallStart, Construct->getParenOrBraceRange().getBegin()),
       SM, getLangOpts(), &Invalid);
   if (Invalid)
     return;
 
   auto Diag = diag(ConstructCallStart, "use %0 instead")
-              << makeSmartPtrFunctionName;
+              << 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,
         "<" + GetNewExprName(New, SM, getLangOpts()) + ">");
   } else {
     ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle);
   }
 
   Diag << FixItHint::CreateReplacement(
       CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd),
-      makeSmartPtrFunctionName);
+      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)),
         ")");
   }
 
   replaceNew(Diag, New, SM);
+  insertHeader(Diag, SM.getFileID(ConstructCallStart));
 }
 
 void MakeSmartPtrCheck::checkReset(SourceManager &SM,
                                    const CXXMemberCallExpr *Reset,
                                    const CXXNewExpr *New) {
   const auto *Expr = cast<MemberExpr>(Reset->getCallee());
   SourceLocation OperatorLoc = Expr->getOperatorLoc();
   SourceLocation ResetCallStart = Reset->getExprLoc();
   SourceLocation ExprStart = Expr->getLocStart();
   SourceLocation ExprEnd =
       Lexer::getLocForEndOfToken(Expr->getLocEnd(), 0, SM, getLangOpts());
 
   auto Diag = diag(ResetCallStart, "use %0 instead")
-              << makeSmartPtrFunctionName;
+              << MakeSmartPtrFunctionName;
 
   Diag << FixItHint::CreateReplacement(
       CharSourceRange::getCharRange(OperatorLoc, ExprEnd),
-      (llvm::Twine(" = ") + makeSmartPtrFunctionName + "<" +
+      (llvm::Twine(" = ") + MakeSmartPtrFunctionName + "<" +
        GetNewExprName(New, SM, getLangOpts()) + ">")
           .str());
 
   if (Expr->isArrow())
     Diag << FixItHint::CreateInsertion(ExprStart, "*");
 
   replaceNew(Diag, New, SM);
+  insertHeader(Diag, SM.getFileID(OperatorLoc));
 }
 
 void MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
                                    const CXXNewExpr *New,
                                    SourceManager& SM) {
   SourceLocation NewStart = New->getSourceRange().getBegin();
   SourceLocation NewEnd = New->getSourceRange().getEnd();
 
Diag << FixItHint::CreateRemoval(
         CharSourceRange::getCharRange(NewStart, InitRange.getBegin()));
     Diag << FixItHint::CreateRemoval(
         SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd));
     break;
   }
   }
 }
 
+void MakeSmartPtrCheck::insertHeader(DiagnosticBuilder &Diag, FileID FD) {
+  if (MakeSmartPtrFunctionHeader.empty()) {
+    return;
+  }
+  if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
+          FD, MakeSmartPtrFunctionHeader,
+          /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader)) {
+    Diag << *IncludeFixit;
+  }
+}
+
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

docs/clang-tidy/checks/modernize-make-shared.rst

 
 .. code-block:: c++
 
   my_ptr.reset(new MyPair(1, 2));
 
   // becomes
 
   my_ptr = std::make_shared<MyPair>(1, 2);
+
+Options
+-------
+
+.. option:: MakeSmartPtrFunction
+
+   A string specifying the name of make-shared-ptr function. Default is
+   `std::make_shared`.
+
+.. option:: MakeSmartPtrFunctionHeader
+
+   A string specifying the corresponding header of make-shared-ptr function.
+   Default is `memory`.
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.

docs/clang-tidy/checks/modernize-make-unique.rst

 
 .. code-block:: c++
 
   my_ptr.reset(new MyPair(1, 2));
 
   // becomes
 
   my_ptr = std::make_unique<MyPair>(1, 2);
+
+Options
+-------
+
+.. option:: MakeSmartPtrFunction
+
+   A string specifying the name of make-unique-ptr function. Default is
+   `std::make_unique`.
+
+.. option:: MakeSmartPtrFunctionHeader
+
+   A string specifying the corresponding header of make-unique-ptr function.
+   Default is `memory`.
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.

test/clang-tidy/Inputs/modernize-smart-ptr/shared_ptr.h

+namespace std {
+
+template <typename type>
+class shared_ptr {
+public:
+  shared_ptr();
+  shared_ptr(type *ptr);
+  shared_ptr(const shared_ptr<type> &t) {}
+  shared_ptr(shared_ptr<type> &&t) {}
+  ~shared_ptr();
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release();
+  void reset();
+  void reset(type *pt);
+  shared_ptr &operator=(shared_ptr &&);
+  template <typename T>
+  shared_ptr &operator=(shared_ptr<T> &&);
+
+private:
+  type *ptr;
+};
+
+}  // namespace std

test/clang-tidy/Inputs/modernize-smart-ptr/unique_ptr.h

+namespace std {
+
+template <typename T>
+class default_delete {};
+
+template <typename type, typename Deleter = std::default_delete<type>>
+class unique_ptr {
+public:
+  unique_ptr();
+  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);
+  void reset(type pt);
+  unique_ptr &operator=(unique_ptr &&);
+  template <typename T>
+  unique_ptr &operator=(unique_ptr<T> &&);
+
+private:
+  type *ptr;
+};
+
+}  // namespace std

test/clang-tidy/modernize-make-shared-header.cpp

+// RUN: %check_clang_tidy %s modernize-make-shared %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:     [{key: modernize-make-shared.MakeSmartPtrFunction, \
+// RUN:       value: 'my::MakeShared'}, \
+// RUN:      {key: modernize-make-shared.MakeSmartPtrFunctionHeader, \
+// RUN:       value: 'make_shared_util.h'} \
+// RUN:     ]}" \
+// RUN:   -- -std=c++11 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
+
+#include "shared_ptr.h"
+// CHECK-FIXES: #include "make_shared_util.h"
+
+void f() {
+  std::shared_ptr<int> P1 = std::shared_ptr<int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use my::MakeShared instead
+  // CHECK-FIXES: std::shared_ptr<int> P1 = my::MakeShared<int>();
+}

test/clang-tidy/modernize-make-shared.cpp

-// RUN: %check_clang_tidy %s modernize-make-shared %t
+// RUN: %check_clang_tidy %s modernize-make-shared %t -- -- -std=c++11 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
 
-namespace std {
-
-template <typename type>
-class shared_ptr {
-public:
-  shared_ptr();
-  shared_ptr(type *ptr);
-  shared_ptr(const shared_ptr<type> &t) {}
-  shared_ptr(shared_ptr<type> &&t) {}
-  ~shared_ptr();
-  type &operator*() { return *ptr; }
-  type *operator->() { return ptr; }
-  type *release();
-  void reset();
-  void reset(type *pt);
-  shared_ptr &operator=(shared_ptr &&);
-  template <typename T>
-  shared_ptr &operator=(shared_ptr<T> &&);
-
-private:
-  type *ptr;
-};
-}
+#include "shared_ptr.h"
+// CHECK-FIXES: #include <memory>
 
 struct Base {
   Base();
   Base(int, int);
 };
 
 struct Derived : public Base {
   Derived();

test/clang-tidy/modernize-make-unique-header.cpp

+// RUN: %check_clang_tidy %s modernize-make-unique %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:     [{key: modernize-make-unique.MakeSmartPtrFunction, \
+// RUN:       value: 'my::MakeUnique'}, \
+// RUN:      {key: modernize-make-unique.MakeSmartPtrFunctionHeader, \
+// RUN:       value: 'make_unique_util.h'} \
+// RUN:     ]}" \
+// RUN:   -- -std=c++11 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
+
+#include "unique_ptr.h"
+// CHECK-FIXES: #include "make_unique_util.h"
+
+void f() {
+  std::unique_ptr<int> P1 = std::unique_ptr<int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use my::MakeUnique instead
+  // CHECK-FIXES: std::unique_ptr<int> P1 = my::MakeUnique<int>();
+}

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

-// RUN: %check_clang_tidy %s modernize-make-unique %t
+// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++11 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
 
-namespace std {
-
-template <typename T>
-class default_delete {};
-
-template <typename type, typename Deleter = std::default_delete<type>>
-class unique_ptr {
-public:
-  unique_ptr();
-  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);
-  void reset(type pt);
-  unique_ptr &operator=(unique_ptr &&);
-  template <typename T>
-  unique_ptr &operator=(unique_ptr<T> &&);
-
-private:
-  type *ptr;
-};
-}
+#include "unique_ptr.h"
+// CHECK-FIXES: #include <memory>
 
 struct Base {
   Base();
   Base(int, int);
 };
 
 struct Derived : public Base {
   Derived();