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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, looks better now.
I saw there are still a few undone comments, please mark them done when you address them.



================
Comment at: docs/clang-tidy/checks/abseil-make-unique.rst:19
+
+per the Abseil tips and guidelines at https://abseil.io/tips/126.
----------------
Eugene.Zelenko wrote:
> Please use hyperlink like:
> 
> The `Abseil Style Guide <https://abseil.io/tips/126>`_ discusses this issue in more detail.
This comment is undone.


================
Comment at: docs/clang-tidy/checks/abseil-make-unique.rst:8
+
+The abseil-make-unique check is an alias, please see
+`modernize-make-unique <modernize-make-unique.html>`_ for more information.
----------------
This is not really alias, `abseil-make-unique` is different than `modernize-make-unique`, please add more details to the document here.


================
Comment at: test/clang-tidy/abseil-make-unique.cpp:9
+template <typename type, typename Deleter = std::default_delete<type>>
+class unique_ptr {
+public:
----------------
we have an implementation in Inputs/modernize-smart-ptr/unique_ptr.h, please use it.



================
Comment at: test/clang-tidy/abseil-make-unique.cpp:59
+  std::unique_ptr<int> P1 = std::unique_ptr<int>(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr<int> P1 = absl::make_unique<int>(1);
----------------
since the check shares the underlying modernize-make-unique implementation, I think there is no need to repeat most test cases here (those are covered in `test/clang-tidy/modernize-make-unique.cpp`).

I'd suggest

- add a simple test to verify this check provides abseil-related fixes (abseil header, absl::make_unique);
- adding test cases for list initialization (make sure we ignore those in this check);



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

https://reviews.llvm.org/D55044