Rust 2024 match ergonomics and `needless_borrowed_reference`

[traviscross]

In Rust 2024, we're making some changes to match ergonomics. Those changes are:

• Rule 1C: When the DBM (default binding mode) is not move (whether or not behind a reference), writing mut, ref, or ref mut on a binding is an error.
• Rule 2C: Reference patterns can only match against references in the scrutinee when the DBM is move.

The net result of this is that we're pushing some code to use a fully-explicit match rather than using match ergonomics, and this is what the edition migration lints do.

Here's where clippy comes in. Consider:

let [ref _x] = &[0u8]; // _x: &u8

This is accepted in Rust 2021 and clippy does not warn about it even though that ref in the pattern is redundant. We'll be rejecting this code in Rust 2024 under Rule 1C, and the migration lints will move it to the fully-explicit form:

fn main() {
    let &[ref _x] = &[0u8]; // _x: &u8
    //[clippy]~^ WARN dereferencing a slice pattern where every element takes a reference
    //[clippy]~| NOTE `#[warn(clippy::needless_borrowed_reference)]` on by default
}

However, as noted above, clippy lints on that.

warning: dereferencing a slice pattern where every element takes a reference
 --> src/main.rs:2:9
  |
2 |     let &[ref _x] = &[0u8]; // _x: &u8
  |         ^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
  = note: `#[warn(clippy::needless_borrowed_reference)]` on by default
help: try removing the `&` and `ref` parts
  |
2 -     let &[ref _x] = &[0u8]; // _x: &u8
2 +     let [_x] = &[0u8]; // _x: &u8
  |

The suggestion given in this case is accurate, but it may be a bit unfortunate for clippy to lint against how rustc suggested rewriting the code. It's not practical for rustc to be more clever here; we really just want to migrate these cases to the corresponding fully-explicit form.

With my edition hat on, in the interest of making the adoption of Rust 2024 the most seamless for the most people, I'd probably prefer that this lint be made allow-by-default by clippy before the release of Rust 2024, but I'm interested to hear what the @rust-lang/clippy team thinks about this and what options there may be here.

Tracking:

• Tracking issue for Rust 2024: Match ergonomics rules 1C/2C rust#131414

Other open issues about this lint:

• needless_borrowed_reference is too opinionated #10245
• needless_borrowed_reference for lambdas not immediately typed #11566
• needless_borrowed_reference makes invalid suggestions #13035

cc @Nadrieril @joshtriplett

[Centri3]

If I understand, the main concern is that this contradicts the change made by rustc before, and not the suggestion itself? Is there a reason to use the code rustc suggests over clippy's?

[Manishearth]

Yeah, I don't actually consider this as a case of the lints contradicting, the lint is just refining.

Previous editions had this as a first class concept: the 2018 edition design had "idiom lints", where the edition lints would transform 2015 code to code that compiles on both editions (this was really useful during migration if the lints weren't perfect, you don't want your codebase in a situation where you can't run some compiler to get lint output. You also want your migration lints to not be too complicated). A subsequent set of "edition idiom" lints would "clean it up" to be more idiomatic (but potentially no longer working on 2015).

I don't think such a multi step refinement process causes any problems. It is already the case that a compiler upgrade will often cause new code to be linted with Clippy, this is just a different kind of compiler upgrade.

This particular lint is not one I feel strongly about, but I don't think the justification provided here rises to the level of downgrading this lint.

[flip1995]

Oh, I missed that the Clippy suggestion is correct, when skimming the issue earlier:

2 -     let &[ref _x] = &[0u8]; // _x: &u8
2 +     let [_x] = &[0u8]; // _x: &u8

In that case, I would say this is already the behavior we want. Clippy lints also sometimes suggest code, that is then linted by another Clippy lint, so it is the same multi step refinement process.

I agree with Manish, that this is not a reason for downgrading the lint, but rather a lucky coincidence, that Clippy can help improving the code that the rust edition lints suggest.

[traviscross]

Another option to consider would be to have clippy start to lint against this:

let [ref _x] = &[0u8]; // _x: &u8

The ref there is redundant in all editions and can be removed.

That way, with respect to the edition migration, clippy would be linting on the "before" code as well as on the "after" code.

[Manishearth]

Clippy lints also sometimes suggest code, that is then linted by another Clippy lint, so it is the same multi step refinement process.

Yeah, that's a good point: This sort of multi step refinement is standard in the Clippy world, we try to produce lint free suggestions but it's not a strong attempt. This isn't considered a bug in the first lint.

That way, with respect to the edition migration, clippy would be linting on the "before" code as well as on the "after" code.

That would be acceptable I think.

[Centri3]

Another option to consider would be to have clippy start to lint against this:

Agree with this solution