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

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 31 09:06:13 PDT 2019


gribozavr added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:311
+          }
+        }
+        if (CEArg->isStdInitListInitialization())
----------------
So `Foo(Bar{1, 2})` is not problematic, it is just that we can't tell it apart from `Foo({1,2})`?


================
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));
----------------
hokein wrote:
> 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.
Sorry I don't understand -- so what the file used to say, `std::make_unique<J>(E{1, 2}, 1)`, is correct, and this patch introduces a regression?  Or is some other syntax necessary?

Please add FIXMEs anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736





More information about the cfe-commits mailing list