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

[Chris Kennelly via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d2c095e5a6b: [clang-tidy] Omit std::make_unique/make_shared 
for default initialization. (authored by ckennelly).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

Files:
  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
  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
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(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();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(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();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew 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();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
  

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

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

@njames93: Any additional comments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 309936.
ckennelly edited the summary of this revision.
ckennelly added a comment.

updating commit message


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

Files:
  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
  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
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(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();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(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();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew 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();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
   // Create the unique_ptr as a parameter to a function.
   int T = g(std::unique_ptr(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: int T

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

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 309934.
ckennelly added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

Files:
  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
  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
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(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();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(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();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew 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();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
   // Create the unique_ptr as a parameter to a function.
   int T = g(std::unique_ptr(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: int T = g(std::make_unique());
+  T = g(std::unique_ptr(new int));
 

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

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 306302.
ckennelly added a comment.

Added option to allow rewrites for default to value initialization


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

Files:
  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
  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
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(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();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(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();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew 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();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
   // Create the unique_ptr as a parameter to a function.
   int T = g(std::unique_ptr(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: int T = g

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

njames93: Ping




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp:51
   // CHECK-FIXES: std::shared_ptr P1 = std::make_shared();
+  std::shared_ptr P2 = std::shared_ptr(new int);
 

steveire wrote:
> ckennelly wrote:
> > steveire wrote:
> > > I'm a bit confused. Why don't we want to transform this?
> > `std::make_shared()` is equivalent to `std::shared_ptr(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.
> 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).
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.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp:51
   // CHECK-FIXES: std::shared_ptr P1 = std::make_shared();
+  std::shared_ptr P2 = std::shared_ptr(new int);
 

ckennelly wrote:
> steveire wrote:
> > I'm a bit confused. Why don't we want to transform this?
> `std::make_shared()` is equivalent to `std::shared_ptr(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.
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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp:51
   // CHECK-FIXES: std::shared_ptr P1 = std::make_shared();
+  std::shared_ptr P2 = std::shared_ptr(new int);
 

steveire wrote:
> I'm a bit confused. Why don't we want to transform this?
`std::make_shared()` is equivalent to `std::shared_ptr(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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp:51
   // CHECK-FIXES: std::shared_ptr P1 = std::make_shared();
+  std::shared_ptr P2 = std::shared_ptr(new int);
 

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 302731.
ckennelly added a comment.

Expand tests to cover value and default initialization


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

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
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(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();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(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();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew 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();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
   // Create the unique_ptr as a parameter to a function.
   int T = g(std::unique_ptr(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: int T = g(std::make_unique());
+  T = g(std::unique_ptr(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(new Derived());
+  auto PderivedNoparen = std::unique_ptr(new Derived);
 
   // OK to replace 

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

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(new int)` with 
`std::make_unique()` 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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 301649.
ckennelly marked 3 inline comments as done.
ckennelly added a comment.

Apply review feedback from njames93


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

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();
 
-  // Without parenthesis.
+  // Without parenthesis, default initialization.
   std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
 
   P2.reset(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
   auto P3 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: auto P3 = std::make_unique();
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P4 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  std::unique_ptr P5 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  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();
 
@@ -137,6 +131,8 @@
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+Q = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
@@ -446,12 +442,12 @@
   // CHECK-FIXES: FFs = std::make_unique(Num2);
 
   std::unique_ptr 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(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 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();
 
   // We use 'bool' instead of '_Bool'.
   typedef std::unique_ptr 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();
 
@@ -476,12 +472,12 @@
 // CHECK-FIXES: BasePtr StructType = std::make_unique();
 
 #define PTR unique_ptr
-  std::unique_ptr Macro = std::PTR(new int);
+  std::unique_ptr Macro = std::PTR(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
 // CHECK-FIXES: std::unique_ptr Macro = std::make_unique();
 #undef PTR
 
-  std::unique_ptr Using = unique_ptr_(new int);
+  std::unique_ptr Using = unique_ptr_(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr Using = std::make_unique();
 }
@@ -505,7 +501,7 @@
   Nest.reset(new std::unique_ptr(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead
   // CHECK-FIXES: Nest = std::make_unique>(new int);
-  Nest->reset(new int);
+  Nest->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warn

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

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 301648.
ckennelly added a comment.

Apply feedback from ymandel


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

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();
 
-  // Without parenthesis.
+  // Without parenthesis, default initialization.
   std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
 
   P2.reset(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
   auto P3 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: auto P3 = std::make_unique();
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P4 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  std::unique_ptr P5 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  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();
 
@@ -137,6 +131,8 @@
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+Q = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
@@ -446,12 +442,12 @@
   // CHECK-FIXES: FFs = std::make_unique(Num2);
 
   std::unique_ptr 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(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 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();
 
   // We use 'bool' instead of '_Bool'.
   typedef std::unique_ptr 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();
 
@@ -476,12 +472,12 @@
 // CHECK-FIXES: BasePtr StructType = std::make_unique();
 
 #define PTR unique_ptr
-  std::unique_ptr Macro = std::PTR(new int);
+  std::unique_ptr Macro = std::PTR(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
 // CHECK-FIXES: std::unique_ptr Macro = std::make_unique();
 #undef PTR
 
-  std::unique_ptr Using = unique_ptr_(new int);
+  std::unique_ptr Using = unique_ptr_(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr Using = std::make_unique();
 }
@@ -505,7 +501,7 @@
   Nest.reset(new std::unique_ptr(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead
   // CHECK-FIXES: Nest = std::make_unique>(new int);
-  Nest->reset(new int);
+  Nest->reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
   // CHECK-FIXES:

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

[Nathan James via Phabricator via cfe-commits]

njames93 requested changes to this revision.
njames93 added a comment.
This revision now requires changes to proceed.

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.




Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:132
   // which maybe unexpected and cause performance regression.
-  if (New->isArray() && !New->hasInitializer())
+  const bool Initializes = New->hasInitializer() ||
+   
!utils::type_traits::isTriviallyDefaultConstructible(

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:135
+   New->getAllocatedType(), *Result.Context);
+  if (!Initializes) {
 return;

No braces around the body.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp:54
 
   // Without parenthesis.
   std::shared_ptr P2 = std::shared_ptr(new int);

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90392/new/

https://reviews.llvm.org/D90392

___
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.

[Chris Kennelly via Phabricator via cfe-commits]

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();
 
-  // Without parenthesis.
+  // Without parenthesis, default initialization.
   std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
 
   P2.reset(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
   auto P3 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: auto P3 = std::make_unique();
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P4 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  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();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  std::unique_ptr P5 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  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();
 
@@ -137,6 +131,8 @@
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+Q = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
@@ -446,12 +442,12 @@
   // CHECK-FIXES: FFs = std::make_unique(Num2);
 
   std::unique_ptr 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(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 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();
 
   // We use 'bool' instead of '_Bool'.
   typedef std::unique_ptr 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();
 
@@ -476,12 +472,12 @@
 // CHECK-FIXES: BasePtr StructType = std::make_unique();
 
 #define PTR unique_ptr
-  std::unique_ptr Macro = std::PTR(new int);
+  std::unique_ptr Macro = std::PTR(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
 // CHECK-FIXES: std::unique_ptr Macro = std::make_unique();
 #undef PTR
 
-  std::unique_ptr Using = unique_ptr_(new int);
+  std::unique_ptr Using = unique_ptr_(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr Using = std::make_unique();
 }
@@ -505,7 +501,7 @@
   Nest.reset(new std::unique_ptr(new int));
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique