tweak intrusive collection post some more and publish it
authorRalf Jung <post@ralfj.de>
Tue, 10 Apr 2018 11:56:24 +0000 (13:56 +0200)
committerRalf Jung <post@ralfj.de>
Tue, 10 Apr 2018 12:00:07 +0000 (14:00 +0200)
ralf/_posts/2018-04-10-safe-intrusive-collections-with-pinning.md [moved from ralf/_drafts/safe-intrusive-collections-with-pinning.md with 93% similarity]

@@ -1,6 +1,6 @@
 ---
 title: "Safe Intrusive Collections with Pinning"
-categories: rust
+categories: research rust
 ---
 
 In my [last post]({{ site.baseurl }}{% post_url 2018-04-05-a-formal-look-at-pinning %}), I talked about the new ["pinned references"](https://github.com/rust-lang/rfcs/blob/master/text/2349-pin.md) which guarantee that the data at the memory it points to will not, ever, be moved elsewhere.
@@ -428,8 +428,9 @@ It seems, after all, that the fourth typestate arises anyway.
 We have to make a decision here, before stabilizing `Pin`: Do we want `insert` with a shared reference to be legal?
 Or do want to declare that `&Pin<T>` and `&&T` are interchangeable types?
 One may argue that adding an entry to a collection logically changes that entry, so really this should require an exclusive pinned reference.
-However, given that the changed API seems entirely safe to use, I have to admit that's a somewhat weak argument.
-Converting between `&Pin<T>` and `&&T` seems to have no tangible benefit other than satisfying my model.
+However, given that the changed collection API seems entirely safe to use, I have to admit that's a somewhat weak argument.
+If the discussion so far is any indication, people want to write such APIs and will probably not be happy if we tell them that's illegal because there is a way to convert between `&Pin<T>` and `&&T`.
+After all, that conversion seems to have no tangible benefit other than satisfying my model.
 
 For these reasons, I now think that removing the shared pinned reference may have been a mistake: It turns out that, if we want to allow an intrusive collection like the above that works with `&Pin`, we need the fourth typestate anyway.
 At that point we might as well have a type reflecting it, avoiding the confusion and the double indirection that comes with `&Pin`.
@@ -438,8 +439,17 @@ That just seems like the worst of both worlds.
 
 However, now we already have a [version of the futures crate](https://aturon.github.io/2018/04/06/futures2/) using the revised `Pin`, so I don't know if changing it again is realistic. :/
 I feel bad about that.  Can we still fix this before everything gets stabilized?
-Others [have](https://github.com/rust-lang/rfcs/pull/2349#issuecomment-373206171) [argued](https://github.com/rust-lang/rfcs/pull/2349#issuecomment-378555691) for a shared pinned reference after it got removed from the API, and I am repentantly joining their choir now after arguing against them previously.
-Sorry for that, and thanks for not being convinced by me!
+Others [have](https://github.com/rust-lang/rfcs/pull/2349#issuecomment-373206171) [argued](https://github.com/rust-lang/rfcs/pull/2349#issuecomment-378555691) for a shared pinned reference after it got removed from the API, and I argued against them as I did not understand how it could be useful.
+Thanks for not being convinced by me!
+
+However, there is one strange aspect to this "shared pinned" typestate: Due to [`Pin::deref`](https://doc.rust-lang.org/beta/std/mem/struct.Pin.html#method.deref), we can turn a "shared pinned" reference into a shared reference.
+We can go from `&Pin<T>` to `&&T`.
+In other words, the shared pinned typestate invariant must *imply* the invariant for the (general, unpinned) shared typestate.
+The reason for `Pin::deref` to exist is mostly a rustc implementation detail [related to using `Pin<Self>` as the type of `self`](https://github.com/rust-lang/rfcs/pull/2349#issuecomment-372475895), but this detail has some significant consequences: Even with a separate shared pinned typestate, we can still not define `RefCell` in a way that a pinned `RefCell` pins its contents.
+In particular, we cannot have a method like `fn get_pin(self: Pin<RefCell<T>>) -> Pin<T>`.
+Removing `Pin::deref` (or restricting it to types that implement `Unpin`) would solve this issue.
+I spelled out the details [in the RFC issue](https://github.com/rust-lang/rfcs/pull/2349#issuecomment-372109981).
+So, if we want to declare that shared pinning is a typestate in its own right---which overall seems desirable---do we want it to be restricted like this due to an implementation detail of arbitrary self types?
 
 ## 4 Conclusion
 
@@ -448,4 +458,9 @@ Back when I was learning Rust, I remember thinking how intrusive collections wit
 I am happy to learn that I was wrong!
 I am impressed by the creativity that went into coming up with these APIs, and looking forward to analyzing more of them in the future.
 
+The situation around shared pinning is still open, and it seems we need to have a discussion about what model we ultimately want to adopt---which code we ultimately want to be legal---and whether we want to change the `Pin` types before stabilization.
+Unfortunately I am four days late in my race against the [first significant user of this API](https://aturon.github.io/2018/04/06/futures2/).
+
+Anyway, as usual, please let me know what you think!
+
 #### Footnotes