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

Permit mutation of &mut pointers whose pointees have been borrowed #10520

Closed
nikomatsakis opened this issue Nov 16, 2013 · 7 comments
Closed
Labels
A-borrow-checker Area: The borrow checker NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-medium Medium priority

Comments

@nikomatsakis
Copy link
Contributor

Now that the swap operator has been removed, I do not believe the
restriction against mutating LV is needed, and in fact it prevents
some useful patterns. For example, the following function will
fail to compile:

       fn mut_shift_ref<'a,T>(x: &mut &'a mut [T]) -> &'a mut T {
           // `mut_split` will restrict mutation against *x:
           let (head, tail) = (*x).mut_split(1);

           // Hence mutating `*x` yields an error here:
           *x = tail;
           &mut head[0]
       }

Note that this function -- which adjusts the slice *x in place so
that it no longer contains the head element and then returns a
pointer to that element separately -- is perfectly valid. It is
currently implemented using unsafe code. I believe that now that
the swap operator is removed from the language, we could liberalize
the rules and make this function be accepted normally. The idea
would be to have the assignment to *x kill the loans of *x and
its subpaths -- after all, those subpaths are no longer accessible
through *x, since it has been overwritten with a new value. Thus
those subpaths are only accessible through prior existing borrows
of *x, if any. The danger of the swap operator was that it
allowed *x to be mutated without making the subpaths of *x
inaccessible: worse, they became accessible through a new path (I
suppose that we could have supported a swap operator, too, if needed, by moving the loans over to the new path).

@pnkfelix
Copy link
Member

cc me

@nikomatsakis
Copy link
Contributor Author

Issue #11558 includes another example.

@huonw
Copy link
Member

huonw commented Aug 4, 2014

Triage, no change that I know of.

@nikomatsakis
Copy link
Contributor Author

This may have become more important. As part of #20341, we started doing coercions on assignments of the form:

x = y

If x has type &mut X, then this causes a reborrow of y whereas it used to cause a move. Unfortunately, this can break code that used to work. Here is a reduced example:

fn foo(mut vec: Vec<int>) {
    let mut iter = vec.iter_mut();
    let mut cur = iter.next().unwrap();
    let mut next = iter.next().unwrap();
    loop {
        *next = 22; // ERROR cannot assign to `next` because it is borrowed
        cur = next; // NOTE borrowed here
        next = iter.next().unwrap();
    }
}
fn main() {}

This can be worked around by forcing a move using a dummy newtype:

struct Dummy<T>(T);

fn foo(mut vec: Vec<isize>) {
    let mut iter = vec.iter_mut();
    let mut cur = Dummy(iter.next().unwrap());
    let mut next = iter.next().unwrap();
    loop {
        *next = 22;
        cur = Dummy(next);
        next = iter.next().unwrap();
    }
}
fn main() {}

But that's awful!

@nikomatsakis
Copy link
Contributor Author

Nominating because it may be worth trying to fix this for ergonomic reasons (or else maybe reverting the coercion change).

@brson
Copy link
Contributor

brson commented Jul 14, 2016

Triage: closing. Old wishlist. Do RFC.

@brson brson closed this as completed Jul 14, 2016
nivkner added a commit to nivkner/rust that referenced this issue Oct 7, 2017
update FIXME(rust-lang#6298) to point to open issue 15020
update FIXME(rust-lang#6268) to point to RFC 811
update FIXME(rust-lang#10520) to point to RFC 1751
remove FIXME for emscripten issue 4563 and include target in `test_estimate_scaling_factor`
remove FIXME(rust-lang#18207) since node_id isn't used for `ref` pattern analysis
remove FIXME(rust-lang#6308) since DST was implemented in rust-lang#12938
remove FIXME(rust-lang#2658) since it was decided to not reorganize module
remove FIXME(rust-lang#20590) since it was decided to stay conservative with projection types
remove FIXME(rust-lang#20297) since it was decided that solving the issue is unnecessary
remove FIXME(rust-lang#27086) since closures do correspond to structs now
remove FIXME(rust-lang#13846) and enable `function_sections` for windows
remove mention of rust-lang#22079 in FIXME(rust-lang#22079) since this is a general FIXME
remove FIXME(rust-lang#5074) since the restriction on borrow were lifted
@shepmaster
Copy link
Member

shepmaster commented May 18, 2018

Zombie revivification. The updated example is

fn foo(mut vec: Vec<i32>) {
    let mut iter = vec.iter_mut();
    let mut cur = iter.next().unwrap();
    let mut next = iter.next().unwrap();
    loop {
        *next = 22;
        cur = next;
        next = iter.next().unwrap();
    }
}
fn main() {}

And error:

error[E0506]: cannot assign to `*next` because it is borrowed
 --> src/main.rs:6:9
  |
6 |         *next = 22;
  |         ^^^^^^^^^^ assignment to borrowed `*next` occurs here
7 |         cur = next;
  |               ---- borrow of `*next` occurs here

error[E0499]: cannot borrow `*next` as mutable more than once at a time
  --> src/main.rs:7:15
   |
7  |         cur = next;
   |               ^^^^ mutable borrow starts here in previous iteration of loop
...
10 | }
   | - mutable borrow ends here

error[E0506]: cannot assign to `next` because it is borrowed
 --> src/main.rs:8:9
  |
7 |         cur = next;
  |               ---- borrow of `next` occurs here
8 |         next = iter.next().unwrap();
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to borrowed `next` occurs here

This code now compiles with NLL enabled.

@shepmaster shepmaster added the NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. label May 18, 2018
flip1995 pushed a commit to flip1995/rust that referenced this issue Mar 24, 2023
Use uninit checking from rustc

rustc has proper heuristics for actually checking whether a type allows being left uninitialized (by asking CTFE). We can now use this for our helper instead of rolling our own bad version with false positives.

I added this in rustc in rust-lang#108669

Fix rust-lang#10407

changelog: [`uninit_vec`]: fix false positives
changelog: [`uninit_assumed_init`]: fix false positives
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 6, 2023
In uninit checking, add fallback for polymorphic types

After rust-lang#10520, we always assumed that polymorphic types do not allow to be left uninitialized. But we can do better, by peeking into polymorphic types and adding a few special cases for going through tuples, arrays (because the length may be polymorphic) and blanket allowing all unions (like MaybeUninit).

fixes rust-lang#10551

changelog: [uninit_vec]: fix false positive for polymorphic types
changelog: [uninit_assumed_init]: fix false positive for polymorphic types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants