[PATCH] D55044: [cfe-dev] clang-tidy check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang created this revision.
axzhang added reviewers: klimek, EricWF.
axzhang added a project: clang-tools-extra.
Herald added a subscriber: mgorny.

Add a new check for https://abseil.io/tips/126, to detect uses of constructing 
a std::unique_ptr with a call to new, and recommend substituting with 
absl::make_unique or absl::WrapUnique. Note that some functionality may overlap 
with existing checks to substitute with std::make_unique, but this check 
differs in that it is specific to Abseil, and in how it utilizes both 
absl::make_unique and absl::WrapUnique.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/MakeUniqueCheck.cpp
  clang-tidy/abseil/MakeUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t
+namespace std {
+template 
+struct default_delete {};
+
+template >
+class unique_ptr {
+ public:
+  unique_ptr();
+  explicit unique_ptr(T*);
+  void reset(T*);
+};
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct B {
+  int x;
+  int y;
+};
+
+int* ReturnPointer();
+void ExpectPointer(std::unique_ptr p);
+
+std::unique_ptr MakeAndReturnPointer() {
+  // Make smart pointer in return statement
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr a(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: auto a = absl::make_unique(1);
+
+  std::unique_ptr b;
+  b.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to resetting unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: b = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr c(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: auto c = absl::make_unique(1, 2);
+
+  std::unique_ptr d;
+  d.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to resetting unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: d = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr e(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: auto e = absl::make_unique();
+
+  std::unique_ptr f;
+  f.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to resetting unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: f = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr g((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: auto g = absl::make_unique(3);
+
+  std::unique_ptr h(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: auto h = absl::make_unique(4);
+
+  std::unique_ptr i;
+  i.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to resetting unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: i = absl::make_unique(5);
+
+  std::unique_ptr j;
+  j.reset(((new int(6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::make_unique to resetting unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: j = absl::make_unique(6);
+
+  // Construct unique_ptr within a function
+  ExpectPointer(std::unique_ptr(new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer absl::make_unique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: ExpectPointer(absl::make_unique(5));
+
+  // Brace initialization
+  std::unique_ptr k(new B{1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique to constructing unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: auto k = absl::WrapUnique(1, 2);
+
+  std::unique_ptr l;
+  l.reset(new B{3, 4});
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique to resetting unique_ptr with new [abseil-make-unique]
+  // CHECK-FIXES: l = absl::WrapUnique(3, 4);
+}
+
+// Checks within namespaces
+namespace std {
+  unique_ptr k(new int(7));
+  

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 190559.
axzhang added a comment.

Apologies for the extended hiatus. I have changed the implementation to an 
alias to modernize-make-unique, and have added an extra option to 
modernize-make-unique that ignores C++11 features such as list initializations, 
to allow `absl::make_unique` to function correctly. Also, after looking more 
into `absl::WrapUnique`, it seems like this also is not really the function to 
use for brace initializations, so no extra functionality needs to be added to 
the existing modernize-make-unique check.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use 

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 190676.
axzhang added a comment.

Updated diff with full context.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+}
Index: docs/clang-tidy/checks/modernize-make-unique.rst

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 190775.
axzhang added a comment.

Make fixes to the `UseLegacyFunction` option, renaming to `IgnoreListInit`.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+}
Index: docs/clang-tidy/checks/modernize-make

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 192394.
axzhang added a comment.

Add documentation about the added option for `modernize-make-unique`.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+}
Index: docs/clang-tidy/checks/modernize-make-uniqu

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang marked an inline comment as done.
axzhang added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:94
  equalsBoundNode(PointerType),
  CanCallCtor)
   .bind(NewExpression)),

lebedev.ri wrote:
> ```
> CanCallCtor, anyOf(unless(IgnoreListInit), 
> unless(hasInitializationStyle(CXXNewExpr::ListInit
> ```
I can't compile `unless(IgnoreListInit)`, but `unless(IgnoreListInit ? 
unless(anything()) : anything())` does work. I'm not sure how idiomatic that is 
though.


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang marked 2 inline comments as done and an inline comment as not done.
axzhang added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:94
  equalsBoundNode(PointerType),
  CanCallCtor)
   .bind(NewExpression)),

axzhang wrote:
> lebedev.ri wrote:
> > ```
> > CanCallCtor, anyOf(unless(IgnoreListInit), 
> > unless(hasInitializationStyle(CXXNewExpr::ListInit
> > ```
> I can't compile `unless(IgnoreListInit)`, but `unless(IgnoreListInit ? 
> unless(anything()) : anything())` does work. I'm not sure how idiomatic that 
> is though.
Actually, `unless(anything())` also does not compile. How should I make a 
matcher that won't trigger if `IgnoreListInit` is true?


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 194435.
axzhang marked an inline comment as not done.

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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+}
Index: docs/clang-tidy/checks/modernize-make-unique.rst
=

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

If no more changes need to be made, is this okay to be merged in?


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang marked an inline comment as done.
axzhang added a comment.

I'm having some issues with the AbseilTidyModule options. I am storing 
IgnoreListInit as true for the abseil-make-unique check, but when I run the 
check it shows IgnoreListInit as being false. Any ideas why this is happening?


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang marked an inline comment as done.
axzhang added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+  IgnoreListInit(Options.get("IgnoreListInit", false)) {}
 

hintonda wrote:
> You’re setting it false here.
I thought that false was the default value if the options do not contain 
"IgnoreListInit"?


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang marked 3 inline comments as done.
axzhang added inline comments.



Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
+  IgnoreListInit(Options.get("IgnoreListInit", false)) {}
 

hintonda wrote:
> hintonda wrote:
> > axzhang wrote:
> > > hintonda wrote:
> > > > You’re setting it false here.
> > > I thought that false was the default value if the options do not contain 
> > > "IgnoreListInit"?
> > Do you have a test that passes this option?  I didn’t see one.
> I could be wrong, but when you do an Options.get(), that looks at what was 
> passed on the commandline, and if it wasn't found, it sets the default 
> provided.
Fixed by changing the Options value to "1" instead of true in the 
AbseilTidyModule


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 195791.
axzhang added a comment.

Simplify tests and fix issues with Options.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
+
+#include "unique_ptr.h"
+#include "initializer_list.h"
+// CHECK-FIXES: #include "absl/memory/memory.h"
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct B {
+  B(std::initializer_list);
+  B();
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+
+  // Ignore initializer-list constructors
+  std::unique_ptr PInit = std::unique_ptr(new B{1, 2});
+  PInit.reset(new B{1, 2});
+}
Index: docs/clang-tidy/checks/modernize-make-unique.rst
===
--- docs/clang-tidy/checks/modernize-make-unique.rst
+++ docs/clang-tidy/checks/modernize-make-unique.rst
@@ -48,3 +48,9 @@
 
If set to non-zero, the check will not give warnings inside macros. Default
is `1`.
+
+.. option:: IgnoreListInit
+   
+   If set to non-zero, the check will ignore list initializations of `new`
+   expressions. Default is `0`.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -13,6 +13,7 @@
abseil-duration-subtraction
abseil-duration-unnecessary-conversion
abseil-faster-strsplit-delimiter
+   abseil-make-unique (redirects to modernize-make-unique) 
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
Index: docs/clang-tidy/checks/abseil-make-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-make-unique.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - abseil-make-unique
+.. meta::
+   :http-equiv=refresh: 5;URL=abseil-make-unique.html
+
+abseil-make-unique
+==
+
+This check finds the creation of ``std::unique_ptr`` objects by explicitly
+calling the constructor and a ``new`` expression, and replaces it with a call
+to ``absl::make_unique``, the Abseil implementation of ``std::make_unique`` 
+in C++11.
+
+.. code-block:: c++
+
+  auto ptr = std::unique_ptr(new int(1));
+
+  // becomes
+
+  auto ptr = absl::make_unique(1);
+
+The Abseil Style Guide _ discusses this issue in
+more detail.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -85,6 +85,11 @@
   Finds and fixes cases where ``absl::Duration`` values are being converted to
   numeric types and back again.
 
+- New alias :doc:`abseil-make-unique
+  ` to :doc:`modernize-make-unique
+  `
+  added.
+
 - New :doc:`abseil-time-subtraction
   ` check.
 
@@ -104,6 +109,11 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `Commen

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

Pinging for visibility.


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

https://reviews.llvm.org/D55044



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 177972.
axzhang added a comment.

Per the suggestions of reviewers, change the check so that it uses 
modernize-make-smart-pointer instead.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/MakeUniqueCheck.cpp
  clang-tidy/abseil/MakeUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,130 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+// CHECK-FIXES: #include 
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr &t) = delete;
+  unique_ptr(unique_ptr &&t) {}
+  ~unique_ptr() {}
+  type &operator*() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr &operator=(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  // Make smart pointer in return statement
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = s

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

In D55044#1329604 , @hokein wrote:

> In fact, the existing `modernize-make-unique` can be configured to support 
> `absl::make_unique`, you'd just need to configure the check option 
> `MakeSmartPtrFunction` to `absl::make_unique` (this is what we do inside 
> google).
>
> The biggest missing part of the `modernize-make-unique` is 
> `absl::WrapUnique`, I think we should extend `MakeSmartPtrCheck` class (maybe 
> add hooks) to support it.


What is the best way to extend `MakeSmartPtrCheck`? The behavior I want to 
achieve is that `absl::WrapUnique` is suggested when brace initialization is 
used, but `absl::make_unique` is used in all other cases.


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

https://reviews.llvm.org/D55044



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