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

Cannot mutably borrow inside a match guard #24535

Closed
Binero opened this issue Apr 17, 2015 · 3 comments
Closed

Cannot mutably borrow inside a match guard #24535

Binero opened this issue Apr 17, 2015 · 3 comments

Comments

@Binero
Copy link
Contributor

Binero commented Apr 17, 2015

I understand this shouldn't be possible in some occasions, because you might be able to modify what you're matching, which can cause the match to break. Why would it be disallowed to mutably borrow something that only exists within the match though?

fn main() {
    let a = 3u8;

    match a {
        0 => (),
        3 if compare(&a, &mut 3) => (),
        _ => (),
    }
}

fn compare(a: &u8, b: &mut u8) -> bool {
    a == b
}

http://is.gd/yuIFGc

I have constructed this minimal case. In my actual code I am running .iter().any() on a variable form outside the match, which isn't allowed because .any() requires a mutable borrow. As the thing I am mutably borrowing only exists within the match, this makes even less sense to me.

Compiler bug?

@amnn
Copy link

amnn commented Apr 17, 2015

Hi @Binero, I actually solved your use case like so: http://is.gd/ffFhbA by using .contains(...) instead of your .iter().any().

@Binero
Copy link
Contributor Author

Binero commented Apr 17, 2015

This certainly is a nice fix (and probably how it should've been done in the first place), however I still feel like this should be possible.

@steveklabnik
Copy link
Member

Changing match in this way would require an RFC, I would believe, as it's a pretty significant change, and would need to be well-thouht-out for soundness reasons. You should open an RFC issue instead. Thanks!

pnkfelix added a commit to pnkfelix/rust that referenced this issue May 29, 2018
Now, if you pass `-Z disable-ast-check-for-mutation-in-guard`, then we
will just allow you to mutably-borrow and assign in guards of `match`
arms.

This is wildly unsound with AST-borrowck. It is also unsound with
MIR-borrowck without further adjustments, which come in later in the
commit series on this Pull Request.

See also rust-lang#24535 and rust-lang/rfcs#1006.
pnkfelix added a commit to pnkfelix/rust that referenced this issue May 29, 2018
bors added a commit that referenced this issue May 30, 2018
…ake-three, r=nikomatsakis

every match arm reads the match's borrowed input

This PR changes the `match` codegen under NLL (and just NLL, at least for now) to make the following adjustments:
 * It adds a `-Z disable-ast-check-for-mutation-in-guard` which, as described, turns off the naive (conservative but also not 100% sound) check for mutation in guards of match arms.
 * We now borrow the match input at the outset and emit a special `ReadForMatch` statement (that, according to the *static* semantics, reads that borrowed match input) at the start of each match arm. The intent here is to catch cases where the match guard mutates the match input, either via an independent borrow or via `ref mut` borrows in that arm's pattern.
 * In order to ensure that `ref mut` borrows do not actually conflict with the emitted `ReadForMatch` statements, I expanded the two-phase-borrow system slightly, and also changed the MIR code gen so that under NLL, when there is a guard on a match arm, then each pattern variable ends up having *three* temporaries associated with it:
   1. The first temporary will hold the substructure being matched; this is what we will move the (substructural) value into *if* the guard succeeds.
   2. The second temporary also corresponds to the same value as the first, but we are just constructing this temporarily for use during the scope of the guard; it is unaliased and its sole referrer is the third temporary.
   3. The third temporary is a reference to the second temporary.
   * (This sounds complicated, I know, but its actually *simpler* than what I was doing before and had checked into the repo, which was to sometimes construct the final value and then take a reference to it before evaluating the guard. See also PR #49870.)

Fix #27282

This also provides a path towards resolving #24535 aka rust-lang/rfcs#1006, at least once the `-Z disable-ast-check-for-mutation-in-guard` is just turned on by default (under NLL, that is. It is not sound under AST-borrowck).
 * But I did not want to make `#![feature(nll)]` imply that as part of this PR; that seemed like too drastic a change to me.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 3, 2023
…wck-test, r=Nilstrieb

Test the borrowck behavior of if-let guards

Add some tests to make sure that if-let guards behave the same as if guards with respect to borrow-checking. Most of them are a naive adaptation, replacing an `if` guard with `if let Some(())`.
This includes regression tests for notable issues that arose for if guards (rust-lang#24535, rust-lang#27282, rust-lang#29723, rust-lang#31287) as suggested in rust-lang#51114 (comment).

cc `@pnkfelix` are there any other tests that you would want to see?
cc tracking issue rust-lang#51114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants