Skip to content

Pattern errors are too imprecise and should be removed in favor of MIR borrowck #45600

Closed

Description

Update by @pnkfelix :

NLL (aka MIR-borrowck) addresses the first case given in the issue, but not this one:

struct Foo(String, String);
fn x(f: Foo) {
    match f {
        Foo(a, ref b) => {} 
    }
}

Original bug report follows


For this code:

struct Foo;
fn main() {
    match Foo {
        x @ Foo if true => x,
        _ => Foo,
    };
}

rustc produces:

error[E0008]: cannot bind by-move into a pattern guard
 --> test.rs:4:9
  |
4 |         x @ Foo if true => x,
  |         ^^^^^^^ moves value into pattern guard

This error comes from check_legality_of_move_bindings in librustc_const_eval/check_match.rs.

The message is at least arguably incorrect: since x is not used in the pattern guard, it doesn't make much sense to claim that the code "moves value into pattern guard".

I suppose you could argue that all bindings in a pattern are always moved/copied to the pattern guard, and the fact that this one is unused doesn't matter. Under that interpretation, it's a matter of improving the error message, and this issue is a dupe of rust-lang/rfcs#684 which seeks to improve that error message in general.

However, I think the code should be allowed, as it's not unsound. I'm not too familiar with compiler internals, but it seems like once MIR borrow checking is finished, it would catch any actually-unsound moves into pattern guards, and this error could be removed altogether. Is that correct?

The other two errors generated by check_legality_of_move_bindings have similar issues:

  1. "cannot bind by-move with sub-bindings"

For one thing, this error seems to be a subset of a different one from elsewhere in check_match.rs, "pattern bindings are not allowed after an @" - at least, I can't think of any code that would trigger this and not that. (Feel free to let me know what I'm missing.) But in any case, this is overly conservative. If we have

struct Foo(i32);

then it should be legal to match x @ Foo(y) (because y is Copy). Again, this seems like something MIR borrowck might be able to handle better, although the frontend might have to make sure the copy comes from the right place (pre- or post-move).

  1. "cannot bind by-move and by-ref in the same pattern"

I don't even know what this error is trying to prevent. The following works fine even with the existing borrowck (f properly becomes a partially moved value):

struct Foo(String, String);
fn x(f: Foo) {
    match f {
        Foo(a, _) => {
            let b = &f.1;
        }
    }
}

So what is wrong with putting b into the pattern?

fn x(f: Foo) {
    match f {
        Foo(a, ref b) => {}
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

A-NLLArea: Non-lexical lifetimes (NLL)Area: Non-lexical lifetimes (NLL)A-borrow-checkerArea: The borrow checkerArea: The borrow checkerE-mediumCall for participation: Medium difficulty. Experience needed to fix: Intermediate.Call for participation: Medium difficulty. Experience needed to fix: Intermediate.F-move_ref_pattern`#![feature(move_ref_pattern)]``#![feature(move_ref_pattern)]`NLL-completeWorking towards the "valid code works" goalWorking towards the "valid code works" goalP-mediumMedium priorityMedium priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions