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

[NLL] Allow conflicting borrows of promoted length zero arrays #52834

Merged
merged 1 commit into from
Aug 1, 2018

Conversation

matthewjasper
Copy link
Contributor

This is currently overkill as there's no way to create two conflicting borrows of any promoted.
It is possible that the following code might not fail due to const eval in the future (@oli-obk?). In which case either the array marked needs to not be promoted, or to be checked for conflicts

static mut A: () = {
    let mut y = None;
    let z;
    let mut done_y = false;
    loop {
        let x = &mut [1];  // < this array
        if done_y {
            z = x;
            break;
        }
        y = Some(x);
        done_y = true;
    }
    some_const_fn(y, z); // some_const_fn expects that y to not alias z.
};

r? @pnkfelix @nikomatsakis

closes #52671
cc #51823

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 29, 2018
// different promoteds - base case, disjoint
debug!("place_element_conflict: DISJOINT-PROMOTED");
Overlap::Disjoint
} else if mir.promoted[p1.0].ignore_borrow_conflicts {
Copy link
Contributor

Choose a reason for hiding this comment

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

couldn't we just check for zero length array type here instead of infecting everything with that info?

Copy link
Member

Choose a reason for hiding this comment

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

i'm with @oli-obk here: Lets see if that (presumably simpler) fix does the job here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I was concerned about performance thinking that it would need to be check way more than it actually is (which is once per promoted).

@pnkfelix pnkfelix 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2018
@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 31, 2018

📌 Commit 84dc485 has been approved by pnkfelix

@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 Jul 31, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Aug 1, 2018
…r=pnkfelix

[NLL] Allow conflicting borrows of promoted length zero arrays

This is currently overkill as there's no way to create two conflicting borrows of any promoted.
It is possible that the following code might not fail due to const eval in the future (@oli-obk?). In which case either the array marked needs to not be promoted, or to be checked for conflicts

```rust
static mut A: () = {
    let mut y = None;
    let z;
    let mut done_y = false;
    loop {
        let x = &mut [1];  // < this array
        if done_y {
            z = x;
            break;
        }
        y = Some(x);
        done_y = true;
    }
    some_const_fn(y, z); // some_const_fn expects that y to not alias z.
};
```

r? @pnkfelix  @nikomatsakis

closes rust-lang#52671
cc rust-lang#51823
bors added a commit that referenced this pull request Aug 1, 2018
Rollup of 15 pull requests

Successful merges:

 - #52793 (Add test for NLL: unexpected "free region `` does not outlive" error )
 - #52799 (Use BitVector for global sets of AttrId)
 - #52809 (Add test for unexpected region for local data ReStatic)
 - #52834 ([NLL] Allow conflicting borrows of promoted length zero arrays)
 - #52835 (Fix Alias intra doc ICE)
 - #52854 (fix memrchr in miri)
 - #52899 (tests/ui: Add missing mips{64} ignores)
 - #52908 (Use SetLenOnDrop in Vec::truncate())
 - #52915 (Don't count MIR locals as borrowed after StorageDead when finding locals live across a yield terminator)
 - #52926 (rustc: Trim down the `rust_2018_idioms` lint group)
 - #52930 (rustc_resolve: record single-segment extern crate import resolutions.)
 - #52939 (Make io::Read::read_to_end consider io::Take::limit)
 - #52942 (Another SmallVec.extend optimization)
 - #52947 (1.27 actually added the `armv5te-unknown-linux-musleabi` target)
 - #52954 (async can begin expressions)

Failed merges:

r? @ghost
@bors bors merged commit 84dc485 into rust-lang:master Aug 1, 2018
@matthewjasper matthewjasper deleted the allow-zst-conflicts branch August 1, 2018 22:06
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.

ICE: mir_borrowck encountered mutable promoted
5 participants