Page MenuHomePhabricator

[clang-tidy] modernize-use-auto: don't remove stars by default
ClosedPublic

Authored by alexfh on Jun 2 2016, 9:22 AM.

Details

Summary

By default, modernize-use-auto check will retain stars when replacing an explicit type with auto: MyType *t = new MyType; will be changed to auto *t = new MyType;, thus resulting in more consistency with the recommendations to use auto * for iterating over pointers in range-based for loops: http://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

The new RemoveStars option allows to revert to the old behavior: with the new option turned on the check will change MyType *t = new MyType; to auto t = new MyType;.

Diff Detail

Repository
rL LLVM

Event Timeline

alexfh updated this revision to Diff 59399.Jun 2 2016, 9:22 AM
alexfh retitled this revision from to [clang-tidy] Add RemoveStars option to the modernize-use-auto check.
alexfh updated this object.
alexfh added reviewers: sbenza, aaron.ballman.
alexfh added a subscriber: cfe-commits.
sbenza edited edge metadata.Jun 2 2016, 9:35 AM

Is it a typo in the description when it says that when RemoveStar is on we will write 'auto*' instead if 'auto'?

aaron.ballman accepted this revision.Jun 2 2016, 9:48 AM
aaron.ballman edited edge metadata.

The description threw me for a loop, but the behavior described in the documentation and tested by the tests matches what I would expect. LGTM!

This revision is now accepted and ready to land.Jun 2 2016, 9:48 AM
Eugene.Zelenko added inline comments.
docs/clang-tidy/checks/modernize-use-auto.rst
155 ↗(On Diff #59399)

Please highlight 0 with `. It's not language construct.

alexfh retitled this revision from [clang-tidy] Add RemoveStars option to the modernize-use-auto check to [clang-tidy] modernize-use-auto: don't remove stars by default.Jun 3 2016, 12:30 PM
alexfh updated this object.
alexfh edited edge metadata.
alexfh updated this revision to Diff 59599.Jun 3 2016, 12:32 PM
alexfh marked an inline comment as done.
  • Updated formatting in the doc.

Is it a typo in the description when it says that when RemoveStar is on we will write 'auto*' instead if 'auto'?

Yep, the whole patch description is a typo, fixed ;)

Prazek added a subscriber: Prazek.Jun 3 2016, 12:43 PM

besides it lgtm

clang-tidy/modernize/UseAutoCheck.cpp
338 ↗(On Diff #59599)

Can you change this comment to make it more clear, or tell me what it do? I don't quite understand it right now

alexfh added inline comments.Jun 3 2016, 1:06 PM
clang-tidy/modernize/UseAutoCheck.cpp
338 ↗(On Diff #59599)

Will this be better?

// All subsequent variables in this declaration should have the same canonical type.
// For example, we don't want to use `auto` in `T *p = new T, **pp = new T*;`.
Prazek added inline comments.Jun 3 2016, 1:31 PM
clang-tidy/modernize/UseAutoCheck.cpp
338 ↗(On Diff #59599)

Much better, thanks!

This revision was automatically updated to reflect the committed changes.