-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Description
As per @vbvictor 's suggestion here, I'd like to propose a new clang-tidy check, bugprone-loop-variable-copied-then-mutated
.
If enabled, this check would warn the programmer when they've implicitly copied a loop variable in each iteration, and then subsequently modify that loop variable in the body of the loop.
This is a bugprone pattern because often the programmer is unaware the variable is being copied implicitly, and thus does not realize they're modifying a copy rather than the underlying value.
For instance, see this bugfix, in which the code was intended to null out the entries it was iterating through, but was not in fact doing that.
If the copy is in fact intentional, the warning can be suppressed by explicitly copying the variable in the body of the loop, rather than in the loop declaration. Essentially this warning would force developers to be explicit when they want to copy the loop variable per iteration, but would not prevent them from doing so.
There is already a performance-for-range-copy
check that addresses unintentional copies in cases where performance may be impacted, but that check would not warn on cases like the one in the linked example. Abugprone-loop-variable-copied-then-mutated
check would warn on that case.
An initial attempt at implementing this can be found in this PR, but note that that PR has been closed out as it implements the check by adding it as an option to performance-for-range-check
. Based on feedback, I think it makes more sense to have this as a separate check (because it is a bugprone concern rather than a performance concern), hence my opening this issue, but I'll have a new PR ready shortly pending whatever feedback I get here.