clarify wording of match ergonomics diagnostics (rust_2024_incompatible_pat lint and error)

[dianne]

Partially addresses #143557:

• Uses different wording than the Edition Guide chapter, to hopefully stand alone a bit better. Instead of referring to the "default binding mode", it now talks about what can't be written "within elided reference patterns". I ended up going with "elided" instead of "implicit" in hope that it reads bit less like it should behave the same as an explicit reference pattern, but I'm not totally happy with that wording.
• The explanatory note still points to where the default binding mode was introduced, but only refers to its effect, not what we call it. How that relates to the rest of the diagnostic may still be a bit of a puzzle, but hopefully it isn't too much of one? It also doesn't make sense anymore for the case of & written under a by-ref binding mode, so I've left the note out in that case (but kept the label). It's more cramped, but talking about binding modes would feel like a non-sequitur for the error about & patterns without further explanation.
• Links to the stable version of the Edition Guide instead of the nightly version. It looks like almost every link to the Edition Guide in diagnostics is to the nightly version, presumably for the same reason as here: the diagnostics were added before the new Edition was stabilized, then never updated. I'll make a separate PR to clean up the others.

This only changes the diagnostic messages, not the code suggestion or the Edition Guide.

r? @Nadrieril or reassign

[rustbot]

Nadrieril is not on the review rotation at the moment.
They may take a while to respond.

[rustbot]

Some changes occurred in match checking

cc @Nadrieril

[Nadrieril]

I'm torn about this, the "make the implicit explicit" wording has the benefit that it's a logical consequence of the error: "can't do X when there's an implicit borrow" => "make the thing explicit".

What do you think of:

error: cannot borrow again a binding that implicitly borrows its contents
...
note: matching on a reference type with a non-reference pattern implicitly borrows the contents
...
help: match on the reference explicitly to avoid the implicit borrow

And uh actually, why do we suggest adding a & pattern instead of removing the ref binding? I don't remember what led to this choice. Imo we should move users away from explicit binding modes. Even if they were doing [ref x]: &mut [T] to downgrade &mut to &, I think it's fine to suggest removing the ref. If they really want &mut [ref x] they'll know to write that, and otherwise they can often shared-bottow the scrutinee instead, or let coercions just work.

[dianne]

I like the "to avoid the implicit borrow"! I was worried adding something like that would make the help too long when refs need to be added, but I do like having this when they don't. I've pushed an update with that, though I changed it to "to avoid implicitly borrowing" to not have to worry about pluralization. When refs do need adding, I haven't found a way to make room, but the current "and borrow explicitly using a variable binding mode" message is also explicit about making the borrow explicit, so that might be sufficient.

For the "cannot borrow again a binding that implicitly borrows its contents" error message, would that be for if the only problem with the pattern is ref/ref mut under an inherited ref? Since we use a single error for all the match ergonomics problems in a pattern (so that we can give a single suggestion), I've tried to make the messages a bit more terse than is maybe ideal so that they can fit together without getting too overwhelming. Maybe there's a better approach though?

And uh actually, why do we suggest adding a & pattern instead of removing the ref binding? I don't remember what led to this choice.

For this file's suggestions (experimental/mixed-editions.rs), it's because the current ref-eliding suggestion logic makes some assumptions that don't work for match ergonomics 2.0, so I made it not run when those feature gates are enabled. The desugaring suggestions might also be a bit wrong (I don't remember), but the ref-eliding logic could produce empty suggestions, which was a problem. We'll need to rework the diagnostics before stabilization, but we'll be able to do better then too. Also the ref in this particular error was from a macro expansion, so it can't be elided. The removing-ref tests are in migration_lint.rs. I'd just linked to the first test stderr on the diff page; it didn't occur to me it might not be the most helpful one to review ^^;

For [ref x]: &mut [T], I've kept the ref in the suggestion for two reasons. First, it's because the suggestion logic is shared by the rust_2024_incompatible_pat lint, where applying a type-changing suggestion could cause valid Rust 2021 code to stop compiling. Second, in pathological cases, &mut [ref x] and [x] may both compile but behave differently at runtime, due to the change in type influencing trait selection. If we want to discourage writing that, maybe we could lint on it.

We do suggest removing ref in other cases (see migration-lint.rs), but e.g. if &s need to be added we give up and desugar everything for simplicity, since that may result in needing to add more &s and binding modifiers elsewhere in the pattern. I had a too-complex PR for giving nicer suggestions more often, and I think you suggested a simpler approach, but we ended up closing the PR since we'd already covered the most common case and we'll be able to do better for match ergonomics 2.0.

[Nadrieril]

Ok yeah I remember that. This is good enough and we can indeed revisit once we're going to stabilization.

[dianne]

At this point, maybe the hard error version of this would be better serviced by linking to the Reference rather than the Edition Guide? Specifically https://doc.rust-lang.org/reference/patterns.html#r-patterns.ident.binding. There's less highlighting of the error cases, but being less focused on Edition migration could be helpful going forward. It also uses (and elaborates on!) the "non-reference pattern" wording and is very methodical about explaining match ergonomics, which I think is helpful. It's a bit easier for me to follow than the Edition Guide's presentation, too. Of course, if we want to be consistent with terminology and presentation, we'll probably also want to update the Edition Guide; #143557 indicates that it may need some rewriting.

[Nadrieril]

Oh yeah that reads much better, it even has the "edition differences" clearly indicated. +1 on this

[dianne]

Done! I went with https://doc.rust-lang.org/reference/patterns.html#binding-modes for the hard error; it goes to the same section but it's a bit cleaner (and more legible) than the rule link I suggested earlier. The lint still directs to the edition guide since it's paired with a "warning: this changes meaning in Rust 2024" and it seems helpful to have as much explanation as possible for the edition change specifically.

[Nadrieril]

Sounds good!

[Nadrieril]

I quite like this now, the "implicit borrow" terminology feels quite clear. r=me after the link change!

View changes since this review

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Nadrieril]

@bors r+

[bors]

📌 Commit ec99e3e has been approved by Nadrieril

It is now in the queue for this repository.