[PATCH] D62845: [clang-tidy] Fix make-unique tests on C++2a.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D62845#1529190 <https://reviews.llvm.org/D62845#1529190>, @gribozavr wrote:

> In D62845#1529149 <https://reviews.llvm.org/D62845#1529149>, @hokein wrote:
>
> > > 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?
>
>
> Fair enough.
>
> >> 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.
>
> Indeed, this is complicated.  Could you add tests for `new NoCopyMoveCtor{1, 2}` with TODOs (the message suggests the user to do the impossible).


Done, but note that adding extra data fields to the `NoCopyMoveCtor` is uncompilable in C++2a :(



================
Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique-cxx14.cpp:1
-// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
+// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
 
----------------
gribozavr wrote:
> WDYT about merging two tests and adding run lines like this:
> 
> ```
> // RUN: %check_clang_tidy -std=c++14,c++17 -check-suffix=CXX_14_17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr -D CXX_14_17=1
> // RUN: %check_clang_tidy -std=c++2a -check-suffix=CXX_2A %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr -DCXX_2A=1
> ```
Good point, done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62845