Skip to content

Moves in guard expressions aren't handled properly #22073

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
Aatch opened this issue Feb 8, 2015 · 6 comments · Fixed by #22580
Closed

Moves in guard expressions aren't handled properly #22073

Aatch opened this issue Feb 8, 2015 · 6 comments · Fixed by #22580
Assignees
Milestone

Comments

@Aatch
Copy link
Contributor

Aatch commented Feb 8, 2015

This code causes a segfault (without optimisations on):

use std::fmt;
fn main() {
    let a = std::rc::Rc::new(1);

    let x = (1, 2);

    match x {
        (_, y) |
        (y, _) if take(y, a) => (),
        _ => ()
    }

}

fn take<T:fmt::Debug>(x: usize, t: T) -> bool {
    println!("{:?}", t);
    x == 1
}

as does this similar code:

use std::fmt;
fn main() {
    let a = std::rc::Rc::new(1);

    let x = (1, 2);

    match x {
        (_, y) if take(y, a) => (),
        (y, _) if take(y, a) => (),
        _ => ()
    }

}

fn take<T:fmt::Debug>(x: usize, t: T) -> bool {
    println!("{:?}", t);
    x == 1
}

This is because the current control flow graph essentially visits each arm in parallel. With multiple patterns, each pattern within the arm is visited, then the guard is visited after all the patterns have been visited, even though this doesn't reflect the real semantics (we visit the pattern, then the guard, then the next pattern, then the guard again).

@Aatch
Copy link
Contributor Author

Aatch commented Feb 8, 2015

I'm working on a fix for both of these issues. Should be done fairly soon I hope.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 8, 2015

@Aatch whoa, I thought we didn't support moves into guards in the first place?

(That is, not that I object to you fixing this ... but I had thought there was an explicit decision not to support such guards, at least not in the short term, because we were not confident we could make it work!)

Am I mistaken?

@Aatch
Copy link
Contributor Author

Aatch commented Feb 8, 2015

@pnkfelix Well I managed to get it to work (you're actually assigned to the PR fixing it). You may be thinking of mutation in guards though, since that's disallowed.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 8, 2015

@Aatch ah that is what I was thinking of, indeed. (My mind isn't made up about whether it makes sense to support moves into guards; if we got away with disallowing mutation, why not moves as well, at least for the short term.)

@pnkfelix
Copy link
Member

1.0 beta, P-backcompat-lang.

@pnkfelix
Copy link
Member

(this comment was accidentally posted here, and has moved to: #22075 (comment) )

@pnkfelix pnkfelix reopened this Feb 20, 2015
pnkfelix pushed a commit to pnkfelix/rust that referenced this issue Feb 22, 2015
Update the graphviz tests accordingly.

Fixes rust-lang#22073. (Includes regression test for the issue.)

(Factoring of aatch CFG code, Part 4.)
Manishearth added a commit to Manishearth/rust that referenced this issue Feb 24, 2015
 aatch's cfg revisions, namely to match expressions

Revise handling of match expressions so that arms branch to next arm.

Update the graphviz tests accordingly.

Fixes rust-lang#22073. (Includes regression test for the issue.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants