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.

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

[NLL] Remove base_place #53932

Merged
merged 1 commit into from
Sep 8, 2018
Merged

[NLL] Remove base_place #53932

merged 1 commit into from
Sep 8, 2018

Conversation

matthewjasper
Copy link
Contributor

This function was supposed to make Box less special. But

  • I think that the consensus is that MIR borrowck is going to fully special case Box
  • It wasn't implemented correctly, it's looking at the type of the wrong Place, resulting in weird behaviour:
#![feature(nll)]
type A = Box<i32>; // If this is changed to another type then this will compile.

pub fn foo(x: Box<(String, A)>) {
    let a = x.0; // This will compile if these lines are swapped
    let b = x.1;
}

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 3, 2018
@memoryruins memoryruins added the A-NLL Area: Non Lexical Lifetimes (NLL) label Sep 3, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 4, 2018

📌 Commit faf80ad has been approved by nikomatsakis

@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 Sep 4, 2018
@kennytm kennytm mentioned this pull request Sep 7, 2018
Closed
kennytm added a commit to kennytm/rust that referenced this pull request Sep 7, 2018
@kennytm
Rollup merge of rust-lang#53932 - matthewjasper:remove-base-path, r=n…
39f1b46 
…ikomatsakis

[NLL] Remove base_place

This function was supposed to make `Box` less special. But

* I think that the consensus is that MIR borrowck is going to fully special case `Box`
* It wasn't implemented correctly, it's looking at the type of the wrong `Place`, resulting in weird behaviour:

```rust
#![feature(nll)]
type A = Box<i32>; // If this is changed to another type then this will compile.

pub fn foo(x: Box<(String, A)>) {
    let a = x.0; // This will compile if these lines are swapped
    let b = x.1;
}
```

r? @nikomatsakis
kennytm added a commit to kennytm/rust that referenced this pull request Sep 8, 2018
@kennytm
Rollup merge of rust-lang#53932 - matthewjasper:remove-base-path, r=n…
0158694 
…ikomatsakis

[NLL] Remove base_place

This function was supposed to make `Box` less special. But

* I think that the consensus is that MIR borrowck is going to fully special case `Box`
* It wasn't implemented correctly, it's looking at the type of the wrong `Place`, resulting in weird behaviour:

```rust
#![feature(nll)]
type A = Box<i32>; // If this is changed to another type then this will compile.

pub fn foo(x: Box<(String, A)>) {
    let a = x.0; // This will compile if these lines are swapped
    let b = x.1;
}
```

r? @nikomatsakis
@kennytm kennytm mentioned this pull request Sep 8, 2018
Merged
bors added a commit that referenced this pull request Sep 8, 2018
@bors
Auto merge of #54051 - kennytm:rollup, r=kennytm
16c1598 
Rollup of 11 pull requests

Successful merges:

 - #51366 (stabilize #[panic_handler])
 - #53162 (rustdoc: collect trait impls as an early pass)
 - #53932 ([NLL] Remove base_place)
 - #53942 (Rewrite `precompute_borrows_out_of_scope` for fewer hash table lookups.)
 - #53973 (Have rust-lldb look for the rust-enabled lldb)
 - #53981 (Implement initializer() for FileDesc)
 - #53987 (rustbuild: allow configuring llvm version suffix)
 - #53993 (rustc_resolve: don't record uniform_paths canaries as reexports.)
 - #54007 (crates that provide a `panic_handler` are exempt from the `unused_extern_crates` lint)
 - #54040 (update books for next release)
 - #54050 (Update `petgraph` dependency to 0.4.13 to fix build with nightly)

Failed merges:

r? @ghost
kennytm added a commit to kennytm/rust that referenced this pull request Sep 8, 2018
@kennytm
Rollup merge of rust-lang#53932 - matthewjasper:remove-base-path, r=n…
7569d92 
…ikomatsakis

[NLL] Remove base_place

This function was supposed to make `Box` less special. But

* I think that the consensus is that MIR borrowck is going to fully special case `Box`
* It wasn't implemented correctly, it's looking at the type of the wrong `Place`, resulting in weird behaviour:

```rust
#![feature(nll)]
type A = Box<i32>; // If this is changed to another type then this will compile.

pub fn foo(x: Box<(String, A)>) {
    let a = x.0; // This will compile if these lines are swapped
    let b = x.1;
}
```

r? @nikomatsakis
bors added a commit that referenced this pull request Sep 8, 2018
@bors
Auto merge of #54051 - kennytm:rollup, r=kennytm
05cb29e 
Rollup of 10 pull requests

Successful merges:

 - #53315 (use `NonZeroU32` in `newtype_index!`macro, change syntax)
 - #53932 ([NLL] Remove base_place)
 - #53942 (Rewrite `precompute_borrows_out_of_scope` for fewer hash table lookups.)
 - #53973 (Have rust-lldb look for the rust-enabled lldb)
 - #53981 (Implement initializer() for FileDesc)
 - #53987 (rustbuild: allow configuring llvm version suffix)
 - #53993 (rustc_resolve: don't record uniform_paths canaries as reexports.)
 - #54007 (crates that provide a `panic_handler` are exempt from the `unused_extern_crates` lint)
 - #54040 (update books for next release)
 - #54050 (Update `petgraph` dependency to 0.4.13 to fix build with nightly)
@bors bors merged commit faf80ad into rust-lang:master Sep 8, 2018
@matthewjasper matthewjasper deleted the remove-base-path branch September 8, 2018 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
A-NLL Area: Non Lexical Lifetimes (NLL) 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.
5 participants
@matthewjasper @nikomatsakis @bors @memoryruins @rust-highfive