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

UnsafePinned: allow aliasing of pinned mutable references #3467

Merged
merged 35 commits into from
Jun 18, 2024
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
51bed0f
UnsafeAliased: port text from hackmd
RalfJung Aug 1, 2023
d599c1e
typos and nits
RalfJung Aug 1, 2023
c2bbec7
polyfill; Send/Sync question
RalfJung Aug 1, 2023
60ca0fa
remove some wrong cells
RalfJung Aug 2, 2023
6910494
detail example
RalfJung Aug 4, 2023
a1a05fb
Unpin hack migration plan
RalfJung Aug 4, 2023
a10d70c
remove stable feature gate
RalfJung Aug 5, 2023
9535a37
add alternative: drop aliasing rules for mutable references
RalfJung Aug 7, 2023
06b52df
link to real-world intrusive linked list example
RalfJung Aug 9, 2023
0369352
add get_mut_pinned method
RalfJung Aug 10, 2023
3e97a7e
fix typo
RalfJung Aug 15, 2023
a8f2516
add !Unpin impl to UnsafeAliased
RalfJung Aug 15, 2023
a55adde
add some links to prior discussions
RalfJung Sep 13, 2023
5f03b25
rename UnsafeAliased → UnsafePinned
RalfJung Nov 4, 2023
ce937b8
explain the naming choice
RalfJung Nov 4, 2023
60c255f
update UnsafeUnpin trait notes
RalfJung Nov 4, 2023
9881c94
explain why this is so much more awkward than UnsafeCell
RalfJung Nov 11, 2023
36b694d
be more clear about the soundness issues around '&mut UnsafePinned'
RalfJung Nov 25, 2023
7158ce2
also explain a non-alternative
RalfJung Nov 25, 2023
5ff0df8
fix a typo: mutation -> aliasing
RalfJung Nov 26, 2023
086412d
we should block niches
RalfJung Nov 26, 2023
7190a78
drawback: losing too much noalias
RalfJung Nov 28, 2023
d793712
elaborate on `Unpin + !UnsafeUnpin`
RalfJung Nov 28, 2023
9327537
fix typo
RalfJung Dec 22, 2023
86e0188
add example for why this is tied to pinning
RalfJung Feb 10, 2024
14b9019
update UnsafePinned docs regarding public API exposure
RalfJung Feb 11, 2024
5b2d690
Send and Sync
RalfJung Mar 10, 2024
1f31c7b
fix code example
RalfJung May 3, 2024
76a4035
make UnsafePinned derive Copy, Send, Sync
RalfJung May 3, 2024
bfb8c4b
expand back-compat note
RalfJung May 3, 2024
e98f367
add fixed code example
RalfJung May 29, 2024
474389f
add open question around naming
RalfJung May 29, 2024
3efb695
update filename and RFC number
RalfJung Jun 17, 2024
519aeb6
Clean up trailing whitespace
traviscross Jun 18, 2024
b950de5
Add tracking issue for RFC 3467
traviscross Jun 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
explain the naming choice
  • Loading branch information
