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

For box expressions, use NZ drop instead of a free block #43772

Merged
merged 1 commit into from
Aug 12, 2017

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Aug 9, 2017

This falls naturally out of making drop elaboration work with box
expressions, which is probably required for sane MIR borrow-checking.
This is a pure refactoring with no intentional functional effects.

r? @nagisa

@aidanhs aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2017
This falls naturally out of making drop elaboration work with `box`
expressions, which is probably required for sane MIR borrow-checking.
This is a pure refactoring with no intentional functional effects.
@@ -805,9 +726,8 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>,
block.unit()
}

fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
fn build_diverge_scope<'a, 'gcx, 'tcx>(_tcx: TyCtxt<'a, 'gcx, 'tcx>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go ahead and remove this parameter.

@nagisa
Copy link
Member

nagisa commented Aug 10, 2017

@bors r+

Feel free to bors r=nagisa if you fix the nit yourself.

@bors
Copy link
Contributor

bors commented Aug 10, 2017

📌 Commit 17d2bcd has been approved by nagisa

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2017
@bors
Copy link
Contributor

bors commented Aug 12, 2017

⌛ Testing commit 17d2bcd with merge b8266a9...

bors added a commit that referenced this pull request Aug 12, 2017
For box expressions, use NZ drop instead of a free block

This falls naturally out of making drop elaboration work with `box`
expressions, which is probably required for sane MIR borrow-checking.
This is a pure refactoring with no intentional functional effects.

r? @nagisa
@bors
Copy link
Contributor

bors commented Aug 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nagisa
Pushing b8266a9 to master...

@bors bors merged commit 17d2bcd into rust-lang:master Aug 12, 2017
@bors bors mentioned this pull request Aug 12, 2017
7 tasks
@oli-obk
Copy link
Contributor

oli-obk commented Aug 14, 2017

I think this caused let x = box 4i32; to no longer produce a StorageLive for the temporary used to assign x.

fn main() -> () {
    let mut _0: ();                      // return pointer
    scope 1 {
        let _1: std::boxed::Box<i32>;    // "x" in scope 1 at src/main.rs:4:9: 4:10
    }
    let mut _2: std::boxed::Box<i32>;

    bb0: {
        StorageLive(_1);                 // scope 0 at src/main.rs:4:9: 4:10
        _2 = Box(i32);                   // scope 0 at src/main.rs:4:13: 4:21
        (*_2) = const 4i32;              // scope 0 at src/main.rs:4:17: 4:21
        _1 = _2;                         // scope 0 at src/main.rs:4:13: 4:21
        StorageDead(_2);                 // scope 0 at src/main.rs:4:21: 4:21
        _0 = ();                         // scope 0 at src/main.rs:3:11: 5:2
        drop(_1) -> bb1;                 // scope 0 at src/main.rs:5:2: 5:2
    }

    bb1: {
        StorageDead(_1);                 // scope 0 at src/main.rs:5:2: 5:2
        return;                          // scope 0 at src/main.rs:5:2: 5:2
    }
}

Is this intended?

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 14, 2017

Is this intended?

No.

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 14, 2017

I think the difference is that we started emitting StorageDead, rather than stopping emitting StorageLive.

bors added a commit that referenced this pull request Aug 15, 2017
emit StorageLive for box temporaries

We started emitting StorageDead, so we better emit the corrseponding
StorageLive to avoid problems.

cc #43772 rust-lang/miri#303
@ghost ghost mentioned this pull request Aug 17, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Aug 17, 2017
refactor(mir): remove unused argument

Small cleanup that shouldn't have any impact, as it's a small thing introduced in rust-lang#43772
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants