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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
