- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
Implement parsing of pinned borrows #135731
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
Conversation
| rustbot has assigned @petrochenkov. Use  | 
| Some changes occurred to the CTFE machinery cc @rust-lang/wg-const-eval | 
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
ceaa211    to
    e57789f      
    Compare
  
    | Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt | 
| r? compiler | 
| 
 This is not the semantic we want or a good placeholder for it.  What we want is that  fn f<T: Unpin>(x: &mut T) {
    let _: Pin<&mut T> = &pin mut *x; // i.e., `Pin::new(&mut *x)`.
}That is, if the type in the place is  After that, we start talking about how to extend this to safe pin projection to struct fields. | 
| r? traviscross | 
| 
 I agree with you, and my previous statement was not precise. What I mean is similar to yours. I expect  The pinned places might be defined as: A place is a pinned place if one of the following conditions holds: 
 Here are a few examples to explain: #[derive(Default)]
struct Foo<T> {
    x: T,
}
fn f<T: Default, U: Default + Unpin>() {
   // Case 1: not `Unpin`, and not pinned
   let mut a = Foo::<T>::default(); // `a` and `a.x` are non-pinned places but not pinned places
   let _: &mut Foo = &mut a; // ok
   let _: &mut T = &mut a.x; // ok
   let _: Pin<&mut Foo> = &pin mut a; // ERROR
   let _: Pin<&mut T> = &pin mut a.x; // ERROR
   // Case 2: not `Unpin`, and pinned
   let pin mut b = Foo::<T>::default(); // `b` and `b.x` are pinned places but not non-pinned places
   let _: &mut Foo = &mut b; // ERROR
   let _: &mut Foo = &mut b.x; // ERROR
   let _: &pin mut Foo = &pin mut b; // ok
   let _: &pin mut Foo = &pin mut b.x; // ok
   // Case 3: `Unpin`, and not pinned
   let mut c = Foo::<U>::default(); // `c` and `c.x` are both pinned and non-pinned places
   let _: &mut Foo = &mut c; // ok
   let _: &mut T = &mut c.x; // ok
   let _: Pin<&mut Foo> = &pin mut c; // ok
   let _: Pin<&mut T> = &pin mut c.x; // ok
   // Case 4: `Unpin`, and pinned
   let pin mut d = Foo::<U>::default(); // WARN (useless `pin`) // `d` and `d.x` are both pinned and non-pinned places
   let _: &mut Foo = &mut d; // ok
   let _: &mut T = &mut d.x; // ok
   let _: Pin<&mut Foo> = &pin mut d; // ok (useless `pin`)
   let _: Pin<&mut T> = &pin mut d.x; // ok (useless `pin`)
} | 
| Thanks, that's helpful. Going through the examples, starting on Case 1, we're actually instead interested in this working: struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
    let _: Pin<&mut Foo<T>> = &pin mut x; //~ OK
    let _: &Foo<T> = &x; //~ OK
    let _: &mut Foo<T> = &mut x; //~ ERROR place has been pinned
    let _move: Foo<T> = x; //~ ERROR place has been pinned
}The  In this way,  This is also why we're not sold on doing  Regarding the safe pin projections to fields, let's do that in a separate PR so we can set those questions aside for the moment. That a place is pinned doesn't necessarily imply that we can safely pin project to a field without taking a number of other steps. | 
| Thanks for your feedback too! Regarding your code snippet, I have some questions. First, this is the semantics of  struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
    let _: Pin<&mut Foo<T>> = pin!(x); //~ OK
    let _: &Foo<T> = &x; //~ ERROR place has been moved
    let _: &mut Foo<T> = &mut x; //~ ERROR place has been moved
    let _move: Foo<T> = x; //~ ERROR place has been moved
}Then in your version,  
 What I expect is, the borrow checker should allow the original place to be mutably borrowed or moved after all pinned borrows expired (if we don't introduce  fn ignore<T>(_: T) {}
struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
    {
        let _pinned: Pin<&mut Foo<T>> = &pin mut x; //~ OK
        let _: &Foo<T> = &x; //~ OK
        // Mutable borrows or moves are not allowed during the pinned borrow is alive
        let _: &mut Foo<T> = &mut x; //~ ERROR place has been pinned
        let _move: Foo<T> = x; //~ ERROR place has been pinned
        // Make sure `_pinned` is used
        ignore(_pinned);
    }
    let _: &Foo<T> = &x; //~ OK
    // Mutable borrows or moves are allowed again after the pinned borrow expires
    let _: &mut Foo<T> = &mut x; //~ OK
    let _move: Foo<T> = x; //~ OK
} | 
e57789f    to
    1c081ea      
    Compare
  
    | 
 This can't be allowed.  The contract with  What the borrow checker must do here, then, is to flag the  struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
    let _: &mut Foo<T> = &mut x; //~ OK
    //~^ Before we pin the thing, it's OK for mutable references to
    //~| exist to it.
    //~|
    //~| Note that the above mutable reference is dead at this point.
    let _: Pin<&mut Foo<T>> = &pin mut x; //~ OK
    //~^ Now we create a pinned mutable reference.  This is allowed
    //~| since the earlier mutable reference is dead.  Once this
    //~| pinned reference is created, we can no longer allow the
    //~| pointed to memory to be invalidated or repurposed until
    //~| after `drop` has been run, even after this pinned reference
    //~| is dead.
    //~|
    //~| Note that the pinned reference is dead at this point.
    let _: &Foo<T> = &x; //~ OK
    //~^ We can allow shared references because they don't allow us
    //~| to invalidate or repurpose the memory.
    let _: Pin<&mut Foo<T>> = &pin mut x; //~ OK
    //~^ We can allow a new pinned mutable reference for the same
    //~| reason.
    let _: Pin<&Foo<T>> = &pin const x; //~ OK
    //~^ Or a new pinned shared reference, for that matter.
    let _: &mut Foo<T> = &mut x; //~ ERROR place has been pinned
    //~^ We can't allow a mutable reference to exist, though, because
    //~| that would violate the contract.
    let _move: Foo<T> = x; //~ ERROR place has been pinned
    //~^ Neither can we allow the once-pinned thing to be moved.
}What the  | 
| 
 Oh, yes, I forgot that  | 
