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

2019-06-06 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL362679: [clang-tidy] Fix make-unique tests on C++2a. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

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

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Looks great, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 __

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

2019-06-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D62845#1529190 , @gribozavr wrote: > In D62845#1529149 , @hokein wrote: > > > > I think we should be looking at the intent of the test rather than its > > > name. > > > > > > The intent

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

2019-06-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 202940. hokein marked 2 inline comments as done. hokein added a comment. address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 Files: clang-tools-ex

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

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In 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 sta

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

2019-06-04 Thread Haojian Wu via Phabricator via cfe-commits
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

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

2019-06-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 202922. hokein added a comment. Tests for C++2a. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 Files: clang-tools-extra/test/clang-tidy/modernize-make-unique-cxx14.c

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

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. In D62845#1528887 , @hokein wrote: > In D62845#1528791 , @gribozavr wrote: > > > I'd suggest to add a separate file that covers the exact language modes > > needed. > > > > The C++14 test

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

2019-06-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D62845#1528791 , @gribozavr wrote: > I'd suggest to add a separate file that covers the exact language modes > needed. > > The C++14 test that we have right now is about C++14-or-later, testing the > availability of std::make_u

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

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. I'd suggest to add a separate file that covers the exact language modes needed. The C++14 test that we have right now is about C++14-or-later, testing the availability of std::make_unique. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

2019-06-04 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment. I'm also not sure what the intent behind these tests is. Maybe the right fix is to add a constructor that can be called? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62845/new/ https://reviews.llvm.org/D62845 __

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

2019-06-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: gribozavr. Herald added a subscriber: xazax.hun. Herald added a project: clang. These test cases are illgal in C++2a ("new Foo{}" needs to see the default constructor), so move them to the C++14-only tests. Repository: rG LLVM Github Monor