Skip to content

[mir-opt] Fix Inline pass to handle inlining into box expressions #67333

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 22, 2019

Conversation

wesleywiser
Copy link
Member

@wesleywiser wesleywiser commented Dec 15, 2019

r? @oli-obk

Before, the test case just ICE'd here:

ref place => bug!("Return place is {:?}, not local", place),

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 15, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Dec 15, 2019

Would x.field = foo(); have hit the same ICE before?

@wesleywiser
Copy link
Member Author

Would x.field = foo(); have hit the same ICE before?

I tried this

#[inline]
fn bar() -> i32 {
    3
}

fn main() {
    let mut foo = (1, 2);
    foo.1 = bar();
}

and it doesn't trigger the issue because we assign the return of bar() to a temporary and then assign that temporary to foo.1 after the call. This ICE occurs specifically when the destination contains a projection.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 15, 2019

This ICE occurs specifically when the destination contains a projection.

So... let x = (42, foo());? Or how is box special in this case?

@wesleywiser
Copy link
Member Author

box is special because it causes dest_needs_borrow() to return true. This triggers the reborrow and means that when the Integrator runs, destination has a projection. For let x = (42, foo());, destination is a plain Local.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 15, 2019

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 15, 2019

📌 Commit b50cee3 has been approved by oli-obk

@bors
Copy link
Collaborator

bors commented Dec 15, 2019

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@bors bors 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 Dec 15, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 19, 2019
bors added a commit that referenced this pull request Dec 19, 2019
Rollup of 9 pull requests

Successful merges:

 - #67321 (make htons const fn)
 - #67328 (Remove now-redundant range check on u128 -> f32 casts)
 - #67333 ([mir-opt] Fix `Inline` pass to handle inlining into `box` expressions)
 - #67354 (Fix pointing at arg when cause is outside of call)
 - #67363 (Fix handling of wasm import modules and names)
 - #67382 (Remove some unnecessary `ATTR_*` constants.)
 - #67389 (Remove `SO_NOSIGPIPE` dummy variable on platforms that don't use it.)
 - #67393 (Enable opting out of specific default LLVM arguments.)
 - #67394 (Remove outdated references to @t from comments)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Dec 20, 2019
@Centril
Copy link
Contributor

Centril commented Dec 20, 2019

Failed in #67448 (comment), @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 20, 2019
@wesleywiser wesleywiser force-pushed the fix_inline_into_box_place branch from b50cee3 to f1325a7 Compare December 21, 2019 01:40
@wesleywiser
Copy link
Member Author

Rebased and modified new test to skip wasm which aborts instead of unwinding.

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Dec 21, 2019

📌 Commit f1325a7 has been approved by oli-obk

@bors
Copy link
Collaborator

bors commented Dec 21, 2019

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 21, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 21, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 21, 2019
bors added a commit that referenced this pull request Dec 21, 2019
Rollup of 7 pull requests

Successful merges:

 - #67160 (Make GATs less ICE-prone.)
 - #67333 ([mir-opt] Fix `Inline` pass to handle inlining into `box` expressions)
 - #67420 (use _val to ignore parameter of any::type_name_of_val)
 - #67469 (Remove rustc-dev from the default nightly components)
 - #67489 (Drop petgraph dependency from bootstrap)
 - #67490 (Document privacy of RangeInclusive fields)
 - #67491 (use Result::map_or for bootstrap)

Failed merges:

r? @ghost
@bors bors merged commit f1325a7 into rust-lang:master Dec 22, 2019
# for free to join this conversation on GitHub. Already have an account? # 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.

5 participants