[clang-tidy] Omit std::make_unique/make_shared for default initialization.
authorChris Kennelly <ckennelly@ckennelly.com>
Thu, 29 Oct 2020 02:45:09 +0000 (22:45 -0400)
committerChris Kennelly <ckennelly@ckennelly.com>
Tue, 8 Dec 2020 15:34:17 +0000 (10:34 -0500)
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.

Reviewed By: hokein

Differential Revision: https://reviews.llvm.org/D90392

clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-default-init.cpp [new file with mode: 0644]
clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp

index c00067f..a7aaca1 100644 (file)
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "../utils/TypeTraits.h"
 #include "MakeSharedCheck.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
@@ -49,13 +50,17 @@ MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
           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(
@@ -120,14 +125,18 @@ void MakeSmartPtrCheck::check(const MatchFinder::MatchResult &Result) {
   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);
index 7a1bba6..ca12d77 100644 (file)
@@ -50,6 +50,7 @@ private:
   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,
index b0935fa..9c1fcea 100644 (file)
@@ -48,3 +48,9 @@ Options
 
    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`.
index 80c1b3b..cd474d3 100644 (file)
@@ -48,3 +48,9 @@ Options
 
    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`.
index e046801..21f9cb1 100644 (file)
@@ -38,48 +38,61 @@ std::shared_ptr<Base> getPointer() {
   // 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.
@@ -87,40 +100,54 @@ void basic() {
     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);
@@ -130,6 +157,10 @@ void basic() {
   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
@@ -231,15 +262,17 @@ void initialization(int T, Base b) {
 
 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;
@@ -248,14 +281,16 @@ void aliases() {
 // CHECK-FIXES: BasePtr StructType = std::make_shared<Base>();
 
 #define PTR shared_ptr<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> 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() {
@@ -263,10 +298,12 @@ void whitespaces() {
   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
 }
 
@@ -277,9 +314,10 @@ void nesting() {
   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() {
@@ -289,9 +327,11 @@ void reset() {
   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);
 }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-default-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-default-init.cpp
new file mode 100644 (file)
index 0000000..2d79e70
--- /dev/null
@@ -0,0 +1,73 @@
+// 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);
+}
index 16cb522..f4241be 100644 (file)
@@ -83,49 +83,60 @@ std::unique_ptr<Base> getPointer() {
   // 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.
@@ -133,40 +144,55 @@ void basic() {
     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);
@@ -176,6 +202,10 @@ void basic() {
   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
@@ -446,12 +476,12 @@ void initialization(int T, Base b) {
   // 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]);
@@ -459,15 +489,17 @@ void initialization(int T, Base b) {
 
 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;
@@ -476,14 +508,16 @@ void aliases() {
 // CHECK-FIXES: BasePtr StructType = std::make_unique<Base>();
 
 #define PTR unique_ptr<int>
-  std::unique_ptr<int> Macro = std::PTR(new int);
-// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
-// CHECK-FIXES: std::unique_ptr<int> Macro = std::make_unique<int>();
+  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() {
@@ -491,10 +525,12 @@ void whitespaces() {
   auto Space = std::unique_ptr <int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead
   // CHECK-FIXES: auto Space = std::make_unique<int>();
+  auto 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
 }
 
@@ -505,7 +541,7 @@ void nesting() {
   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>();
 }
@@ -517,11 +553,13 @@ void reset() {
   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__
@@ -542,9 +580,15 @@ class UniqueFoo : public std::unique_ptr<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>();
   }
 };
 
@@ -552,7 +596,9 @@ class UniqueFoo : public std::unique_ptr<Foo> {
 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() {