| Yes, the intuition point is something we'll I'm sure consider later. We want to try it first in the way mentioned. | 
| What @traviscross said above sounds right to me. I would also mention that we want dispatch on  | 
| ☔ The latest upstream changes (presumably #138114) made this pull request unmergeable. Please resolve the merge conflicts. | 
276fa47    to
    cc5e4cb      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked @eholk to have a look here, as he and I had talked a lot about what we wanted for this as a lang matter. He had that look, and it looks OK to him there as an initial step toward where we're going. (Thanks to him for having a look.)
That's my sense too in looking this over now. I'd reviewed this more carefully before, and this seems to have made the adjustments that I'd asked for to align with where we're going.
Thanks to @frank-king for putting this together.  I'll hand it over to @compiler-errors.  Feel free to reassign if desired -- my suggestion would be @eholk in that case.  r=me along with your own review.
r? compiler-errors
cc5e4cb    to
    afdb54a      
    Compare
  
    | @bors r=eholk,traviscross | 
…holk,traviscross Implement parsing of pinned borrows This PR implements part of rust-lang#130494. EDIT: It introduces `&pin mut $place` and `&pin const $place` as sugars for `std::pin::pin!($place)` and its shared reference equivalent, except that `$place` will not be moved when borrowing. The borrow check will be in charge of enforcing places cannot be moved or mutably borrowed since being pinned till dropped. ### Implementation steps: - [x] parse the `&pin mut $place` and `&pin const $place` syntaxes - [ ] borrowck of `&pin mut|const` - [ ] support autoref of `&pin mut|const` when needed
Rollup of 15 pull requests Successful merges: - #135731 (Implement parsing of pinned borrows) - #138780 (Add `#[loop_match]` for improved DFA codegen) - #142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - #142633 (Error on invalid signatures for interrupt ABIs) - #142768 (Avoid a bitcast FFI call in transmuting) - #142825 (Port `#[track_caller]` to the new attribute system) - #142844 (Enable short-ice for Windows) - #142934 (Tweak `-Zmacro-stats` measurement.) - #142955 (Couple of test suite fixes for cg_clif) - #142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - #142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - #142982 (Corrected spelling mistake in c_str.rs) - #142983 (Taint body on invalid call ABI) - #142988 (Update wasm-component-ld to 0.5.14) - #142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #135731 - frank-king:feature/pin-borrow, r=eholk,traviscross Implement parsing of pinned borrows This PR implements part of #130494. EDIT: It introduces `&pin mut $place` and `&pin const $place` as sugars for `std::pin::pin!($place)` and its shared reference equivalent, except that `$place` will not be moved when borrowing. The borrow check will be in charge of enforcing places cannot be moved or mutably borrowed since being pinned till dropped. ### Implementation steps: - [x] parse the `&pin mut $place` and `&pin const $place` syntaxes - [ ] borrowck of `&pin mut|const` - [ ] support autoref of `&pin mut|const` when needed
Rollup of 15 pull requests Successful merges: - rust-lang/rust#135731 (Implement parsing of pinned borrows) - rust-lang/rust#138780 (Add `#[loop_match]` for improved DFA codegen) - rust-lang/rust#142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - rust-lang/rust#142633 (Error on invalid signatures for interrupt ABIs) - rust-lang/rust#142768 (Avoid a bitcast FFI call in transmuting) - rust-lang/rust#142825 (Port `#[track_caller]` to the new attribute system) - rust-lang/rust#142844 (Enable short-ice for Windows) - rust-lang/rust#142934 (Tweak `-Zmacro-stats` measurement.) - rust-lang/rust#142955 (Couple of test suite fixes for cg_clif) - rust-lang/rust#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - rust-lang/rust#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - rust-lang/rust#142982 (Corrected spelling mistake in c_str.rs) - rust-lang/rust#142983 (Taint body on invalid call ABI) - rust-lang/rust#142988 (Update wasm-component-ld to 0.5.14) - rust-lang/rust#142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
…kingjubilee Rollup of 15 pull requests Successful merges: - rust-lang#135731 (Implement parsing of pinned borrows) - rust-lang#138780 (Add `#[loop_match]` for improved DFA codegen) - rust-lang#142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - rust-lang#142633 (Error on invalid signatures for interrupt ABIs) - rust-lang#142768 (Avoid a bitcast FFI call in transmuting) - rust-lang#142825 (Port `#[track_caller]` to the new attribute system) - rust-lang#142844 (Enable short-ice for Windows) - rust-lang#142934 (Tweak `-Zmacro-stats` measurement.) - rust-lang#142955 (Couple of test suite fixes for cg_clif) - rust-lang#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - rust-lang#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - rust-lang#142982 (Corrected spelling mistake in c_str.rs) - rust-lang#142983 (Taint body on invalid call ABI) - rust-lang#142988 (Update wasm-component-ld to 0.5.14) - rust-lang#142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
…raviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of #130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (#135731).~ CC `@traviscross`
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC `@traviscross`
This PR implements part of #130494.
EDIT: It introduces
&pin mut $placeand&pin const $placeas sugars forstd::pin::pin!($place)and its shared reference equivalent, except that$placewill not be moved when borrowing. The borrow check will be in charge of enforcing places cannot be moved or mutably borrowed since being pinned till dropped.Implementation steps:
&pin mut $placeand&pin const $placesyntaxes&pin mut|const&pin mut|constwhen needed