[clang-tidy] Omit std::make_unique/make_shared for default initialization.

[ckennelly]

Summary

This extends the check for default initialization in arrays added in
547f89d6070 to include scalar types and exclude them from the suggested fix for
make_unique/make_shared.

Rewriting std::unique_ptr<int>(new int) as std::make_unique<int>() (or for
other, similar trivial T) switches from default initialization to value
initialization, a performance regression for trivial T. For these use cases,
std::make_unique_for_overwrite is more suitable alternative.

[ymandel]

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

139

No braces around the body.

clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

62–63

Please extend comment to briefly explain why there's no fix for this case.

[njames93]

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

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

136–139

Drop the const, we don't use const on local variables in the codebase.

[ckennelly]

Apply feedback from ymandel

[ckennelly]

Apply review feedback from njames93

[ckennelly]

In D90392#2362118, @njames93 wrote:

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

I disagree with printing a warning but not a fix.

These uses should migrate to std::make_unique_for_overwrite/std::make_shared_for_overwrite. I am planning on sending a follow-up patch for that but want to avoid the existing make-unique/make-shared checks try to migrate default initialization use cases.

[lebedev.ri]

In D90392#2362308, @ckennelly wrote:

In D90392#2362118, @njames93 wrote:

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

I disagree with printing a warning but not a fix.

These uses should migrate to std::make_unique_for_overwrite/std::make_shared_for_overwrite. I am planning on sending a follow-up patch for that but want to avoid the existing make-unique/make-shared checks try to migrate default initialization use cases.

Reminder that there is more than one version of C++ standard, and users are not obligated to be using some particular version, and the checks should not be making that decision for user.

[ckennelly]

In D90392#2362327, @lebedev.ri wrote:

In D90392#2362308, @ckennelly wrote:

In D90392#2362118, @njames93 wrote:

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

I disagree with printing a warning but not a fix.

These uses should migrate to std::make_unique_for_overwrite/std::make_shared_for_overwrite. I am planning on sending a follow-up patch for that but want to avoid the existing make-unique/make-shared checks try to migrate default initialization use cases.

Reminder that there is more than one version of C++ standard, and users are not obligated to be using some particular version, and the checks should not be making that decision for user.

Understood, but replacing std::unique_ptr<int>(new int) with std::make_unique<int>() does change program behavior. Once the code migrates to value initialization, it can be hard to determine later whether that initialization ends up being unnecessary when C++20 or an analogue for std::make_unique_for_overwrite is available.

I'm hesitant to nudge users towards the behavior change when alternatives would be available (maintain the status quo, have C++20, preadopt a make_unique_for_overwrite helper like we do with this clang-tidy for pre-C++14 builds).

[ckennelly]

Ping

[lebedev.ri]

I think for starters this patch needs a better description, explaining not *what* it does, but *why* it does what it does.
Also, it is probably not good to change (break) existing test coverage.
Instead, it is best to add new tests, and adjust existing check lines.

[ckennelly]

Expand tests to cover value and default initialization

[steveire]

clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

51

I'm a bit confused. Why don't we want to transform this?

[ckennelly]

clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

51

std::make_shared<int>() is equivalent to std::shared_ptr<int>(new int()). This value initializes (read: zeroes out the value).

The statement here is only default initializing (read: leaves the value uninitialized). For this use case, we should use std::make_shared_for_overwrite when it is available.

[steveire]

clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

51

I see. Did you consider an option to transform these anyway? It seems at least as likely that for real-world code bases they are not initialized accidentally (as opposed to being a deliberate choice as assumed here).

[ckennelly]

njames93: Ping

clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

51

The default initialization case for arrays was added because it caused several performance regressions. I want to get the complete initialization check implemented, which will then make the make_unique_for_overwrite rewrite straight forward (just !initializating).

An option could then be added in a subsequent commit.

[lebedev.ri]

In D90392#2362118, @njames93 wrote:

IIUC, this is handling the case where Ptr.reset(new int) which is different to Ptr.reset(new int()) because the former doesn't initialise the int while the latter default(zero) initialises it.
If that's correct I still think we should still warn about this case, but don't suggest an auto-fix as that will change behaviour.
Maybe put a note explaining why it can't be auto-fixed.

+1

[ckennelly]

Added option to allow rewrites for default to value initialization

[ckennelly]

Ping

[ckennelly]

