This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Refactor loop-convert to bring most of the checking into matchers
Needs ReviewPublic

Authored by njames93 on Mar 4 2021, 8:20 AM.

Details

Summary

Move all checking on the for loop declaration into the matchers.
Once a match is generated clients only need to examine the body of the for loop to ensure its convertible.
This also brings the handling of match results for Iterator and PseudoArray loops much more inline.

This supercedes D97889

Diff Detail

Event Timeline

njames93 created this revision.Mar 4 2021, 8:20 AM
njames93 requested review of this revision.Mar 4 2021, 8:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2021, 8:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Good progress! This check also implements its own StmtAncestorASTVisitor which should probably be removed in favor of the ParentMapContext eventually (not in this patch).

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

There's already a (deprecated) hasDefaultArgument in ASTMatchers.h with the same implementation.

It is deprecated in favor of hasInitializer.

152

Why not use lambdas?

236

It always makes sense to use auto for matchers. Here, you hide an unnecessary implicit conversion from BindableMatcher<Stmt> to Matcher<Stmt>.

Below you use auto for a matcher. Why be inconsistent?

859

Why change auto to VarDecl (repeated) here?

njames93 marked 3 inline comments as done.Mar 10 2021, 1:48 PM
njames93 added inline comments.
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
859

Changed it when I removed the initializer, forget to undo it when I added the initializer back.

njames93 updated this revision to Diff 329765.Mar 10 2021, 1:48 PM
njames93 marked an inline comment as done.

Update.

Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 5:31 AM
njames93 updated this revision to Diff 341350.Apr 28 2021, 4:27 PM

Remove unnecessary test case.