Page MenuHomePhabricator

[clang-tidy] modernize-loop-convert reverse iteration support
Needs ReviewPublic

Authored by njames93 on Jun 18 2020, 6:00 AM.

Details

Summary

Enables support for transforming loops of the form

for (auto I = Cont.rbegin(), E = Cont.rend(); I != E;++I)

This is done automatically in C++20 mode using std::ranges::reverse_view but there are options to specify a different function to reverse iterator over a container.
This is the first step, down the line I'd like to possibly extend this support for array based loops

for (unsigned I = Arr.size() - 1;I >=0;--I) Arr[I]...

Currently if you pass a reversing function with no header in the options it will just assume that the function exists, however as we have the ASTContext it may be as wise to check before applying, or at least lower the confidence level if we can't find it.

Diff Detail

Event Timeline

njames93 created this revision.Jun 18 2020, 6:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 18 2020, 6:00 AM

It'll be interesting to run improved check over LLVM code base.

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
990

I think IsEnabled should be initialized as member or in constructor body because of non-trivial logic.

996

Something need to be changed in: not, Disabling - comma to dot or capitalization.

clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
123

Please separate with empty line.

141

Please highlight ranges::reverse_view and llvm::reverse with double back-ticks.

njames93 updated this revision to Diff 271723.Jun 18 2020, 8:07 AM
njames93 marked 4 inline comments as done.

Address reviewer comments.

njames93 updated this revision to Diff 271856.Jun 18 2020, 3:01 PM
  • Change default c++20 include to ranges instead of algorithm

It'll be interesting to run improved check over LLVM code base.

I haven't tried running it over llvm, but I have had success using it with clangd in my editor, every loop I've tried seems to work a charm.

njames93 updated this revision to Diff 272030.Jun 19 2020, 5:29 AM
  • Set and store the IncludeStyle
njames93 updated this revision to Diff 272034.Jun 19 2020, 5:49 AM

Add documentation about the include style optione

Harbormaster completed remote builds in B61004: Diff 272034.
njames93 updated this revision to Diff 273913.Jun 28 2020, 2:33 AM

Rebased inline with new enum handling added in D82188

njames93 updated this revision to Diff 278398.Thu, Jul 16, 1:49 AM

Updated release notes for version bump.

njames93 updated this revision to Diff 282757.Mon, Aug 3, 4:10 PM
  • Rebased trunk
  • Cleaned up test cases
  • Added support for specifying to include as system include.