hokein added a comment. > I think we should be looking at the intent of the test rather than its name. > > The intent looks like testing how the check works when `std::make_unique` is > available from the standard library (as opposed to some kind of replacement > like `absl::make_unique`). See the patch that introduced it: > https://reviews.llvm.org/D43766 > > So modernize-make-unique-cxx14 is actually "C++14 or later". (Probably it > should be renamed.)
yeap, it seems to me that "modernize-make-unique-cxx14" is redundant, "modernize-make-unique" should cover what it tests, I believe. We also have "modernize-make-unique-cxx11" which runs on C++11 mode only, maybe we just repurpose the `modernize-make-unique-cxx14`, what do you think? > I see. Assuming it is desired behavior, I'd say for these cases we should > create separate files that are specifically run in C++14 and 17, and another > one for C++2a onward. > > But is it desired behavior? That is, can we generate a call to > `std::make_unique` in C++14 in practice -- would it compile? The fix is compilable for C++14, but it is tricky to support it: 1. `new NoCopyMoveCtor{}`: the `make_unique` fix is compilable 2. `new NoCopyMoveCtor{1, 2}`: the `make_unique` fix is not compilable The AST for case 1) and 2) are the same in C++14, supporting that would introduce hacky change to the logic here <https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp#L400>. I'd leave it as-is now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits