From f3a5e11c5cf7bdd74270a5509091cee3ee9fba86 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Apr 2018 13:56:24 +0200 Subject: [PATCH] tweak intrusive collection post some more and publish it --- ...afe-intrusive-collections-with-pinning.md} | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) rename ralf/{_drafts/safe-intrusive-collections-with-pinning.md => _posts/2018-04-10-safe-intrusive-collections-with-pinning.md} (93%) diff --git a/ralf/_drafts/safe-intrusive-collections-with-pinning.md b/ralf/_posts/2018-04-10-safe-intrusive-collections-with-pinning.md similarity index 93% rename from ralf/_drafts/safe-intrusive-collections-with-pinning.md rename to ralf/_posts/2018-04-10-safe-intrusive-collections-with-pinning.md index 3074503..9ae5bd5 100644 --- a/ralf/_drafts/safe-intrusive-collections-with-pinning.md +++ b/ralf/_posts/2018-04-10-safe-intrusive-collections-with-pinning.md @@ -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` 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` 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` 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` 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` 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>) -> Pin`. +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 -- 2.30.2