Page MenuHomePhabricator

[clang-tidy] Enhance modernize-make-unique to handle unique_ptr.reset()
ClosedPublic

Authored by malcolm.parsons on Oct 22 2016, 2:46 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

malcolm.parsons retitled this revision from to [clang-tidy] Enhance modernize-make-unique to handle unique_ptr.reset().
malcolm.parsons updated this object.
malcolm.parsons added a subscriber: cfe-commits.

Correct operator signature.
Remove unused parameter.
Rename method.
Reformat.

Besides this looks good

test/clang-tidy/modernize-make-shared.cpp
122 ↗(On Diff #75541)

I think the warning here could be better. The user is using make_shared here.
Maybe ' warning: use std::make_shared with zero arguments ...', but only in this case

129 ↗(On Diff #75541)

I think it is good to replace it even with auto, like
auto PBase = std::make_shared<Base>(new Derived());

For shared_ptr we can even do better, that we can't do for unique_ptr - we
coud change it to
auto PBase = std::make_shared<Derived>();
because all conversions works.
Of course not in this patch, but it would be good to leave a comment about this here.

test/clang-tidy/modernize-make-shared.cpp
122 ↗(On Diff #75541)

The user isn't using make_shared.

129 ↗(On Diff #75541)

A smart pointer to Derived cannot be reset with a pointer to Base.

Prazek added inline comments.Oct 23 2016, 7:08 AM
test/clang-tidy/modernize-make-shared.cpp
122 ↗(On Diff #75541)

true, I've read that incorectly

129 ↗(On Diff #75541)

oh yea, I was only thinking about downcasting smart pointer

aaron.ballman added inline comments.Oct 26 2016, 1:11 PM
clang-tidy/modernize/MakeSmartPtrCheck.cpp
74 ↗(On Diff #75541)

Can elide the braces for the if and else.

147 ↗(On Diff #75541)

I kind of wonder if this should be using Twine to avoid a lot of extra allocations and copies.

150 ↗(On Diff #75541)

Elide braces

Remove braces.
Use Twine.

malcolm.parsons marked 9 inline comments as done.Oct 29 2016, 12:26 PM
aaron.ballman accepted this revision.Oct 31 2016, 8:40 AM
aaron.ballman edited edge metadata.

LGTM!

This revision is now accepted and ready to land.Oct 31 2016, 8:40 AM
malcolm.parsons edited edge metadata.

Rebase.

This revision was automatically updated to reflect the committed changes.