RalfJung committed Nov 4, 2023
commit ce937b88c494ebd52751972b3ae46890ea7f1a4d
18 changes: 11 additions & 7 deletions text/0000-unsafe-aliased.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,16 @@ Here is a [polyfill on current Rust](https://play.rust-lang.org/?version=stable&

### Comparison with some other types that affect aliasing

- `UnsafeCell`: disables aliasing (and affects but does not fully disable dereferenceable) behind shared refs, i.e. `&UnsafeCell<T>` is special. `UnsafeCell<&T>` (by-val, fully owned) is not special at all and basically like `&T`; `&mut UnsafeCell<T>` is also not special.
- `UnsafePinned`: disables aliasing (and affects but does not fully disable dereferenceable) behind mutable refs, i.e. `&mut UnsafePinned<T>` is special. `UnsafePinned<&mut T>` (by-val, fully owned) is not special at all and basically like `&mut T`; `&UnsafePinned<T>` is also not special.
- `UnsafeCell`: disables aliasing (and affects but does not fully disable dereferenceable) behind shared refs, i.e. `&UnsafeCell<T>` is special. `UnsafeCell<&T>` (by-val, fully owned) is not special at all and basically like `&T`; `&mut UnsafeCell<T>` is also not special. Safe wrappers around this type can expose mutability behind shared references, such as `&RefCell<T>`.
- `UnsafePinned`: disables aliasing (and affects but does not fully disable dereferenceable) behind mutable refs, i.e. `&mut UnsafePinned<T>` is special. `UnsafePinned<&mut T>` (by-val, fully owned) is not special at all and basically like `&mut T`; `&UnsafePinned<T>` is also not special. Safe wrappers around this type can expose sharing that involves *pinned* mutable references, such as `Pin<&mut MyFuture>`.
- [`MaybeDangling`](https://github.com/rust-lang/rfcs/pull/3336): disables aliasing and dereferencable *of all references (and boxes) directly inside it*, i.e. `MaybeDangling<&[mut] T>` is special. `&[mut] MaybeDangling<T>` is not special at all and basically like `&[mut] T`.

# Drawbacks
[drawbacks]: #drawbacks

It's yet another wrapper type adjusting our aliasing rules and very easy to mix up with `UnsafeCell` or [`MaybeDangling`](https://github.com/rust-lang/rfcs/pull/3336).
Furthermore, it is an extremely subtle wrapper type, as the `duplicate` example shows.
`Unique` is a very strange trait, since it does not come with much of a safety requirement -- unlike `Freeze`, which quite clearly expresses some statement about the invariant of a type when shared, it is much less clear what the corresponding statement would be for `Unique`. (More work on RustBelt is needed to determine the details here.)
`UnsafeUnpin` is a very strange trait, since it does not come with much of a safety requirement -- unlike `Freeze`, which quite clearly expresses some statement about the invariant of a type when shared, it is much less clear what the corresponding statement would be for `UnsafeUnpin`. (More work on RustBelt is needed to determine the details here.)

# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives
Expand All @@ -338,7 +338,7 @@ However, of course one could imagine alternatives:
- Keep the status quo. The current sitaution is that we only make aliasing requirements on mutable references if the type they point to is `Unpin`. This is unsatisfying: `Unpin` was never meant to have this job. A consequence is that a stray `impl Unpin` on a `Wrapper<T>`-style type can [lead to subtle miscompilations](https://internals.rust-lang.org/t/surprising-soundness-trouble-around-pollfn/17484/15) since it re-adds aliasing requirements for the inner `T`.
Contrast this with the `UnsafeCell` situation, where it is not possible for (stable) code to just `impl Freeze for T` in the wrong way -- `UnsafeCell` is *always* recognized by the compiler.

On the other hand, `UnsafePinned` is rather quirky in its behavior and having two marker traits (`Unique` and `Unpin`) might be too confusing, so sticking with `Unpin` might not be too bad in comparison.
On the other hand, `UnsafePinned` is rather quirky in its behavior and having two marker traits (`UnsafeUnpin` and `Unpin`) might be too confusing, so sticking with `Unpin` might not be too bad in comparison.

If we do that, however, it seems preferrable to transition `Unpin` to an unsafe trait. There *is* a clear statement about the types' invariants associated with `Unpin`, so an `impl Unpin` already comes with a proof obligation. It just happens to be the case that in a module without unsafe, one can always arrange all the pieces such that the proof obligation is satisifed.
This is mostly a coincidence and related to the fact that we don't have safe field projections on `Pin`. That said, solving this also requires solving the trouble around `Drop` and `Pin`, where effectively an `impl Drop` does an implicit `get_mut_unchecked`, i.e., implicitly assumes the type is `Unpin`.
Expand All @@ -352,6 +352,12 @@ However, of course one could imagine alternatives:
But that is completely against the direction Rust has had for 8 years now, and it would mean removing LLVM `noalias` annotations for mutable references (and likely boxes) entirely.
That is sacrificing optimization potential for the common case in favor of simplifying niche cases such as self-referential structs -- which is against the usual design philosophy of Rust.

### Naming

An earlier proposal suggested to call the type `UnsafeAliased`, since the type is not inherently tied to pinning.
However, it is not possible to write wrappers around `UnsafeAliased` such that we can have mutation behind `&mut MyType`. One has to use pinning for that: `Pin<&mut MyType>`.
Because of that, the RFC proposes that we suggestively put pinning into the name of the type, so that people don't confuse it with a general mechanism for aliasing mutable references.
It is more like the core primitive behind pinning, where whenever a type is pinned that is caused by an `UnsafePinned` field somewhere inside it.

# Prior art
[prior-art]: #prior-art
Expand All @@ -367,9 +373,7 @@ Adding something like this to Rust has been discussed many times throughout the

- How do we transition code that relies on `Unpin` opting-out of aliasing guarantees to the new type? Here's a plan: define the `PhantomPinned` type as `UnsafePinned<()>`.
Places in the standard library that use `impl !Unpin for` and the generator lowering are adjusted to use `UnsafePinned` instead.
Then as long as nobody outside the standard library used the unstable `impl !Unpin for`, switching the `noalias`-opt-out to the `Unique` trait is actually backwards compatible with the (never explicitly supported) `Unpin` hack!
- The name of the type needs to be bikeshed. `UnsafePinned` might be too close to `UnsafeCell`, but that is a deliberate choice to indicate that this type has an effect when it appears in the *pointee*, unlike types like `MaybeUninit` or [`MaybeDangling`](https://github.com/rust-lang/rfcs/pull/3336) that have an effect on aliasing rules when wrapped around the *pointer*.
Like `UnsafeCell`, the aliasing allowed here is "interior". Other possible names: `UnsafeSelfReferential`, `UnsafePinned`, ...
Then as long as nobody outside the standard library used the unstable `impl !Unpin for`, switching the `noalias`-opt-out to the `UnsafeUnpin` trait is actually backwards compatible with the (never explicitly supported) `Unpin` hack!
- Relatedly, in which module should this type live?
- Should this type `derive(Copy)`? `UnsafeCell` does not, which is unfortunate because it now means some people might use `T: Copy` as indication that there is no `UnsafeCell` in `T`.
- `Unpin` [also affects the `dereferenceable` attribute](https://github.com/rust-lang/rust/pull/106180), so the same would happen for this type. Is that something we want to guarantee, or do we hope to get back `dereferenceable` when better semantics for it materialize on the LLVM side?
Expand Down