Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider Rule 4E (early) for RFC 3627 #130501

Open
Nadrieril opened this issue Sep 18, 2024 · 5 comments
Open

Reconsider Rule 4E (early) for RFC 3627 #130501

Nadrieril opened this issue Sep 18, 2024 · 5 comments
Labels
A-patterns Relating to patterns and pattern matching C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Nadrieril
Copy link
Member

Nadrieril commented Sep 18, 2024

Context

RFC3627 set out to improve some unintuitive edges of match ergonomics. The subtlest part involves fixing this case:

let [x]: &[&T] = ...;
// x: &&T
let [&x]: &[&T] = ...;
// x: T

where a & pattern appears to remove two layers of references. T-lang agreed on the desireability of "eat-one-layer" instead, namely that a & pattern should only ever remove one layer of reference.

For that, the RFC proposes rule 2: "When a reference pattern matches against a reference, do not update the default binding mode". While this is arguably a straightforward change from an implementation perspective, let me show you that it does not appropriately solve the problem we set out to solve from a language perspective.

Issue

The reason is simple: there are two references at play in &&T; with rule 2 we match the pattern against the inner one of these. Some consequences (you can try these out in my online tool which can run both TC's and my solvers; just note that rule4-early has bugs when combined with rule 5):

  • The mutability that matters is the inner one:
let [x]: &[&mut T] = ...;
// x: &&mut T
let [&x]: &[&mut T] = ...;
// with rule 2: Type error
// with rule 4E: x: &mut T + borrow checking error
let [&mut x]: &[&mut T] = ...;
// with rule 2: x: &T
// with rule 4E: type error

let [x]: &mut [&T] = ...;
// x: &mut &T
let [&x]: &mut [&T] = ...;
// with rule 2: `x: &mut T`, which causes a borrow-checking error
// with rule 4E: type error
let [&mut x]: &mut [&T] = ...;
// with rule 2: Type error
// with rule 4E: x: &T
  • References are considered inherited when they shouldn't, which is visible with mut or ref bindings:
let [&x]: &[&T] = ...;
// with rule 2: x: &T
// with rule 4E: x: &T
let [&ref x]: &[&T] = ...;
// with rule 2: x: &T because the reference was considered inherited and `ref x` overrides that
// with rule 4E: x: &&T

let &[x]: &[&T] = ...;
// with rule 2: x: &T
// with rule 4E: x: &T
let &[ref x]: &[&T] = ...;
// with rule 2: x: &&T because the reference was not considered inherited
// with rule 4E: x: &&T
  • Combined with the rest of RFC3627, we get weirdly inconsistent behaviors such as:
let [&mut (ref x)]: &mut [&mut T] = ...;
// RFC3627: `x: &T` because we got an inherited `&mut` and `ref` overrode it
// with rule 4E: x: &&mut T
let [&mut (ref x)]: &mut [&    T] = ...;
// RFC3627: `x: &&T` because the mutability mismatch triggered rule 4 instead
// with rule 4E: x: &&T

In short: rule 2 does "eat-one-layer" but eats the wrong layer. The fix is simply to eat the other one. In the language of RFC3627: "when the binding mode is ref or ref mut, match the pattern against the binding mode as if it was a reference"; this has been called "rule 4-early" in our discussions.

Edition

RFC3627 proposed to enable rules 1 and 2 over the edition. I propose to instead enable rules 1 and 4-early over the edition. Note that rule 4-early also replaces rule 4.

While I'm at it I would like to add a small additional rule, to enable fixing all the surprises:

  • Rule 1.5: When the DBM (default binding mode) is not move, writing ref on a binding is an error.

We can always revert to the previous behavior (i.e. ref x swallows a reference if it is inherited) if we wish to later.

cc @traviscross

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 18, 2024
@jieyouxu jieyouxu added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 18, 2024
@fmease fmease added the A-patterns Relating to patterns and pattern matching label Sep 18, 2024
@traviscross traviscross added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 18, 2024
@traviscross traviscross changed the title RFC3627's rule 2 is not what we want Reconsider RFC 3627 Rule 4 (+ Rule 2) Sep 18, 2024
@traviscross traviscross changed the title Reconsider RFC 3627 Rule 4 (+ Rule 2) Reconsider RFC 3627 Rule 4 (+ Rule 2)? Sep 18, 2024
@traviscross
Copy link
Contributor

RFC3627 proposed to enable rules 1 and 2 over the edition. I propose to instead enable rules 1 and 4-early over the edition. Note that rule 4-early also replaces rule 4.

Since Rule 4E (early), as proposed here, is edition-dependent and makes the operation of the other rules visible within patterns, this makes Rule 3 and Rule 5 edition-dependent also. So this proposal must either require adopting all the rules ahead of an edition or alternatively requiring a separate edition for adopting Rule 3 and Rule 5.

@traviscross traviscross changed the title Reconsider RFC 3627 Rule 4 (+ Rule 2)? Reconsider Rule 4E (early) for RFC 3627? Sep 18, 2024
@traviscross
Copy link
Contributor

Here's what we had said about this alternative in the 2024-06-26 lang design meeting (Match ergonomics, part 3):

Do "Rule 4 early" variant

That rule is:

If the DBM is ref or ref mut, match a reference pattern against the DBM as though it were a type before considering the scrutinee.

By contrast, without this, Rule 4 only matches against the DBM when, given a reference pattern, the scrutinee is a non-reference type [ed: later somewhat amended with Rule 4X] (and therefore could not match against the pattern).

The effect of this early variant is to break the primacy of structural matching. E.g., with that variant, this does not work, even though the &mut in the pattern exactly matches the structure in the scrutinee:

let [&mut [x]] = &[&mut [T]];
//~^ ERROR mismatched types

This happens because, at that point, we're in the ref DBM, and so the &mut pattern can't match against it, but it tries to due to this early rule.

We think the primacy of structural matching is important, so we haven't adopted this variant.

Further, this variant adds even more backward edges than Rule 4 already does, and we're trying to minimize those.

There's also a practical problem with this variant also; while Rule 4 is backward compatible and can be applied to all editions, the early variant is not (because it breaks structural matching). This means that it could only happen over an edition. Since Rule 4 (in any form) makes the operation of the other rules visible, this could also then make Rule 3 and probably Rule 5 edition-dependent. That's kind of a mess.

@Nadrieril
Copy link
Member Author

Nadrieril commented Sep 18, 2024

So this proposal must either require adopting all the rules ahead of an edition or alternatively requiring a separate edition for adopting Rule 3 and Rule 5

I didn't know better at the time but this proposition doesn't actually change anything:

  • adding rule 5 later is still backwards-compatible, as checked with my tool on all patterns of depth <= 6 and types of depth <= 7;
  • adding rule 3 would indeed require another edition, but this was already the case with the previous proposal of rules 1+2.

@Nadrieril Nadrieril changed the title Reconsider Rule 4E (early) for RFC 3627? Reconsider Rule 4E (early) for RFC 3627 Sep 18, 2024
@traviscross
Copy link
Contributor

traviscross commented Sep 18, 2024

This example does not demonstrate that. Inlining, the example is:

&[x]: &&mut [&mut T]
x: &mut &mut T //~ ERROR, RFC 3627
x: &&mut T //~ RFC 3627

I.e.:

#[derive(Copy, Clone)]
struct T;

fn main() {
    let &[_x] = &&mut [&mut T];
    //~^ ERROR cannot borrow as mutable
}

This case is an error in the current edition, so it does not show an edition-dependent change (it's backward compatible to make an error into a non-error). And Rule 1 and Rule 2 have no effect here.

@Nadrieril
Copy link
Member Author

Nadrieril commented Sep 18, 2024

Oh apologies, I blindly trusted the solvers but it seems this is a case of borrow-checking that's not flagged. I have convinced myself that you're correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-patterns Relating to patterns and pattern matching C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants