Skip to content

Tracking Issue for cell_get_cloned #145329

Open

Description

@frank-king

View all comments

Feature gate: #![feature(cell_get_cloned)]

This is a tracking issue for Cell::get_cloned for T: CloneFromCopy (e.g. Rc<T>, Arc<T>, etc.).

ACP: rust-lang/libs-team#572

Public API

/// Safety: can call `Self::clone(&*ManuallyDrop::new(ptr::read(ptr_to_self)))`
#[marker]
pub unsafe trait CloneFromCell: Clone {}

// Can't do this, unsound
// unsafe impl<T: Copy> CloneFromCell for T {}

unsafe impl<T: ?Sized> CloneFromCell for Rc<T> {}
unsafe impl<T: ?Sized> CloneFromCell for Arc<T> {}
unsafe impl<T: ?Sized> CloneFromCell for rc::Weak<T> {}
unsafe impl<T: ?Sized> CloneFromCell for sync::Weak<T> {}

// basically all std types that don't have indirection:
unsafe impl<T: CloneFromCell, const N: usize> CloneFromCell for [T; N] {}
unsafe impl<T: CloneFromCell> CloneFromCell for Option<T> {}
unsafe impl<T: CloneFromCell, E: CloneFromCell> CloneFromCell for Result<T, E> {}
// ...

impl<T: CloneFromCell> Cell<T> {
    pub fn get_cloned(&self) -> Self;
}

Steps / History

(Remember to update the S-tracking-* label when checking boxes.)

Unresolved Questions

cc @rust-lang/libs-api @rust-lang/lang

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩

Activity

  1. added
    T-libs-api[DEPRECATED; DO NOT USE]
    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
    on Aug 13, 2025
  2. added
    I-lang-radarItems that are on lang's radar and will need eventual work or consideration.
    on Aug 13, 2025
  3. theemathas commented on Aug 13, 2025

    @theemathas
  4. frank-king commented on Aug 13, 2025

    @frank-king
    ContributorAuthor

    unsafe impl<T: Copy> CloneFromCopy for T {}

    This impl seems problematic. Cell::get_cloned has to call T::clone, which may run arbitrary user code, even for Copy types.

    T is copied (via ptr::read) before calling T::clone, and thus there's no reference to the copied T, so T::clone cannot do harm to it.

  5. ais523 commented on Sep 12, 2025

    @ais523
    /// Safety: can call `Self::clone(&*ManuallyDrop::new(ptr::read(ptr_to_self)))`
    #[marker]
    pub unsafe trait CloneFromCopy: Clone {}
    
    …
    
    impl<T: CloneFromCopy> Cell<T> {
        pub fn get_cloned(&self) -> Self;
    }

    I believe that, given the stated safety condition, this is unsound.

    Box<T> where T: Clone is an example of a type which can, according to the stated safety condition, implement CloneFromCopy (assume either that the special case that makes the copy itself UB gets removed, or that you are using a MyBox<T> that doesn't have such a case). The reason is that a copy of a Box is just a pointer to the contents of the box (like the original is), and you can clone via the copy of the pointer just as easily as you can clone via the original pointer.

    However, cloning a Box<T> can change the value of a Cell<Box<T>> (if T has a sufficiently perverse clone implementation). Doing that won't cause the copy of the pointer to be dropped – but it could cause the allocation that it points to to be dropped (e.g. the Clone implementation could take the Box from the Cell by swapping another Box into its place, then drop the Box it took, causing the copy of the Box to now be pointing to deallocated memory and subsequently causing a use-after-free).

    The correct safety requirement is unrelated to the ability to clone from a copy: rather, the requirement we want is that cloning does not write to any cells in a way that could cause the value inside a Cell<T> to be modified, moved out of the cell, or dropped. Cloning from a copy (rather than the original) doesn't actually solve any of these problems: modifying the value could drop something that the copy of the value references, as could dropping it, and moving it out of the cell makes it possible to modify or drop it without the cell being involved. However, once you enforce that more stringent safety requirement, there's no longer a need to actually do the copy out of the cell any more! So CloneFromCopy is probably the wrong trait to be using here – it should be some sort of CloneFromCell that asserts that the cloning operation doesn't change cells in a problematic way.

    (Some context: I've recently been doing a study into what Rust would look like if "create a new value by copying the bits of an existing value, even if the type isn't Copy" were supported by the language and type system as a first-class operation. One of the main conclusions was that to be usable, the copies generally have to have a lifetime – otherwise, you can't guarantee that things that the original value referenced will still be alive by the time you try to use the copy, and thus you can't soundly use them via the copy. That made me realise that the API written here doesn't work, because it doesn't specify a lifetime for how long the copy remains valid, and if you could guarantee that the copy were still valid you could just as easily guarantee that the Cell still had contents that could safely be used directly. Note that without such a lifetime, even Rc can't be cloned safely from a copy: the original Rc that you copied might have been dropped since, and it might have been the last reference.)

  6. frank-king commented on Sep 13, 2025

    @frank-king
    ContributorAuthor

    cloning does not write to any cells in a way that could cause the value inside a Cell to be modified, moved out of the cell, or dropped.

    I strongly agree with you. As from the ACPs #572 and #648, we started from the same point that we want Cell<Rc<T>> be assessed by clone (or by get), and besides Rc<T>, I've just thinking of more general cases, so I proposed a trait CellGet (now the CloneFromCopy) that may apply to types similar to Rc<T>. The current statement of the safety property is not accurate enough and I really appreciate your feedback.

    Is your statement an informal sketch or do you already have done some verification or proof?

    One of the main conclusions was that to be usable, the copies generally have to have a lifetime – otherwise, you can't guarantee that things that the original value referenced will still be alive by the time you try to use the copy, and thus you can't soundly use them via the copy.

    What an interesting conclusion. It sounds just like "borrowing" with auto-deref?

  7. ais523 commented on Sep 13, 2025

    @ais523

    Is your statement an informal sketch or do you already have done some verification or proof?

    It's based on the "cell permission" model, in which each cell is conceptually tied to a "permission object" (you can tie multiple cells to the same object but don't have to), and you can't read the cell unless you could safely shared-borrow its permission object, and can't write the cell unless you could safely exclusive-borrow its permission object. I haven't personally proven the model itself to be correct, but there is related work, which does contain proofs, e.g. ghost-cell is effectively an implementation of the cell permission model and the theory behind it has been formally proven (although I'm not sure whether or not there's a proof that the implementation actually matches the theory).

    The basic idea is that in order to read inside a cell, you have to shared-borrow its permission object, so you can't then do things that require an exclusive reference (write/move) to that same cell as that would require a conflicting borrow. It's difficult to express "the same cell" in the type system, but you can broaden it to "cells of the same type" and get a safety argument that way. I'd consider it more of an informal argument than anything – a formal model exists, but it hasn't been formally proven to be relevant in this context, and the way the model is applied to the situation hasn't been formally proven to be correct either.

    Thinking about what I just wrote, we might not need to explicitly ban drops; you can't drop the cell you're cloning from, because get_cloned takes a shared reference to the cell which prevents it being dropped, and if the cell isn't dropped you can't drop its contents without first overwriting them or moving them out (thus, banning moves and writes is enough).

    One of the main conclusions was that to be usable, the copies generally have to have a lifetime – otherwise, you can't guarantee that things that the original value referenced will still be alive by the time you try to use the copy, and thus you can't soundly use them via the copy.

    What an interesting conclusion. It sounds just like "borrowing" with auto-deref?

    It's definitely related – the primary difference is that the thing that is being borrowed doesn't necessarily need to exist in memory in a usable form at all (e.g. it might be in a register, or might be a field of a #[repr(packed)] struct and thus unreferenceable due to being misaligned). It's pretty similar to a shared reference, but based on values rather than addresses (thus there isn't a pointer behind it, like there would be for a reference, meaning that you can't implement the Deref trait: all you have is a bit pattern).

  8. tgross35 commented on Sep 16, 2025

    @tgross35
    Member

    Seeing some iterations of the implementation PR, I think CloneFromCopy may not be the best name because it seems to indicate some relation to Copy. However, (1) it doesn't apply to everything Copy (which of course implies Clone), and (2) it applies to many things that have nothing to do with Copy.

    Unhelpfully I don't have any great suggestions. PartialPureClone crossed my mind because Clone must be free of side effects for the local type (i.e. the Rc struct itself) but it may have limited side effects via indirection (i.e. the Rc counters). Or FreezeClone for similar reasons.

  9. programmerjake commented on Sep 16, 2025

    @programmerjake
    Member

    I think CloneFromCell is a pretty good name because it literally describes the operation we're trying to do, also because if you have a &Cell<T> you can also obtain a &Cell<Field> for each field of T (edit: stuff in parenthesis turns out to not work: #145329 (comment) this includes stuff like Deref but for like going from &Cell<Box<U>> to &Cell<U> or &Cell<&mut U> to &Cell<U>).

  10. programmerjake commented on Sep 16, 2025

    @programmerjake
  11. ais523 commented on Sep 16, 2025

    @ais523
  12. programmerjake commented on Sep 16, 2025

    @programmerjake
    Member

    The main issue with this version of CloneFromCell is that it doesn't compose well: knowing that T: CloneFromCell doesn't let you implement, e.g., CloneFromCell for Box<T> because the CloneFromCell implementation might write to the cell that contains the box being cloned.

    I thought it composes better than you might expect, because, assuming there's a safe way to go from &Cell<Box<T>> to &Cell<T> (Box<T>: DerefCell from above), T: CloneFromCell can't go messing with any Cell<Box<T>> that might be the passed in Cell. Same kind of thing for any other U: CloneFromCell that might contain a T and call T::clone_from_cell.

    Though now that I think about it more, &Cell<Box<T>> to &Cell<T> is not sound since you can obtain both shared references at once and then swap out and drop the Box<T> while the &Cell<T> is still pointing to it. So, yeah, CloneFromCell should be an unsafe trait and clone_from_cell would have to guarantee it doesn't modify anything that could possibly contain Self either directly or indirectly.

  13. tgross35 commented on Sep 24, 2025

    @tgross35
    Member

    Another concern brought up in the implementation PR: since this is not sound to implement for the general T: Copy but is sound for most Copy items, that means supporting this for all possible types means a manual impl for (almost) every Copy item in std and elsewhere. If API shape can be adjusted to allow a blanket T: Copy impl, that seems advantageous. If that is not possible, it might be worth reframing this as specifically for supporting types that aren't Copy rather than a general accessor (given Copy types can just .get()).

    The exact possibilities here will depend on whether or not the implementation here moves to the Clone impls rust-lang/libs-team#572 (comment).

  14. 32 remaining items

  15. rkjnsn commented on Feb 4, 2026

    @rkjnsn
    Contributor

    It was mentioned briefly above, but it would be very useful if CloneFromCell could be derivable. E.g., I currently have a struct containing a String, an Rc, an enum, and some Copy types, and it would be great if I could just slap #[derive(CloneFromCell)] on it and the enum to be able to use get_cloned without having to write unsafe myself.

    It would be necessary to prevent a derived CloneFromCell from calling a user-defined, potentially-problematic Clone implementation. I can think of two ways to accomplish this.

    1. Only allow deriving CloneFromCell when also deriving Clone.
    2. Don't use Clone when cloning from a cell, and instead provide a unsafe fn clone_from_cell(*const self) -> Self method on CloneFromCell as suggested above. That way the derived CloneFromCell would delegate directly to clone_from_cell on the fields without invoking the Clone implementation at all.

    (1) seems less surprising (cloning won't ever behave different from a cell than normally), while (2) provides a bit more flexibility (e.g., perhaps individually cloning all of the fields works, so deriving CloneFromCell would be valid, but some of them can be skipped, so a custom Clone makes sense).

  16. rkjnsn commented on Feb 4, 2026

    @rkjnsn
    Contributor

    Actually, this got me thinking: could #[derive(Clone)] use the trivial_bounds feature to automatically impl CloneFromCell iff all of the fields are CloneFromCell?

  17. added
    T-libsRelevant to the library team, which will review and decide on the PR/issue.
    and removed
    T-libs-api[DEPRECATED; DO NOT USE]
    on Aug 12, 2026
  18. ChayimFriedman2 commented on Aug 13, 2026

    @ChayimFriedman2
    Contributor

    Should this return Cell<T> or T? IMO returning Cell<T> is weird, most callers will just immediately call into_inner().

  19. ais523 commented on Aug 13, 2026

    @ais523

    The two are equally powerful (because you can write either in terms of the other, using Cell::into_inner or Cell::new), but at least in the cases where I've wanted to use this, returning a T is more natural than returning a Cell<T>. We should probably be optimizing for the common case (and I expect wanting a T to be the common case, but we might need more people to chime in in order to get a representative sample).

  20. programmerjake commented on Aug 13, 2026

    @programmerjake
    Member

    iirc it is/was intended that it would just be <Cell<T> as Clone>::clone so that's why it returns Cell<T> instead of T

  21. ais523 commented on Aug 13, 2026

    @ais523

    I think it makes sense to add it as a method instead of (or as well as) a Clone implementation: in the case where you just want a clone of the contents of the cell, cell.clone().into_inner() is a lot less readable than cell.get_cloned().

    Additionally, there are some problems with implementing it as a trait implementation (Cell already implements Clone, so if you have something that is both Copy and CloneFromCell, the two Clone implementations would conflict with each other, and in extreme cases using unsafe impl might be different). I think the restriction that an implementation of a stable trait on a stable type is insta-stable no longer exists, but if it does still exist, it would also be a problem.

  22. programmerjake commented on Aug 14, 2026

    @programmerjake
    Member

    we wanted to have T: Copy imply T: CloneFromCell via a blanket impl or something, so you don't have the problem of which one to use, if it impls Copy, then both impls will have identical effects

  23. ChayimFriedman2 commented on Aug 14, 2026

    @ChayimFriedman2
    Contributor

    But then found it's unsound? Well, I don't think there's a way to work around the unsoundness (except maybe using specialization which is its own can of worms and also I'm not sure that really solves it), so it'll have to stay a method and therefore IMO should return T.

  24. programmerjake commented on Aug 14, 2026

    @programmerjake
    Member

    But then found it's unsound? Well, I don't think there's a way to work around the unsoundness (except maybe using specialization which is its own can of worms and also I'm not sure that really solves it), so it'll have to stay a method and therefore IMO should return T.

    iirc the unsoundness is around CloneFromCopy doing weird stuff in user Clone impls, Copy doesn't have that problem because we can just skip calling user code entirely.

  25. ChayimFriedman2 commented on Aug 14, 2026

    @ChayimFriedman2
    Contributor

    Yes, it's unsound to have CloneFromCell implemented for any Copy, that's what I meant.

  26. ais523 commented on Aug 14, 2026

    @ais523

    The unsoundness is that not all Copy types can be cloned from a cell, because they might have a Clone implementation that does something other than copying them. This means that to avoid the unsoundness, .get_cloned would have to be written in terms of copies for Copy types but clones for types which aren't Copy, which is specialization (and probably an unsound specialization, at that).

  27. Jules-Bertholet commented on Aug 14, 2026

    @Jules-Bertholet
    Contributor

    I have argued for a while that we should guarantee that specialization (and disagree that it is unsound). See Zulip discussion: https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/.60TrivialClone.60.20sadness/with/616436432

  28. Plecra commented on Oct 19, 2026

    @Plecra

    Ah, I see the problem after running through my mental model of Cell (which i want to formalize some time) and combining it with the lack of runtime type information. You need at least some type information for Cell::<Rc<T>>::get_cloned to be sound, because it's unsound if someone does the following:

    1. Repeatedly clone and forget an `Rc` until its reference count equals its memory location (possible in safe Rust)
    
    2. Write a `Cell<Rc>` to that `Rc` over the reference count in order to give it provenance (not possible in safe Rust, but conceptually a no-op because you're overwriting a value with itself, and the reference count its itself a cell)
    
    3. Shared-borrow the `Cell<Rc>` you just wrote to the reference count (i.e. the cell itself, not its inside; conceptually sound because there are no mutable borrows of it to conflict with, and changing the inside of the cell while it's shared-borrowed is legal so there are no conflicting writes)
    
    4. `get_cloned` on the newly borrowed cell (possible in safe Rust, with this feature)
    
    5. `get_cloned` through the same borrow cell again (stil possible in safe Rust, with this feature)
    

    Step 4 doesn't violate any aliasing rules, nor does it do anything impossible to the original Cell. However, step 4 causes the "inside" of the borrow at step 3 to be pointing to the wrong place, so step 5 causes an attempt to write through a misaligned pointer.

    This chain of events is clearly ridiculous, but it's hard to formalize specifically what is wrong with it and which step to blame. I intuitively want to blame step 2 – my mental model of the situation says that it is safe for Cell::<Rc<T>>::get_cloned to increment the cell's reference count, because it's an integer and an integer can't also be an Rc. But with untyped memory, it may not be possible to make that argument.

    I know it's been a while, but I wanted to add to this.

    I would strongly suggest that (3) is the library unsoundness: This is not respecting the invariants of Rc, which requires that all shared references to it are viewing it at a compatible type (that is, that there does not exist a type in the program which believes it can write to the value of the Rc at a non-Rc type). Simply writing a type to a location isn't enough to justify taking a long-lived reference to that location. In this case, our &Cell<Rc> is only valid until the next possible relevant write

    This method does not write to any cells (Cell, UnsafeCell, etc.) in a way which could write to or move out a value of type Self

    My understanding is that the example was generally intended to justify this through this argument. Pointing the finger at 3 is still generally compatible with what you're saying - "next possible relevant write" is linked to the cell permission model you're talking about (I've been using something really similar, its cool to see it in the wild!)

    I think there's an alternative way to approach the framing of the invariant -

    For the duration of the call, the target is aliased as-if a &Self was live, and must assume that any memory that could contain it with Cell's invariants is temporarily substituted for shared access
    note: In particular, this means it must only have effects permissible via shared access to the content of any such memory

    (I dont know the specific wording being used in current memory model descriptions :) This is only intended as an idea of which objects to focus on, the details can change)

    The above example remains unsound by the unjustified cast to &Cell<Rc> - this safety comment isn't especially relevant, it's true that arbitrary &Cell<usize>s can't be held as &Cell<Rc<T>>s spanning code that could write to any aliased (therefore necessarily treated as Cell-invariants on this thread) memory.

    It's a little evasive to cite "Cell's invariants" without a specific definition, but in a strict sense they're well defined by whatever invariants are established by the Cell constructors. Also seems useful to note that (1) any Cells at more restrictive types couldnt be aliases and (2) any Cells at less restrictive types couldnt be aliases. Either way, a write of the supertype (in the memory sense, Box<T: Sized> <: usize) could corrupt the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCI-lang-radarItems that are on lang's radar and will need eventual work or consideration.S-tracking-unimplementedStatus: The feature has not been implemented.T-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions