Skip to content

Wrong code with assignment in match body #26996

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

Closed
eefriedman opened this issue Jul 12, 2015 · 4 comments
Closed

Wrong code with assignment in match body #26996

eefriedman opened this issue Jul 12, 2015 · 4 comments
Assignees
Labels
A-codegen Area: Code generation

Comments

@eefriedman
Copy link
Contributor

fn main() {
    let mut c = (1, "".to_owned());
    match c {
        c2 => { c.0 = 2; assert_eq!(c2.0, 1); }
    }
}
thread '<main>' panicked at 'assertion failed: `(left == right)` (left: `2`, right: `1`)', <anon>:4
playpen: application terminated with error code 101
@Aatch Aatch added I-wrong A-codegen Area: Code generation labels Jul 13, 2015
@Thiez
Copy link
Contributor

Thiez commented Jul 13, 2015

Actually what happens here is the following:

The tuple you create in c is moved, because it is not Copy. When you perform the match you move out of c, into c2, leaving c uninitialized. Your assignment to c.0 is basically to uninitialized memory, and doesn't have an effect. When you perform the assertion you see the old value of c.0, because your assignment to c.0 had no effect.

The whole thing is confusing because Rust does not warn when you perform useless assignments to moved variables, and it probably should. The whole thing becomes obviously when you add a statement that uses c in the match arm:

fn main() {
    let mut c = (1, "".to_owned());
    match c {
        c2 => { println!("{}", c.0); c.0 = 2; assert_eq!(c2.0, 1); }
    }
}

Edit: @Aatch the labels you have assigned to this issue are incorrect.

@mitaa
Copy link
Contributor

mitaa commented Jul 13, 2015

Your assignment to c.0 is basically to uninitialized memory, and doesn't have an effect. When you perform the assertion you see the old value of c.0, because your assignment to c.0 had no effect.

Don't we see the new value c.0 = 2;, even though we're accessing c2.0, when performing the assertion, which causes it to fail?

Doing this without a match statement passes the same assertion sucessfully:

fn main() {
    let mut c = (1, "".to_owned());
    let c2 = c;
    c.0 = 2;
    assert_eq!(c2.0, 1);
}

@Thiez
Copy link
Contributor

Thiez commented Jul 13, 2015

Oops, you are correct, I am mistaken :(

On Mon, Jul 13, 2015 at 10:52 AM, mitaa [email protected] wrote:

Your assignment to c.0 is basically to uninitialized memory, and doesn't
have an effect. When you perform the assertion you see the old value of
c.0, because your assignment to c.0 had no effect.

Don't we see the new value c.0 = 2;, even though we're accessing c2.0,
when performing the assertion, which causes it to fail?

Doing this without a match statement passes the same assertion sucessfully:

fn main() {
let mut c = (1, "".to_owned());
let c2 = c;
c.0 = 2;
assert_eq!(c2.0, 1);
}


Reply to this email directly or view it on GitHub
#26996 (comment).

@dotdash dotdash self-assigned this Jul 13, 2015
dotdash added a commit to dotdash/rust that referenced this issue Jul 13, 2015
If we match a whole struct or tuple, the "field" for the reassignment
checker will be "None" which means that mutating any field should count
as a reassignment.

Fixes rust-lang#26996.
bors added a commit that referenced this issue Jul 13, 2015
If we match a whole struct or tuple, the "field" for the reassignment
checker will be "None" which means that mutating any field should count
as a reassignment.

Fixes #26996.
@Aatch
Copy link
Contributor

Aatch commented Jul 14, 2015

@Thiez heh, I made the same mistake. Typed out most of a response and then realised that I'd mis-read the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation
Projects
None yet
Development

No branches or pull requests

5 participants