hokein added a comment.
In D62845#1528791 <https://reviews.llvm.org/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_unique.
The test file name ("modernize-make-unique-cxx14") indicates this test is for
C++14, and since we change the existing `modernize-make-unique` test (which
covers more cases) to C++14 or later, I think it is reasonable to restrict the
`cxx14` test to run only on C++14. or am I missing anything?
> 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?
sorry, the check is too complicated to catch up, the test cases are testing the
code path
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp#L405.
Unfortunately, adding a default constructor doesn't fix the problem: the AST
tree is different in C++14/17 and C++2a, which causes different behavior in the
check.
e.g. `new NoCopyMoveCtor{}`
In C++14/17, it looks like below, the check thinks it is an aggregate
initialization (with deleted copy/move constructor) and doesn't generate the
fix.
|-CXXNewExpr <line:26:3, col:22> 'NoCopyMoveCtor *' Function 0x5614cfece8f0
'operator new' 'void *(std::size_t)'
| `-InitListExpr <col:21, col:22> 'NoCopyMoveCtor'
However, in C++2a, the AST is like below, the check thinks it is a direct
initialization with default constructor, and generate the fix.
`-CXXNewExpr <line:26:3, col:22> 'NoCopyMoveCtor *' Function 0x55c9b1b24ba0
'operator new' 'void *(std::size_t)'
`-CXXConstructExpr <col:7, col:22> 'NoCopyMoveCtor' 'void () noexcept'
list zeroing
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
