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

Chris Kennelly via Phabricator via cfe-commits Thu, 29 Oct 2020 06:32:04 -0700

ckennelly created this revision.
ckennelly added reviewers: EricWF, ymandel.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
ckennelly requested review of this revision.


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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90392

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
@@ -96,34 +96,28 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique<int>();
 
-  // Without parenthesis.
+  // Without parenthesis, default initialization.
   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>();
 
   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());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: auto P3 = std::make_unique<int>();
 
-  std::unique_ptr<int> P4 = std::unique_ptr<int>((new int));
+  std::unique_ptr<int> P4 = 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));
+  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))));
+  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>();
-  P5.reset(((((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>();
 
@@ -137,6 +131,8 @@
     Q = unique_ptr<int>(new int());
     // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
     // CHECK-FIXES: Q = std::make_unique<int>();
+
+    Q = unique_ptr<int>(new int);
   }
 
   std::unique_ptr<int> R(new int());
@@ -446,12 +442,12 @@
   // 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,13 +455,13 @@
 
 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>();
 
   // 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>();
 
@@ -476,12 +472,12 @@
 // 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>();
 #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>();
 }
@@ -505,7 +501,7 @@
   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>();
 }
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
@@ -53,14 +53,12 @@
 
   // 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>();
 
-  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 = 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>();
 
@@ -69,7 +67,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_shared instead
   // CHECK-FIXES: auto P3 = std::make_shared<int>();
 
-  std::shared_ptr<int> P4 = std::shared_ptr<int>((new int));
+  std::shared_ptr<int> P4 = 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>();
 
@@ -77,7 +75,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_shared instead [modernize-make-shared]
   // CHECK-FIXES: P4 = std::make_shared<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>();
 
@@ -231,13 +229,13 @@
 
 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>();
 
   // 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>();
 
@@ -248,12 +246,12 @@
 // 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>();
 #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>();
 }
@@ -277,7 +275,7 @@
   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>();
 }
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "../utils/TypeTraits.h"
 #include "MakeSharedCheck.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
@@ -120,15 +121,20 @@
   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())
+  const bool Initializes = New->hasInitializer() ||
+                           !utils::type_traits::isTriviallyDefaultConstructible(
+                               New->getAllocatedType(), *Result.Context);
+  if (!Initializes) {
     return;
+  }
   if (Construct)
     checkConstruct(SM, Result.Context, Construct, Type, New);
   else if (Reset)

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

Reply via email to