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

MIR match bindings have the scope of the match instead of the arm. #32790

Closed
eddyb opened this issue Apr 7, 2016 · 1 comment · Fixed by #32952
Closed

MIR match bindings have the scope of the match instead of the arm. #32790

eddyb opened this issue Apr 7, 2016 · 1 comment · Fixed by #32952
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html

Comments

@eddyb
Copy link
Member

eddyb commented Apr 7, 2016

Reduced from a debuginfo test failing with an implementation of #31005:

fn main() {
    match Ok::<(), ()>(()) {
        Ok(a) => zzz(),
        Err(a) => zzz()
    }
}

fn zzz() {}
fn main() -> () {
    let var0: (); // a in Scope(3) at test2.rs:3:12: 3:13
    let var1: (); // a in Scope(3) at test2.rs:4:13: 4:14
    let mut tmp0: std::result::Result<(), ()>;
    let mut tmp1: ();

    bb0: {
        tmp1 = (); // Scope(5) at test2.rs:2:24: 2:26
        tmp0 = std::prelude::v1::Ok<(), ()>(tmp1,); // Scope(4) at test2.rs:2:11: 2:27
        switch(tmp0) -> [Ok: bb4, Err: bb5]; // Scope(3) at test2.rs:3:9: 3:14
    }

    bb1: {
        return; // Scope(0) at test2.rs:1:1: 6:2
    }

    bb2: {
        return = zzz() -> bb1; // Scope(7) at test2.rs:3:18: 3:23
    }

    bb3: {
        return = zzz() -> bb1; // Scope(10) at test2.rs:4:19: 4:24
    }

    bb4: {
        var0 = ((tmp0 as Ok).0: ()); // Scope(3) at test2.rs:3:12: 3:13
        goto -> bb2; // Scope(3) at test2.rs:3:9: 3:14
    }

    bb5: {
        var1 = ((tmp0 as Err).0: ()); // Scope(3) at test2.rs:4:13: 4:14
        goto -> bb3; // Scope(3) at test2.rs:4:9: 4:15
    }
    Scope(0) {
        Scope(1) {
            Parent: Scope(0)
            Scope(2) {
                Parent: Scope(1)
                Scope(3) {
                    Parent: Scope(2)
                    Scope(4) {
                        Parent: Scope(3)
                        Scope(5) {
                            Parent: Scope(4)
                    Scope(6) {
                        Parent: Scope(3)
                        Scope(7) {
                            Parent: Scope(6)
                            Scope(8) {
                                Parent: Scope(7)
                    Scope(9) {
                        Parent: Scope(3)
                        Scope(10) {
                            Parent: Scope(9)
                            Scope(11) {
                                Parent: Scope(10)
}

You can see that both a bindings and their assignments share the scope of the match, Scope(3).

This appears to be intentional in the implementation of pattern-matching:

        // Before we do anything, create uninitialized variables with
        // suitable extent for all of the bindings in this match. It's
        // easiest to do this up front because some of these arms may
        // be unreachable or reachable multiple times.
        let var_scope_id = self.innermost_scope_id();
        for arm in &arms {
            self.declare_bindings(var_scope_id, &arm.patterns[0]);
        }

However, this breaks debugging (as both variables appear declared at the same time) and potentially MIR-based region/borrow checking, in the future.

@eddyb eddyb added the A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html label Apr 7, 2016
bors added a commit that referenced this issue Apr 13, 2016
Initial implementation of debuginfo in MIR trans.

Progress is made towards #31005, but several issues remain, such as #32790.
@eddyb
Copy link
Member Author

eddyb commented Apr 14, 2016

Not entirely certain that using the arm body's scope, like in #32952, would play nice with MIR borrowck.
It might just be a matter of adjusting the assignments from bind_matched_candidate to be in the same scope as the variables being bound, instead of in the match scope.
cc @nikomatsakis

bors added a commit that referenced this issue Apr 17, 2016
Get all (but one) of debuginfo tests to pass with MIR codegen.

I didn't get much feedback in #31005 so I went ahead and implemented something simple.
Closes #31005, as MIR debuginfo should work now for most usecases.

The `no-debug-attribute` test no longer assumes variables are in scope of `return`.
We might also want to revisit that in #32949, but the test is more reliable now either way.

In order to get one last function in the `associated-type` test pass, this PR also fixes #32790.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant