[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:304
+        // e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+        // an elidable move constructor.
+        if (CEArg->isElidable()) {
----------------
gribozavr wrote:
> Isn't it the other way round -- the move constructor is around the init-list constructor?
yeah, this is what I mean, updated the comment.


================
Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:1
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
 
----------------
gribozavr wrote:
> "-std=c++14-or-later" (will also cover c++2a etc.)
hmm, the test code has some compiler errors on c++2a mode,  it is a different issue (I will address it in a follow-up), added a FIXME. 


================
Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr<J> PJ2 = std::make_unique<J>(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr<J> PJ2 = std::unique_ptr<J>(new J(E{1, 2}, 1));
   PJ2.reset(new J(E{1, 2}, 1));
----------------
gribozavr wrote:
> Why do we print the warning if the fixit is not fixing anything?
We may still want to inform users, these cases can still be converted to `make_unique`, but the check currently is not smart enough to give correct fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736