Add `mem::needs_clone`

[joboet]

Proposal

Problem statement

The standard library uses specialisation to speed up certain operations like data structure cloning or slice filling in cases where the Clone implementation is equivalent to a copy. As evidenced by cases like hashbrown doing the same thing on nightly, it would be helpful for user crates to be able to do this, too.

Unfortunately, even if specialisation were stabilised, this would still not be possible on stable. Since rust-lang/rust#135634, std uses the TrivialClone marker trait for this specialisation, which is automatically implemented by user types that use #[derive(Clone, Copy)]. Thus, TrivialClone must not be stabilised, since that would make removing #[derive(Clone, Copy)] in favour of a manual implementation a breaking change¹.

Solution sketch

Introduce a needs_clone function that returns false only if Clone is definitely side-effect-free:

// in core::mem

const fn needs_clone<T: CloneToUninit + ?Sized>() -> bool;

Just like the presence of a side-effect-free Drop can be detected with needs_drop this would allow user code to skip calling clone in favour of performing a bitwise copy by branching on the return value.

Alternatives

Stabilise TrivialClone, but only allow it in specialisation bounds.

Links and related work

• The PR introducing TrivialClone: stop specializing on Copy rust#135634

Footnotes

1. And even if TrivialClone were implemented differently, this would still be a major semver-hazard. ↩

[programmerjake]

how would this work transitively for types like so (maybe oversimplified):

// assume we can't use derives for some reason
struct S<T>(T);

impl<T: Clone> Clone for S<T> {
    fn clone(&self) -> Self {
        if needs_clone::<T>() {
            Self(self.0.clone())
        } else {
            // fast path
            unsafe { std::ptr::read(self) }
        }
    }
}

// does S::<S<u8>>::clone take the fast path?

that kind of thing can be easily handled by specialization:

struct S<T>(T);
unsafe impl<T: TrivialClone> TrivialClone for S<T> {}

impl<T: Clone> Clone for S<T> {
    fn clone(&self) -> Self {
        SpecClone::spec_clone(&self)
    }
}

trait SpecClone {
    fn spec_clone(&self) -> Self;
}

impl<T: Clone> SpecClone for S<T> {
    default fn spec_clone(&self) -> Self {
        Self(self.0.clone())
    }
}

impl<T: Clone + TrivialClone> SpecClone for S<T> {
    fn spec_clone(&self) -> Self {
        unsafe { std::ptr::read(self) }
    }
}

// S::<S<u8>>::clone takes the fast path due to the transitive TrivialClone impl.

[joboet]

Initially, this would conservatively return true, which is always sound. But eventually a cleverer analysis might employ coinduction to prove that false is correct.

[kennytm]

this should be a const fn right?

also i think the bound should be T: ?Sized + CloneToUninit

[joboet]

this should be a const fn right?

Oh right, obviously.

also i think the bound should be T: ?Sized + CloneToUninit

CloneToInit for slices is already optimised for trivially clonable elements, so I don't think there are that many cases where this is profitable. But sure, why not...

[the8472]

If this needs new analysis in rustc then I think this needs to be brought to T-compiler.

[joboet]

It doesn't, at least not necessarily. My initial implementation is basically just

#![feature(const_trait_impl)]
#![feature(trivial_clone)]
#![feature(min_specialization)]

use std::clone::TrivialClone;

pub const fn needs_clone<T: Clone>() -> bool {
    const trait NeedsClone {
        fn needs_clone() -> bool;
    }

    impl<T: Clone> const NeedsClone for T {
        default fn needs_clone() -> bool {
            true
        }
    }

    impl<T: TrivialClone> const NeedsClone for T {
        fn needs_clone() -> bool {
            false
        }
    }

    <T as NeedsClone>::needs_clone()
}

[the8472]

That isn't much better because it results in public API depending on specialization. So this would commit us to having something, either specialization or a dedicated analysis.

[joboet]

That isn't much better because it results in public API depending on specialization. So this would commit us to having something, either specialization or a dedicated analysis.

As stated above, always returning true is a valid implementation of needs_clone. So this would only imply that the compiler can figure out something better, but not actually require that to be true. It's a similar situation as with needs_drop. We could always remove the compiler part of needs_drop and make it unconditionally return true if it turns out to be unprofitable in the future – but that's unlikely.

[the8472]

We wouldn't add this method if it always returned true, so I don't think that's a good argument.

[Amanieu]

cc @rust-lang/types since this involves specialization. I believe it is sound since it relies on TrivialClone rather than Copy, but it does require exposing some of the specialization behavior directly.

Other concerns raised in the @rust-lang/libs-api meeting were:

• it's not clear what path TrivialClone has towards stabilization.
• if we end up having to always return true anyways, we don't really want to stabilize this.

[BoxyUwU]

With my types hat on I would very much prefer for libs to not stabilize APIs which depend on specialization (or more generally, unstable type system features). I can't speak for the whole team of course but thats my 2c

[oli-obk]

I think we can just add a new intrinsic and let the compiler figure out the answer like we do for needs_drop. Could likely also just use the new intrinsic added by rust-lang/rust#150033 (the intrinsic does not require T: 'static, unlike the public API from that PR).

[jackh726]

I also am an agreement with @BoxyUwU here: I do not want libs features to rely on specialization for a public API. An intrinsic would be okay and better, but still feels pretty not-great to me. It feels like a poor solution to work around the fact that we haven't figured out specialization (or, more accurately, the underlying lang feature(s) that people want specialization to be).

What makes me slightly less uncomfortable here is the trivial fallback of "just return true".

As a more libs-y thought: this feels like the wrong intrinsic - I think it seems better to have something like is_pure_clone or something.