Page MenuHomePhabricator

[clang-tidy] Added option to uniqueptr delete release check
ClosedPublic

Authored by njames93 on Feb 28 2021, 8:04 AM.

Details

Summary

Adds an option, PreferResetCall, currently defaulted to false, to the check.
When true the check will refactor by calling the reset member function.

Diff Detail

Event Timeline

njames93 created this revision.Feb 28 2021, 8:04 AM
njames93 requested review of this revision.Feb 28 2021, 8:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2021, 8:04 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
njames93 updated this revision to Diff 326972.Feb 28 2021, 8:29 AM

Clean up some fix-it logic

Eugene.Zelenko added inline comments.Feb 28 2021, 8:45 AM
clang-tools-extra/docs/clang-tidy/checks/readability-uniqueptr-delete-release.rst
24

Please use single back-ticks for option values. Same below.

Eugene.Zelenko added a project: Restricted Project.Feb 28 2021, 8:45 AM

@Eugene.Zelenko I think there should be a herald rule for marking projects as clang-tools-extra automatically

njames93 updated this revision to Diff 326977.Feb 28 2021, 9:37 AM
njames93 marked an inline comment as done.

Use single back ticks for option values.

njames93 updated this revision to Diff 327077.Mar 1 2021, 4:10 AM

Add tests to ensure Parens are handled correctly.
Add support for pointers to and smart pointers to unique_ptr objects.

aaron.ballman accepted this revision.Mar 1 2021, 12:26 PM

LGTM!

clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
40

Not that I expect this to come up particularly often, but, how about: delete (P2.release)();

This revision is now accepted and ready to land.Mar 1 2021, 12:26 PM
njames93 added inline comments.Mar 1 2021, 12:45 PM
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
40

I don't think Im gonna worry about those given how infrequently bound member functions are used.

aaron.ballman added inline comments.Mar 1 2021, 12:56 PM
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
40

I'm fine with that, but it does come up from people who loathe ADL shenanigans. That said, if it comes up *here*, I'd be surprised.

njames93 added inline comments.Mar 1 2021, 1:09 PM
clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp
40

In its current config it wont detect it at all. I got it to detect bound member functions but the fix-its never seemed to work properly. I've just left it as it. so no warning or (corrupted fix-it).

njames93 updated this revision to Diff 327258.Mar 1 2021, 1:13 PM

Add test demonstrating we aren't detecting bound member functions.

This revision was landed with ongoing or failed builds.Mar 1 2021, 1:52 PM
This revision was automatically updated to reflect the committed changes.