Ping

[lebedev.ri]

I think the patches description still could use some more words,
in particular it still only repeats what the patch does,
but not why it does that.

[hokein]

thanks, I think this patch is good, feel free to land it (and sorry for the delay).

As @lebedev.ri commented, would be nice to mention the motivation of the change (make_shared_for_overwrite?) in the description.

[ckennelly]

rebase

[ckennelly]

updating commit message

[ckennelly]

In D90392#2436639, @hokein wrote:

thanks, I think this patch is good, feel free to land it (and sorry for the delay).

As @lebedev.ri commented, would be nice to mention the motivation of the change (make_shared_for_overwrite?) in the description.

Done

[ckennelly]

@njames93: Any additional comments?

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

protected:
 
   static const char PointerType[];
 
 private:
   utils::IncludeInserter Inserter;
   const std::string MakeSmartPtrFunctionHeader;
   const std::string MakeSmartPtrFunctionName;
   const bool IgnoreMacros;
+  const bool IgnoreDefaultInitialization;
 
   void checkConstruct(SourceManager &SM, ASTContext *Ctx,
                       const CXXConstructExpr *Construct, const QualType *Type,
                       const CXXNewExpr *New);
   void checkReset(SourceManager &SM, ASTContext *Ctx,
                   const CXXMemberCallExpr *Member, const CXXNewExpr *New);
 
   /// Returns true when the fixes for replacing CXXNewExpr are generated.

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

 //===--- MakeSmartPtrCheck.cpp - clang-tidy--------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "../utils/TypeTraits.h"
 #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 {
MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
                                      StringRef MakeSmartPtrFunctionName)
     : ClangTidyCheck(Name, Context),
       Inserter(Options.getLocalOrGlobal("IncludeStyle",
                                         utils::IncludeSorter::IS_LLVM)),
       MakeSmartPtrFunctionHeader(
           Options.get("MakeSmartPtrFunctionHeader", "<memory>")),
       MakeSmartPtrFunctionName(
           Options.get("MakeSmartPtrFunction", MakeSmartPtrFunctionName)),
-      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+      IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+      IgnoreDefaultInitialization(
+          Options.get("IgnoreDefaultInitialization", true)) {}
 
 void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "MakeSmartPtrFunctionHeader", MakeSmartPtrFunctionHeader);
   Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+  Options.store(Opts, "IgnoreDefaultInitialization",
+                IgnoreDefaultInitialization);
 }
 
 bool MakeSmartPtrCheck::isLanguageVersionSupported(
     const LangOptions &LangOpts) const {
   return LangOpts.CPlusPlus11;
 }
 
 void MakeSmartPtrCheck::registerPPCallbacks(const SourceManager &SM,
void MakeSmartPtrCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Reset = Result.Nodes.getNodeAs<CXXMemberCallExpr>(ResetCall);
   const auto *Type = Result.Nodes.getNodeAs<QualType>(PointerType);
   const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression);
 
   // Skip when this is a new-expression with `auto`, e.g. new auto(1)
   if (New->getType()->getPointeeType()->getContainedAutoType())
     return;
 
-  // Be conservative for cases where we construct an array without any
-  // initialization.
+  // Be conservative for cases where we construct and default initialize.
+  //
   // For example,
+  //    P.reset(new int)    // check fix: P = std::make_unique<int>()
   //    P.reset(new int[5]) // check fix: P = std::make_unique<int []>(5)
   //
-  // The fix of the check has side effect, it introduces default initialization
+  // The fix of the check has side effect, it introduces value initialization
   // which maybe unexpected and cause performance regression.
