Page MenuHomePhabricator

[clang-tidy][modernize-loop-convert] Make loop var type human readable
ClosedPublic

Authored by zinovy.nis on May 25 2020, 11:17 PM.

Details

Summary

Without this patch

for(std::set<int>::iterator it = s.begin(); ...)

is converted into

for(std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<int>>>::value_type i : s)

With this patch the output is as expected:

for(int i : s)

Related bug: https://bugs.llvm.org/show_bug.cgi?id=36688

Diff Detail

Event Timeline

zinovy.nis created this revision.May 25 2020, 11:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2020, 11:17 PM
zinovy.nis edited the summary of this revision. (Show Details)May 25 2020, 11:20 PM
Eugene.Zelenko removed a subscriber: Restricted Project.

I'm having trouble with the reproduction of this - https://godbolt.org/z/tsMfcj.
Aside from that this needs some test cases to demonstrate the patch is indeed working

zinovy.nis added a comment.EditedMay 26 2020, 11:14 PM

I'm having trouble with the reproduction of this - https://godbolt.org/z/tsMfcj.
Aside from that this needs some test cases to demonstrate the patch is indeed working

Thanks. Seems like the reproduction is c++-library dependent. Please look at https://godbolt.org/z/tbGG9b where I have manually "emulated" std::set. Or just change compiler to "clang-tidy (trunk) #1 with x86-64 clang (std::embed)" in your sample.
I get the same result (as for my custom "class set") for std::set on 'master' on Windows 10.

Aside from that this needs some test cases to demonstrate the patch is indeed working

IMHO, for testing purposes it's enough to apply my change to 'structures.h' (int -> value_type)

zinovy.nis added a comment.EditedMay 29 2020, 10:12 AM

@njames93, @aaron.ballman do you have any other questions/comments?

njames93 accepted this revision.Jun 3 2020, 4:32 AM

LGTM

This revision is now accepted and ready to land.Jun 3 2020, 4:32 AM
This revision was automatically updated to reflect the committed changes.