This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix a crash in modernize-loop-convert around conversion operators
ClosedPublic

Authored by Szelethus on Nov 4 2021, 9:56 AM.

Details

Summary

modernize-loop-convert checks and fixes when a loop that iterates over the elements of a container can be rewritten from a for(...; ...; ...) style into the "new" C++11 for-range format. For that, it needs to parse the elements of that loop, like its init-statement, such as ItType it = cont.begin(). modernize-loop-convert checks whether the loop variable is initialized by a begin() member function.

When an iterator is initialized with a conversion operator (e.g. for (const_iterator it = non_const_container.begin(); ...), attempts to retrieve the name of the initializer expression resulted in an assert, as conversion operators don't have a valid IdentifierInfo.

I fixed this by making digThroughConstructors dig through conversion operators as well.

Diff Detail

Event Timeline

Szelethus created this revision.Nov 4 2021, 9:56 AM
Szelethus requested review of this revision.Nov 4 2021, 9:56 AM
Szelethus updated this revision to Diff 385459.Nov 8 2021, 5:20 AM
Szelethus edited the summary of this revision. (Show Details)

Clarify the summary.
Delete unnecessary includes.
More fitting iterator names in the test files.

steakhal accepted this revision.Nov 8 2021, 6:13 AM

Looks great.

clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
154–155

I think it's called conversion member functions.

clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
278
This revision is now accepted and ready to land.Nov 8 2021, 6:13 AM

I'll intend to land this by friday unless there are objections!

Please wait for @aaron.ballman approval.

Eugene.Zelenko set the repository for this revision to rG LLVM Github Monorepo.
aaron.ballman accepted this revision.Nov 11 2021, 6:59 AM

LGTM modulo comments from @steakhal!

whisperity added inline comments.Nov 15 2021, 1:57 AM
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
154–168

Perhaps it would be nice if an example was added to the comment here which involves said conversion.

clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
278

This is not specific to Qt, the GUI framework, right?