clippy::redundant_locals is not a correctness lint

[hrxi]

Even its documentation says so. According to the documentation, it might either be a "suspicious" or a "perf" lint.

changelog: [redundant_locals]: Reclassify as "suspicious" lint.

[rustbot]

r? @Centri3

rustbot has assigned @Centri3.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Centri3]

I don't particularly agree. Where does it say it in the documentation? For:

and are likely to be unintended

This kind of wording is normal for correctness lints, see for example
clippy::absurd_extreme_comparisons: "Expressions like max < x are probably mistakes.".
clippy::match_str_case_mismatch: "The arm is unreachable, which is likely a mistake".
This should have stronger wording tho. Mostly mentioning that it could be an error from refactoring.

they may affect rustc's stack allocation.

Is specifically related to cases like rust-lang/rust#112478 which isn't really relevant

In any case I'd say opening a thread on zulip is a good idea. Note this has no impact on performance so it'd only ever be suspicious.

[hrxi]

Correctness to me sounds like there's likely a mistake somewhere, i.e. that the code doesn't do what it's supposed to do. In the code fragment where the lint triggered for me, there was no mistake, it was just an unnecessary line ­— but that's not what a correctness lint is for.

What incorrect code does this lint have in mind that might be detected using this lint?

[Centri3]

correctness is defined in the readme as "code that is outright wrong or useless". This code is outright wrong and useless, while yes what it's doing wrong is a very light bug in nature (which could in the end just be an unnecessary line), I'd say because this could easily be caused by a user-error in refactoring it might be indicative of issues elsewhere too, where that assignment was supposed to do something.

This fits both correctness and suspicious, it's just whether it should deny compilation or not. See also clippy::self_assignment for a similar correctness lint

[Centri3]

Opened a thread on zulip for this: https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/Lint.20level.20for.20.60redundant_locals.60

[hrxi]

I don't feel strongly about whether it's a suspicious or perf lint, I just think that it shouldn't fail compilation outright.

Since you don't feel strongly about its classification as correctness or suspicious, let's just pick suspicious?

(I don't have a zulip account.)

[Centri3]

From the zulip thread: Ok with and in favor of suspicious for now(?) 👍 Just one change I'd recommend: while talking about stack allocations, it could mention this rarely being intentional to shorten lifetimes.

[hrxi]

Just one change I'd recommend: while talking about stack allocations, it could mention this rarely being intentional to shorten lifetimes.

I'm not sure which change you recommend me to make. I don't see anything about lifetimes in the description of the lint.

[Centri3]

It should mention this "redundant" redefinition as being intentional in these cases: https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/Lint.20level.20for.20.60redundant_locals.60/near/489074952