-  if (New->isArray() && !New->hasInitializer())
+  bool Initializes = New->hasInitializer() ||
+                     !utils::type_traits::isTriviallyDefaultConstructible(
+                         New->getAllocatedType(), *Result.Context);
+  if (!Initializes && IgnoreDefaultInitialization)
     return;
   if (Construct)
     checkConstruct(SM, Result.Context, Construct, Type, New);
   else if (Reset)
     checkReset(SM, Result.Context, Reset, New);
 }
 
 void MakeSmartPtrCheck::checkConstruct(SourceManager &SM, ASTContext *Ctx,

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

.. option:: IncludeStyle
 
    A string specifying which include-style is used, `llvm` or `google`. Default
    is `llvm`.
 
 .. option:: IgnoreMacros
 
    If set to `true`, the check will not give warnings inside macros. Default
    is `true`.
+
+.. option:: IgnoreDefaultInitialization
+
+   If set to non-zero, the check does not suggest edits that will transform
+   default initialization into value initialization, as this can cause
+   performance regressions. Default is `1`.

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

.. option:: IncludeStyle
 
    A string specifying which include-style is used, `llvm` or `google`. Default
    is `llvm`.
 
 .. option:: IgnoreMacros
 
    If set to `true`, the check will not give warnings inside macros. Default
    is `true`.
+
+.. option:: IgnoreDefaultInitialization
+
+   If set to non-zero, the check does not suggest edits that will transform
+   default initialization into value initialization, as this can cause
+   performance regressions. Default is `1`.

clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp

 int g(std::shared_ptr<int> P);
 
 std::shared_ptr<Base> getPointer() {
   return std::shared_ptr<Base>(new Base);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_shared instead
   // CHECK-FIXES: return std::make_shared<Base>();
 }
 
+std::shared_ptr<Base> getPointerValue() {
+  return std::shared_ptr<Base>(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_shared instead
+  // CHECK-FIXES: return std::make_shared<Base>();
+}
+
 void basic() {
   std::shared_ptr<int> P1 = std::shared_ptr<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: std::shared_ptr<int> P1 = std::make_shared<int>();
+  std::shared_ptr<int> P2 = std::shared_ptr<int>(new int);
 
   P1.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P1 = std::make_shared<int>();
+  P1.reset(new int);
 
   P1 = std::shared_ptr<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P1 = std::make_shared<int>();
+  P1 = std::shared_ptr<int>(new int);
 
-  // Without parenthesis.
-  std::shared_ptr<int> P2 = std::shared_ptr<int>(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared]
-  // CHECK-FIXES: std::shared_ptr<int> P2 = std::make_shared<int>();
+  // Without parenthesis, default initialization.
+  std::shared_ptr<int> P3 = std::shared_ptr<int>(new int);
 
-  P2.reset(new int);
+  P2.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P2 = std::make_shared<int>();
+  P2.reset(new int);
 
-  P2 = std::shared_ptr<int>(new int);
+  P2 = std::shared_ptr<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P2 = std::make_shared<int>();
+  P2 = std::shared_ptr<int>(new int);
 
   // With auto.
-  auto P3 = std::shared_ptr<int>(new int());
+  auto P4 = std::shared_ptr<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead
-  // CHECK-FIXES: auto P3 = std::make_shared<int>();
+  // CHECK-FIXES: auto P4 = std::make_shared<int>();
+  auto P5 = std::shared_ptr<int>(new int);
 
-  std::shared_ptr<int> P4 = std::shared_ptr<int>((new int));
+  std::shared_ptr<int> P6 = std::shared_ptr<int>((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_shared instead [modernize-make-shared]
-  // CHECK-FIXES: std::shared_ptr<int> P4 = std::make_shared<int>();
+  // CHECK-FIXES: std::shared_ptr<int> P6 = std::make_shared<int>();
+  std::shared_ptr<int> P7 = std::shared_ptr<int>((new int));
 
   P4.reset((((new int()))));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P4 = std::make_shared<int>();
+  P4.reset((((new int))));
 
-  P4 = std::shared_ptr<int>(((new int)));
+  P4 = std::shared_ptr<int>(((new int())));
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P4 = std::make_shared<int>();
+  P4 = std::shared_ptr<int>(((new int)));
 
   {
     // No std.
     using namespace std;
     shared_ptr<int> Q = shared_ptr<int>(new int());
     // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_shared instead
     // CHECK-FIXES: shared_ptr<int> Q = std::make_shared<int>();
+    shared_ptr<int> P = shared_ptr<int>(new int);
 
     Q = shared_ptr<int>(new int());
     // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead
     // CHECK-FIXES: Q = std::make_shared<int>();
+    Q = shared_ptr<int>(new int);
   }
 
   std::shared_ptr<int> R(new int());
+  std::shared_ptr<int> S(new int);
 
   // Create the shared_ptr as a parameter to a function.
   int T = g(std::shared_ptr<int>(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead
   // CHECK-FIXES: int T = g(std::make_shared<int>());
+  T = g(std::shared_ptr<int>(new int));
 
   // Only replace if the type in the template is the same as the type returned
   // by the new operator.
   auto Pderived = std::shared_ptr<Base>(new Derived());
+  auto PderivedNoparen = std::shared_ptr<Base>(new Derived);
 
   // OK to replace for reset and assign
   Pderived.reset(new Derived());
   // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_shared instead
   // CHECK-FIXES: Pderived = std::make_shared<Derived>();
+  Pderived.reset(new Derived);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_shared instead
+  // CHECK-FIXES: Pderived = std::make_shared<Derived>();
 
   Pderived = std::shared_ptr<Derived>(new Derived());
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use std::make_shared instead
   // CHECK-FIXES: Pderived = std::make_shared<Derived>();
+  Pderived = std::shared_ptr<Derived>(new Derived);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use std::make_shared instead
+  // CHECK-FIXES: Pderived = std::make_shared<Derived>();
 
   // FIXME: OK to replace if assigned to shared_ptr<Base>
   Pderived = std::shared_ptr<Base>(new Derived());
+  Pderived = std::shared_ptr<Base>(new Derived);
 
   // FIXME: OK to replace when auto is not used
   std::shared_ptr<Base> PBase = std::shared_ptr<Base>(new Derived());
+  std::shared_ptr<Base> PBase2 = std::shared_ptr<Base>(new Derived);
 
   // The pointer is returned by the function, nothing to do.
   std::shared_ptr<Base> RetPtr = getPointer();
+  std::shared_ptr<Base> RetPtr2 = getPointerValue();
 
   // This emulates std::move.
   std::shared_ptr<int> Move = static_cast<std::shared_ptr<int> &&>(P1);
 
   // Placement arguments should not be removed.
   int *PInt = new int;
   std::shared_ptr<int> Placement = std::shared_ptr<int>(new (PInt) int{3});
   Placement.reset(new (PInt) int{3});
   Placement = std::shared_ptr<int>(new (PInt) int{3});
+
+  std::shared_ptr<int> PlacementNoparen = std::shared_ptr<int>(new (PInt) int);
+  PlacementNoparen.reset(new (PInt) int);
+  PlacementNoparen = std::shared_ptr<int>(new (PInt) int);
 }
 
 // Calling make_smart_ptr from within a member function of a type with a
 // private or protected constructor would be ill-formed.
 class Private {
 private:
   Private(int z) {}
 
void initialization(int T, Base b) {
   // Aggregate initialization without arguments.
   std::shared_ptr<Empty> PEmpty = std::shared_ptr<Empty>(new Empty{});
   // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_shared instead
   // CHECK-FIXES: std::shared_ptr<Empty> PEmpty = std::make_shared<Empty>(Empty{});
 }
 
 void aliases() {
   typedef std::shared_ptr<int> IntPtr;
-  IntPtr Typedef = IntPtr(new int);
+  IntPtr Typedef = IntPtr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_shared instead
   // CHECK-FIXES: IntPtr Typedef = std::make_shared<int>();
+  IntPtr Typedef2 = IntPtr(new int);
 
   // We use 'bool' instead of '_Bool'.
   typedef std::shared_ptr<bool> BoolPtr;
-  BoolPtr BoolType = BoolPtr(new bool);
+  BoolPtr BoolType = BoolPtr(new bool());
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use std::make_shared instead
   // CHECK-FIXES: BoolPtr BoolType = std::make_shared<bool>();
+  BoolPtr BoolType2 = BoolPtr(new bool);
 
   // We use 'Base' instead of 'struct Base'.
   typedef std::shared_ptr<Base> BasePtr;
   BasePtr StructType = BasePtr(new Base);
 // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_shared instead
 // CHECK-FIXES: BasePtr StructType = std::make_shared<Base>();
 
 #define PTR shared_ptr<int>
-  std::shared_ptr<int> Macro = std::PTR(new int);
+  std::shared_ptr<int> Macro = std::PTR(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_shared instead
   // CHECK-FIXES: std::shared_ptr<int> Macro = std::make_shared<int>();
+  std::shared_ptr<int> Macro2 = std::PTR(new int);
 #undef PTR
 
-  std::shared_ptr<int> Using = shared_ptr_<int>(new int);
+  std::shared_ptr<int> Using = shared_ptr_<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_shared instead
   // CHECK-FIXES: std::shared_ptr<int> Using = std::make_shared<int>();
+  std::shared_ptr<int> Using2 = shared_ptr_<int>(new int);
 }
 
 void whitespaces() {
   // clang-format off
   auto Space = std::shared_ptr <int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_shared instead
   // CHECK-FIXES: auto Space = std::make_shared<int>();
+  auto Space2 = std::shared_ptr <int>(new int);
 
   auto Spaces = std  ::    shared_ptr  <int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_shared instead
   // CHECK-FIXES: auto Spaces = std::make_shared<int>();
+  auto Spaces2 = std  ::    shared_ptr  <int>(new int);
   // clang-format on
 }
 
 void nesting() {
   auto Nest = std::shared_ptr<std::shared_ptr<int>>(new std::shared_ptr<int>(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use std::make_shared instead
   // CHECK-FIXES: auto Nest = std::make_shared<std::shared_ptr<int>>(new int);
   Nest.reset(new std::shared_ptr<int>(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_shared instead
   // CHECK-FIXES: Nest = std::make_shared<std::shared_ptr<int>>(new int);
-  Nest->reset(new int);
+  Nest->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_shared instead
   // CHECK-FIXES: *Nest = std::make_shared<int>();
+  Nest->reset(new int);
 }
 
 void reset() {
   std::shared_ptr<int> P;
   P.reset();
   P.reset(nullptr);
   P.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_shared instead
   // CHECK-FIXES: P = std::make_shared<int>();
+  P.reset(new int);
 
   auto Q = &P;
   Q->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead
   // CHECK-FIXES: *Q = std::make_shared<int>();
+  Q->reset(new int);
 }

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

+// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-make-unique %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [{key: modernize-make-unique.IgnoreDefaultInitialization, \
+// RUN:               value: 'false'}] \
+// RUN:             }" \
+// RUN:   -- -I %S/Inputs/modernize-smart-ptr
+
+#include "initializer_list.h"
+#include "unique_ptr.h"
+// CHECK-FIXES: #include <memory>
+
+void basic() {
+  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>();
+  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>();
+
+  P1.reset(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P1 = std::make_unique<int>();
+  P2.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P2 = std::make_unique<int>();
+
+  P1 = std::unique_ptr<int>(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P1 = std::make_unique<int>();
+  P2 = std::unique_ptr<int>(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: 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>();
+  auto P4 = std::unique_ptr<int>(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+  // CHECK-FIXES: auto P4 = std::make_unique<int>();
+
+  std::unique_ptr<int> P5 = 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> P5 = std::make_unique<int>();
+  std::unique_ptr<int> P6 = 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> P6 = std::make_unique<int>();
+
+  P5.reset((new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P5 = std::make_unique<int>();
+  P6.reset((new int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P6 = std::make_unique<int>();
+
+  std::unique_ptr<int[]> P7, P8;
+  P7.reset(new int[5]());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P7 = std::make_unique<int[]>(5);
+
+  P8.reset(new int[5]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P8 = std::make_unique<int[]>(5);
+
+  int Num = 3;
+  P7.reset(new int[Num]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P7 = std::make_unique<int[]>(Num);
+
+  P8.reset(new int[Num]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
+  // CHECK-FIXES: P8 = std::make_unique<int[]>(Num);
+}

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

 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>();
 }
 
+std::unique_ptr<Base> getPointerValue() {
+  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 basic() {
   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>();
+  std::unique_ptr<int> P2 = std::unique_ptr<int>(new int);
 
   P1.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique<int>();
+  P2.reset(new int);
 
   P1 = std::unique_ptr<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique<int>();
+  P1 = std::unique_ptr<int>(new 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>();
+  // Without parenthesis, default initialization.
+  std::unique_ptr<int> P3 = std::unique_ptr<int>(new int);
 
   P2.reset(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique<int>();
 
   P2 = std::unique_ptr<int>(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique<int>();
 
   // With auto.
-  auto P3 = std::unique_ptr<int>(new int());
+  auto P4 = 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>();
+  // CHECK-FIXES: auto P4 = std::make_unique<int>();
+  auto P5 = std::unique_ptr<int>(new int);
 
-  std::unique_ptr<int> P4 = std::unique_ptr<int>((new int));
+  std::unique_ptr<int> P6 = 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> P4 = std::make_unique<int>();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr<int> P6 = std::make_unique<int>();
+  std::unique_ptr<int> P7 = std::unique_ptr<int>((new int));
+
+  P4.reset((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P4 = std::make_unique<int>();
-  std::unique_ptr<int> P5 = std::unique_ptr<int>((((new int))));
+  P5.reset((new int));
+
+  std::unique_ptr<int> P8 = 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> P5 = std::make_unique<int>();
-  P5.reset(((((new int)))));
+  // CHECK-FIXES: std::unique_ptr<int> P8 = std::make_unique<int>();
+  std::unique_ptr<int> P9 = std::unique_ptr<int>((((new int))));
+
+  P5.reset(((((new int())))));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P5 = std::make_unique<int>();
+  P6.reset(((((new 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>();
+    unique_ptr<int> P = unique_ptr<int>(new int);
 
     Q = unique_ptr<int>(new int());
     // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
     // CHECK-FIXES: Q = std::make_unique<int>();
+
+    P = unique_ptr<int>(new int);
   }
 
   std::unique_ptr<int> R(new int());
+  std::unique_ptr<int> S(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>());
+  T = g(std::unique_ptr<int>(new int));
 
   // Only replace if the type in the template is the same as the type returned
   // by the new operator.
   auto Pderived = std::unique_ptr<Base>(new Derived());
+  auto PderivedNoparen = std::unique_ptr<Base>(new Derived);
 
   // OK to replace for reset and assign
   Pderived.reset(new Derived());
   // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_unique instead
   // CHECK-FIXES: Pderived = std::make_unique<Derived>();
+  PderivedNoparen.reset(new Derived);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use std::make_unique instead
+  // CHECK-FIXES: PderivedNoparen = std::make_unique<Derived>();
 
   Pderived = std::unique_ptr<Derived>(new Derived());
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use std::make_unique instead
   // CHECK-FIXES: Pderived = std::make_unique<Derived>();
+  PderivedNoparen = std::unique_ptr<Derived>(new Derived);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use std::make_unique instead
+  // CHECK-FIXES: PderivedNoparen = std::make_unique<Derived>();
 
   // FIXME: OK to replace if assigned to unique_ptr<Base>
   Pderived = std::unique_ptr<Base>(new Derived());
+  Pderived = std::unique_ptr<Base>(new Derived);
 
   // FIXME: OK to replace when auto is not used
   std::unique_ptr<Base> PBase = std::unique_ptr<Base>(new Derived());
+  std::unique_ptr<Base> PBaseNoparen = std::unique_ptr<Base>(new Derived);
 
   // The pointer is returned by the function, nothing to do.
   std::unique_ptr<Base> RetPtr = getPointer();
+  RetPtr = getPointerValue();
 
   // This emulates std::move.
   std::unique_ptr<int> Move = static_cast<std::unique_ptr<int> &&>(P1);
 
   // Placement arguments should not be removed.
   int *PInt = new int;
   std::unique_ptr<int> Placement = std::unique_ptr<int>(new (PInt) int{3});
   Placement.reset(new (PInt) int{3});
   Placement = std::unique_ptr<int>(new (PInt) int{3});
+
+  std::unique_ptr<int> PlacementNoparen = std::unique_ptr<int>(new (PInt) int);
+  PlacementNoparen.reset(new (PInt) int);
+  PlacementNoparen = std::unique_ptr<int>(new (PInt) int);
 }
 
 // Calling make_smart_ptr from within a member function of a type with a
 // private or protected constructor would be ill-formed.
 class Private {
 private:
   Private(int z) {}
 
void initialization(int T, Base b) {
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning:
   // CHECK-FIXES: FFs = std::make_unique<Foo[]>(Num);
   int Num2 = 1;
   FFs.reset(new Foo[Num2]);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning:
   // CHECK-FIXES: FFs = std::make_unique<Foo[]>(Num2);
 
   std::unique_ptr<int[]> FI;
-  FI.reset(new int[5]()); // default initialization.
+  FI.reset(new int[5]()); // value initialization.
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
   // CHECK-FIXES: FI = std::make_unique<int[]>(5);
 
   // The check doesn't give warnings and fixes for cases where the original new
-  // expression doesn't do any initialization.
+  // expression does default initialization.
   FI.reset(new int[5]);
   FI.reset(new int[Num]);
   FI.reset(new int[Num2]);
 }
 
 void aliases() {
   typedef std::unique_ptr<int> IntPtr;
-  IntPtr Typedef = IntPtr(new int);
+  IntPtr Typedef = IntPtr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead
   // CHECK-FIXES: IntPtr Typedef = std::make_unique<int>();
+  IntPtr Typedef2 = IntPtr(new int);
 
   // We use 'bool' instead of '_Bool'.
   typedef std::unique_ptr<bool> BoolPtr;
-  BoolPtr BoolType = BoolPtr(new bool);
+  BoolPtr BoolType = BoolPtr(new bool());
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use std::make_unique instead
   // CHECK-FIXES: BoolPtr BoolType = std::make_unique<bool>();
+  BoolPtr BoolType2 = BoolPtr(new bool);
 
   // We use 'Base' instead of 'struct Base'.
   typedef std::unique_ptr<Base> BasePtr;
   BasePtr StructType = BasePtr(new Base);
 // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use std::make_unique instead
 // CHECK-FIXES: BasePtr StructType = std::make_unique<Base>();
 
 #define PTR unique_ptr<int>
-  std::unique_ptr<int> Macro = std::PTR(new 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>();
+  std::unique_ptr<int> Macro2 = std::PTR(new int);
 #undef PTR
 
-  std::unique_ptr<int> Using = unique_ptr_<int>(new int);
+  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>();
+  std::unique_ptr<int> Using2 = unique_ptr_<int>(new int);
 }
 
 void whitespaces() {
   // clang-format off
   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 Space2 = std::unique_ptr <int>(new 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>();
+  auto Spaces2 = std  ::    unique_ptr  <int>(new int);
   // clang-format on
 }
 
 void nesting() {
   auto Nest = std::unique_ptr<std::unique_ptr<int>>(new std::unique_ptr<int>(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use std::make_unique instead
   // CHECK-FIXES: auto Nest = std::make_unique<std::unique_ptr<int>>(new int);
   Nest.reset(new std::unique_ptr<int>(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead
   // CHECK-FIXES: Nest = std::make_unique<std::unique_ptr<int>>(new int);
-  Nest->reset(new int);
+  Nest->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
   // CHECK-FIXES: *Nest = std::make_unique<int>();
 }
 
 void reset() {
   std::unique_ptr<int> P;
   P.reset();
   P.reset(nullptr);
   P.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead
   // CHECK-FIXES: P = std::make_unique<int>();
+  P.reset(new int);
 
   auto Q = &P;
   Q->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead
   // CHECK-FIXES: *Q = std::make_unique<int>();
+  Q->reset(new int);
 }
 
 #define DEFINE(...) __VA_ARGS__
 template<typename T>
 void g2(std::unique_ptr<Foo> *t) {
   DEFINE(auto p = std::unique_ptr<Foo>(new Foo); t->reset(new Foo););
 }
 void macro() {
   std::unique_ptr<Foo> *t;
   g2<bar::Bar>(t);
 }
 #undef DEFINE
 
 class UniqueFoo : public std::unique_ptr<Foo> {
  public:
   void foo() {
     reset(new Foo);
     this->reset(new Foo);
     // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use std::make_unique instead
     // CHECK-FIXES: *this = std::make_unique<Foo>();
+    this->reset(new Foo());
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use std::make_unique instead
+    // CHECK-FIXES: *this = std::make_unique<Foo>();
     (*this).reset(new Foo);
     // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
     // CHECK-FIXES: (*this) = std::make_unique<Foo>();
+    (*this).reset(new Foo());
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+    // CHECK-FIXES: (*this) = std::make_unique<Foo>();
   }
 };
 
 // Ignore statements inside a template instantiation.
 template<typename T>
 void template_fun(T* t) {
   std::unique_ptr<T> t2 = std::unique_ptr<T>(new T);
+  std::unique_ptr<T> t3 = std::unique_ptr<T>(new T());
   t2.reset(new T);
+  t3.reset(new T());
 }
 
 void invoke_template() {
   Foo* foo;
   template_fun(foo);
 }
 
 void no_fix_for_invalid_new_loc() {
   // FIXME: Although the code is valid, the end location of `new struct Base` is
   // invalid. Correct it once https://bugs.llvm.org/show_bug.cgi?id=35952 is
   // fixed.
   auto T = std::unique_ptr<Base>(new struct Base);
   // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use std::make_unique instead
   // CHECK-FIXES: auto T = std::unique_ptr<Base>(new struct Base);
 }