Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 12, 2017
Merged

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

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
@arielb1
For box expressions, use NZ drop instead of a free block
17d2bcd 
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.
nagisa
nagisa approved these changes Aug 10, 2017
View reviewed changes
src/librustc_mir/build/scope.rs
@@ -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
Collaborator

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
Collaborator

bors commented Aug 12, 2017

⌛ Testing commit 17d2bcd with merge b8266a9...

bors added a commit that referenced this pull request Aug 12, 2017
@bors
Auto merge of #43772 - arielb1:nonfree-block, r=nagisa
b8266a9 
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
Collaborator

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
Merged
7 tasks
@oli-obk
Copy link
Contributor

oli-obk commented Aug 14, 2017 •
edited
Loading

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?

@pnkfelix pnkfelix mentioned this pull request Aug 14, 2017
Merged
@oli-obk oli-obk mentioned this pull request Aug 14, 2017
Closed
@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.

@arielb1 arielb1 mentioned this pull request Aug 14, 2017
Merged
bors added a commit that referenced this pull request Aug 15, 2017
@bors
Auto merge of #43859 - arielb1:nonfree-block-live, r=nagisa
f6a30bd 
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
Merged
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Aug 17, 2017
@frewsxcv
Rollup merge of rust-lang#43920 - michael-zapata:cleanup/mir, r=arielb1
584dacf 
refactor(mir): remove unused argument

Small cleanup that shouldn't have any impact, as it's a small thing introduced in rust-lang#43772
@nbdd0121 nbdd0121 mentioned this pull request Aug 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nagisa nagisa nagisa approved these changes

Assignees

@nagisa nagisa

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@arielb1 @nagisa @bors @oli-obk @aidanhs @Mark-Simulacrum