Page MenuHomePhabricator

[clang] New warning for for-loops where the iteration does not match the loop condition
Needs ReviewPublic

Authored by rtrieu on Jan 30 2020, 6:40 PM.

Details

Reviewers
aaron.ballman
Summary

This is a warning for when the increment/decrement in a for loop does not match the condition in the loop. If the condition has a relational operator, one operand can be deduced to be the larger value and the other operand the smaller value when the loop is run. For the loop to terminate, the smaller value needs to increase or the larger value needs to decrease, while the opposite will cause the loop to not terminate.

Correct:

for (...; x < y; ++x) {}
for (...; x < y; --y) {}

Incorrect:

for (...; x < y; --x) {}
for (...; x < y; ++y) {}

The warning comes with two attached notes. One is to flip the direction of the comparison (">" to "<", etc) and other is to change the increment decrement step ("--" to "++" or vice versa).

An exception is made for unsigned values, since some code uses the unsigned overflow/underflow characteristics.

Diff Detail

Event Timeline

rtrieu created this revision.Jan 30 2020, 6:40 PM

It's good question where this check belongs. There are two loop related checks in CLang-tidy: bugprone-infinite-loop and bugprone-too-small-loop-variable.

clang/lib/Sema/SemaStmt.cpp
1754

Please don't use auto unless type is spelled in same statement or iterator.

1760

Please use const auto *, because type is spelled in same statement.

1764

Please use const auto *, because type is spelled in same statement.

1780

Please use const auto *, because type is spelled in same statement.

Please run clang-format over patch.

1784

Please use const auto *, because type is spelled in same statement.

1813

Please don't use auto unless type is spelled in same statement or iterator.

Eugene.Zelenko retitled this revision from New warning for for-loops where the iteration does not match the loop condition to [clang] New warning for for-loops where the iteration does not match the loop condition.Jan 30 2020, 7:09 PM
Eugene.Zelenko added a reviewer: aaron.ballman.
Eugene.Zelenko set the repository for this revision to rG LLVM Github Monorepo.
Eugene.Zelenko added a project: Restricted Project.

Check should also consider cases like: i += 2; i = i + 2; i -= 2; i = i - 2. Probably same with multiplications, divisions and shifts.