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

Simplify lightweight clones, including into closures and async blocks #3680

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Aug 20, 2024

Provide a feature to simplify performing lightweight clones (such as of
Arc/Rc), particularly cloning them into closures or async blocks, while
still keeping such cloning visible and explicit.

A very common source of friction in asynchronous or multithreaded Rust
programming is having to clone various Arc<T> reference-counted objects into
an async block or task. This is particularly common when spawning a closure as
a thread, or spawning an async block as a task. Common patterns for doing so
include:

// Use new names throughout the block
let new_x = x.clone();
let new_y = y.clone();
spawn(async move {
    func1(new_x).await;
    func2(new_y).await;
});

// Introduce a scope to perform the clones in
{
    let x = x.clone();
    let y = y.clone();
    spawn(async move {
        func1(x).await;
        func2(y).await;
    });
}

// Introduce a scope to perform the clones in, inside the call
spawn({
    let x = x.clone();
    let y = y.clone();
    async move {
        func1(x).await;
        func2(y).await;
    }
});

All of these patterns introduce noise every time the program wants to spawn a
thread or task, or otherwise clone an object into a closure or async block.
Feedback on Rust regularly brings up this friction, seeking a simpler solution.

This RFC proposes solutions to minimize the syntactic weight of
lightweight-cloning objects, particularly cloning objects into a closure or
async block, while still keeping an indication of this operation.


This RFC is part of the "Ergonomic ref-counting" project goal, owned by
@jkelleyrtp. Thanks to @jkelleyrtp and @nikomatsakis for reviewing. Thanks to
@nikomatsakis for key insights in this RFC, including the idea to use use.

Rendered

@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the RFC. I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. labels Aug 20, 2024
text/0000-use.md Outdated Show resolved Hide resolved
@5225225
Copy link

5225225 commented Aug 20, 2024

Personally, I don't feel that the non-closure/block use cases of this are really strong enough to warrant adding this, and the closure/block use case can be fixed with clone blocks.

The example

let obj: Arc<LargeComplexObject> = new_large_complex_object();
some_function(obj.use); // Pass a separate use of the object to `some_function`
obj.method(); // The object is still owned afterwards

could just be written as some_function(obj.clone()) with the only downsides being "that will still compile even if obj is expensive to clone" (Which is likely more easily solvable through a lint rather than a language feature), and not being able to remove redundant clones.

Which can presumably be solved either by making LLVM smarter about atomics for the specific case of Arc, or having an attribute on a clone impl that gives it the semantics use is being given here (Which would benefit all code that uses that type, not just new code that has been written to use .use)

The ergonomics of needing to clone in a block are annoying though, I agree, but that's a smaller feature by being able to do:

spawn(async clone {
    func1(x).await;
    func2(y).await;
});

and similarly for closures.

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

the closure/block use case can be fixed with clone blocks

The problem with a clone block/closure is that it would perform both cheap and expensive clones. A use block/closure will only perform cheap clones (e.g. Arc::clone), never expensive ones (e.g. Vec::clone).

Even without the .use syntax, async use blocks and use || closures provide motivation for this.

or having an attribute on a clone impl that gives it the semantics use is being given here (Which would benefit all code that uses that type, not just new code that has been written to use .use)

I think there's potential value there (and I've captured this in the RFC); for instance, we could omit a clone of a String if the original is statically known to be dead. I'd be concerned about changing existing semantics, though, particularly if we don't add a way for users to bypass that elision (which would add more complexity).

@Diggsey
Copy link
Contributor

Diggsey commented Aug 20, 2024

I'm afraid I'm pretty negative about this RFC.

Use trait

I don't like the Use concept as applied here: I don't think it makes sense to tie the concept of a "lightweight clone" to the syntax sugar for cloning values into a closure. Why can't I clone heavy-weight objects into a closure? It seems like an arbitrary distinction imposed by the compiler, when the compiler cannot possibly know what the performance requirements of my code are.

I could imagine there might be scenarios where knowing if a clone is light-weight is useful, but I don't think this is one of them.

.use keyword

I think the suffix .use form is unnecessary when you can already chain .clone(), and it's confusing for new users that .clone() requires brackets, whilst .use does not. Consistency is important. .use does not do anything that couldn't be done via a method, so it should be method - in general the least powerful language construct should be chose, in the same way that you wouldn't use a macro where a function would suffice.

However, x.use does not always invoke Clone::clone(x); in some cases the compiler can optimize away a use.

I don't like the "can" in this statement. Optimizations should fall into one of two camps:

  1. Automatic. These optimizations are based on the "as if" principle - ie. the program executes just as if the optimization was not applied, it just runs faster.
  2. Opt-in. This covers things like guaranteed tail calls, where the programmer says "I want this to be a tail-call" and the compiler returns an error if it can't do it.

Giving the compiler implementation a choice which has program-visible side-effects, and then specifying a complicated set of rules for when it should apply the optimization is just asking for trouble (eg. see C++'s automatic copy elision...) and I don't want to work in a language where different compilers might make the exact same code execute in significantly different ways.

use closure

If any object referenced within the closure or block does not implement Use (including generic types whose bounds do not require Use), the closure or block will attempt to borrow that object instead

I think this fallback is dangerous, as it means that implementing Use for existing types can have far-reaching implications for downstream code, making it a backwards compatibility hazard.

Motivation

Getting back to the original motivation: making reference counting more seamless, I think simply adding a syntax or standard library macro for cloning values into a closure or async block would go a long way to solving the issue... Potentially even all the way.

If a more significant change is needed, then I think this should be a type of variable binding (eg. let auto mut x = ...) where such variables are automatically cloned as necessary, but I hope such a significant change is not needed.

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

I've added a new paragraph in the "Rationale and alternatives" section explaining why async clone/clone || would not suffice:

Rather than specifically supporting lightweight clones, we could add a syntax
for closures and async blocks to perform any clones (e.g. async clone /
clone ||). This would additionally allow expensive clones (such as
String/Vec). However, we've had many requests to distinguish between
expensive and lightweight clones, as well as ecosystem conventions attempting
to make such distinctions (e.g. past guidance to write Arc::clone/Rc::clone
explicitly). Having a syntax that only permits lightweight clones would allow
users to confidently use that syntax without worrying about an unexpectedly
expensive operation. We can then provide ways to perform the expensive clones
explicitly, such as the use(x = x.clone()) syntax suggested in
[future possibilities][future-possibilities].

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

@Diggsey wrote:

I don't think it makes sense to tie the concept of a "lightweight clone" to the syntax sugar for cloning values into a closure. Why can't I clone heavy-weight objects into a closure?

You can; I'm not suggesting that we couldn't provide a syntax for that, too. However, people have asked for the ability to distinguish between expensive and lightweight clones. And a lightweight clone is less of a big deal, making it safer to have a lighter-weight syntax and let users mostly not worry about it. We could additionally provide syntax for performing expensive clones; I've mentioned one such syntax in the future work section, but we could consider others as well if that's a common use case.

I think the suffix .use form is unnecessary when you can already chain .clone()

That assumes that users want to call .clone(), rather than calling something that is always lightweight. If we separate out that consideration, then the question of whether this should be .use or a separate (new) trait method is covered in the alternatives section. I think it'd be more unusual to have the elision semantics and attach them to what otherwise looks like an ordinary trait method, but we could do that.

.use does not do anything that couldn't be done via a method, so it should be method

This is only true if we omitted the proposed elision behavior, or if we decide that it's acceptable for methods to have elision semantics attached to them. I agree that in either of those cases there's no particular reason to use a special syntax rather than a method.

I don't like the "can" in this statement. [...] Giving the compiler implementation a choice which has program-visible side-effects, and then specifying a complicated set of rules for when it should apply the optimization is just asking for trouble

This is a reasonable point. I personally don't think this would cause problems, but at a minimum I'll capture this in the alternatives section, and we could consider changing the elision behavior to make it required. The annoying thing about making it required is that we then have to implement it before shipping the feature and we can never make it better after shipping the feature. I don't think that's a good tradeoff.

Ultimately, though, I think the elisions aren't the most important part of this feature, and this feature is well worth shipping without the elisions, so if the elisions fail to reach consensus we can potentially ship the feature without the elisions. (Omitting the elisions entirely is already called out as an alternative.)

Getting back to the original motivation: making reference counting more seamless, I think simply adding a syntax or standard library macro for cloning values into a closure or async block would go a long way to solving the issue... Potentially even all the way.

See the previous points about people wanting to distinguish lightweight clones specifically. This is a load-bearing point: I can absolutely understand that if you disagree with the motivation of distinguishing lightweight clones, the remainder of the RFC then does not follow. The RFC is based on the premise that people do in fact want to distinguish lightweight clones specifically.

If a more significant change is needed, then I think this should be a type of variable binding (eg. let auto mut x = ...) where such variables are automatically cloned as necessary

I've added this as an alternative, but I don't think that would be nearly as usable.

@joshtriplett
Copy link
Member Author

joshtriplett commented Aug 20, 2024

@Diggsey wrote:

use closure

If any object referenced within the closure or block does not implement Use (including generic types whose bounds do not require Use), the closure or block will attempt to borrow that object instead

I think this fallback is dangerous, as it means that implementing Use for existing types can have far-reaching implications for downstream code, making it a backwards compatibility hazard.

While I don't think this is dangerous, I do think it's not the ideal solution, and I'd love to find a better way to specify this. The goal is to use the things that need to be used, and borrow the things for which a borrow suffices. For the moment, I've removed this fallback, and added an unresolved question.

@davidhewitt
Copy link
Contributor

Thank you for working on this RFC! PyO3 necessarily makes heavy use of Python reference counting so users working on Rust + Python projects may benefit significantly from making this more ergonomic. The possibility to elide operations where unnecessary is also very interesting; while it's a new idea to me, performance optimizations are always great!

I have some questions:

  • The name Use for the trait was quite surprising to me. Reading the general description of the trait and the comments in this thread, it seems like "lightweight cloning" or "shallow cloning" is generally the property we're aiming for. Why not call the trait LightweightClone or ShallowClone? (Maybe can note this in rejected alternatives?)

  • The RFC text doesn't make it clear to me why use & move on blocks / closures need to be mutually exclusive. In particular what if I want to use an Arc<T> and move a Vec<Arc<T>> at the same time; if I'm not allowed the move keyword then I guess I have to fall back to something like let arc2 = arc.use; and then moving both values? Seems like this is potentially confusing / adds complexity.

  • I would like to see further justification why the rejection of having Use provide automatic cloning for these types. I could only find one short justification in the text: "Rust has long attempted to keep user-provided code visible, such as by not providing copy constructors."

    • We already have user-provided code running in Deref operations for most (all?) of the types for which Use would be beneficial. Is it really so bad to make these types a bit more special, if it's extremely ergonomic and makes room for optimizations of eliding .clone() where the compiler can see it?
    • Further, I think Clone is already special: for types which implement Copy, a subtrait of Clone, we already ascribe special semantics. Why could we not just add ShallowClone as another subtrait of Clone which allows similar language semantics (but doesn't go as far as just a bit copy, which would be incorrect for these types)?

@kennytm
Copy link
Member

kennytm commented Aug 21, 2024

Since "Precise capturing" #3617 also abuses the use keyword this may be confusing to teach about the 3 or 4 unrelated usage of the keyword (use item; / impl Trait + use<'captured> / use || closure & async use { block } / rc.use).

@burdges
Copy link

burdges commented Aug 21, 2024

We should really not overload the usage of the keyword use so much, but ignoring the keyword..

Isn't it easier to understand if we've some macro for the multiple clones that run before the code that consumes them, but still inside some distinguished scope?

{
    same_clones!(x,y,z);
    spawn(async move { ... });
}

In this, the same_clones! macro expands to

let x = x.clone();
let y = y.clone();
let z = z.clone();

We use this multi-clones pattern outside async code too, so this non-async specific approach benefits everyone.

@alexheretic
Copy link
Member

I agree with the motivation but can't shake that "use" is a poor name and this mechanism seems a bit arcane.

Following on from #3680 (comment) comment proposing #[autoclone(...)] syntax, I like that this provides a solution for those wanting either explicit or implicit ref counting / cheap cloning. I still think there is value in a marker trait for "this type is cheap to clone" and for handling of those to be more ergonomic by default.

Would it be enough to have:

  • A CheapClone marker trait similar to Copy. No new methods or keyword usage.
  • You can opt into Copy-like auto clone behaviour for CheapClone types #![auto_cheap_clone(true)].
  • In the next edition CheapClone auto cheap clones become the default behaviour (but ofc may be toggled off).

This seems fairly simple and ergonomic. The downside is only for those that don't want auto cheap clones having to remember to disable it, but this could be clearly documented with the edition.

@piegamesde
Copy link

I think this RFC tackles two different issues at once and intermingles them:

  1. "Cheap" or "shallow" cloning
  2. Cloning into closures

The RFC text currently focuses on the shallow cloning, while not going far enough onto the cloning into closures problem. Some thoughts I'd see addressed:

  • That this is not only relevant for async-heavy code. Other situations where one may need to clone a lot into closures:
    • Custom managed runtimes, like DSL interpreters and GCs
    • Bindings to languages with an external runtime
  • Especially, there exist macros like glib::clone! to help out with this, which should be mentioned as prior art. Any solution within the language should make these macros redundant.
  • Moreover, in many situations cloning into a closure does not need to be restricted to shallow cloning. Especially heavy closures which fork off long-lived threads are not performance-critical w.r.t. this.

@ssokolow
Copy link

ssokolow commented Sep 4, 2024

I vaguely remember reading some ideas about a postfix super syntax, which could maybe make this pattern more compact:

As someone who is not a novice, but struggles with sleep issues that can often help me relate to people who struggle more to grasp concepts, I have to say that .super looks bad.

Conceptually, I see .super having the same problems as proposals to introduce new bindings mid-expression which are then visible outside the expression. It just feels surprising and momentarily confusing to allow that kind inline suspension of normal scoping behaviour.

...especially when this is scoping in the context of ownership and borrowing, ownership and borrowing is already something that takes effort to learn, and, unless I missed a big announcement, NLL Problem Case #3 is still a work in progress, providing even more learning friction.


If every invocation of `.use` or `use ||` here resulted in a call to
`Clone::clone`, this program would call `Clone::clone` 10 times (and have 11
`Arc`s to drop); however, the compiler can elide all the uses in `main` and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f takes the Arc by-value... how can the compiler elide a use here?

Copy link
Contributor

@zachs18 zachs18 Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea is that since the body of f is known and would also "work" with &Arc, the compiler could "rewrite" it to instead take &Arc using the "If x is not statically known to be dead, but all of the following conditions are met, the compiler may elide an x.use and use &x instead: ..." optimization listed above. (Saying nothing of how feasible implementing such an optimization would be)

[unresolved-questions]: #unresolved-questions

Should we allow *any* `x.clone()` call to be elided if a type is `Use` (or `Use
+ Copy`)? If we did this, that would not give us a way to explicitly avoid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The + at the beginning of the line here is interpreted as a list by markdown

blocks). Effectively, these conditions mean that the user could theoretically
have refactored the code to use `&x` rather than `x.use`.

An example of these elisions:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to also add an example of x.use -> &x

as `async use { ... }`, analogous to the use of `move`. (Note that `use` and
`move` are mutually exclusive.)

For any object referenced within the closure or block that a `move`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this a little bit confusing. You can probably write this better than me but I'd be more explicit about that the closure you're referring to is the "use" closure and I also think you wanted to say async block?. So maybe something like ...

Suggested change
For any object referenced within the closure or block that a `move`
For any object referenced within the use closure or use async block that a `move`

Still my phrasing reads weird but I guess you'd get what I meant to say :).

@marziply
Copy link

marziply commented Sep 17, 2024

In order to combat the ergonomic pain of cloning clones of Arcs and other "cheap" copy types with threads and channels, I've often implemented in some form the code snippet below on projects I have worked on in the past. While others have shared their distaste for .use, I personally find it would be much more useful (and readable in my opinion) to have that specific syntax to avoid all of the boilerplate .clone() calls.

/// Clone the target twice and return both as a tuple. This is a convenience
/// method for operations where two clones are needed, saving a let binding.
/// A common example is cloning an [`std::sync::Arc`] and sharing a cloned
/// reference between threads.
///
/// ```rust
/// use std::sync::Arc;
/// use std::thread::spawn;
///
/// let foo: Arc<i32> = Arc::default();
/// let (clone1, clone2) = foo.duplicate();
///
/// spawn(move || {
///   println!("clone1: {clone1}");
/// });
///
/// println!("clone2: {clone2}");
/// ```
pub trait Duplicate<T> {
  fn duplicate(self) -> (T, T);

  fn duplicate_from<V>(value: V) -> (Self, Self)
  where
    Self: From<V> + Clone,
  {
    let item = Self::from(value);

    (item.clone(), item.clone())
  }

  fn duplicate_from_default() -> (Self, Self)
  where
    Self: Default + Clone,
  {
    Self::duplicate_from(Self::default())
  }
}

impl<T> Duplicate<T> for T
where
  T: Clone,
{
  fn duplicate(self) -> (T, T) {
    (self.clone(), self.clone())
  }
}

@kennytm
Copy link
Member

kennytm commented Sep 18, 2024

@marziply the point of this RFC is removing all let binding boilerplates for a closure to capture-by-(shallow)-clone, I'm not sure how .duplicate() is going to simplify anything as the let is still right there:

    let (clone1, clone2) = foo.duplicate();
//  ^^^

BTW for generating multiple copies of an Arc at once, the (unstable) function array::repeat is enough:

#![feature(array_repeat)]
use std::{array, sync::Arc};
fn main() {
    let foo = Arc::new(1_u32);
    let [a, b, c, d, e] = array::repeat(foo);  // <--
    assert_eq!(Arc::strong_count(&a), 5);
}

In stable Rust we could use array::from_fn but this will generate one extra clone + drop.

    let [a, b, c, d, e] = array::from_fn(move |_| foo.clone());

@withoutboats
Copy link
Contributor

(NOT A CONTRIBUTION)

Unlike most commenters on this thread, I do think giving "normal type semantics" (meaning they can be used any number of times) to some types that are fairly cheap to copy but not by memcpy is a very good idea. However, like most other commenters, I think this RFC is not the solution to that problem. My impression of it is that it is an attempt to split the baby: on the one hand, there are people (like me) who think implicitly incrementing refcounts is fine and want to eliminate this clutter; on the other hand, there are people (like most people who engage on RFC threads) who disagree for whatever reason. This RFC satisfies neither group.

Fundamentally, I think the concept of this RFC is misguided: the design dispute is not at all about the syntax for "using a type without moving it." I think everyone is perfectly happy with how you use a value without moving it for types that support using without moving them: you just use them, like literally every other language, no special operator needed. The design dispute is about which types can be used without moving them: one party wants it to be strictly aligned with types that can be used by executing memcpy, another party wants some types which requires some other code execution. This RFC is a solution to a problem Rust doesn't have.

Accepting this RFC, in addition to making neither party happy, would introduce a whole additional load of complexity on users who now need to understand the difference between using a Copy value, moving a non-Copy value, .useing a Use value, and cloning a Clone value. This is already an area which is stretching Rust's complexity budget for new users; adding more features will not make it any easier to learn.

@marziply
Copy link

@marziply the point of this RFC is removing all let binding boilerplates for a closure to capture-by-(shallow)-clone, I'm not sure how .duplicate() is going to simplify anything as the let is still right there:

    let (clone1, clone2) = foo.duplicate();
//  ^^^

BTW for generating multiple copies of an Arc at once, the (unstable) function array::repeat is enough:

#![feature(array_repeat)]
use std::{array, sync::Arc};
fn main() {
    let foo = Arc::new(1_u32);
    let [a, b, c, d, e] = array::repeat(foo);  // <--
    assert_eq!(Arc::strong_count(&a), 5);
}

In stable Rust we could use array::from_fn but this will generate one extra clone + drop.

    let [a, b, c, d, e] = array::from_fn(move |_| foo.clone());

My comment was a means to justify this feature - I'm not fond of having to clone in this way, the trait in the example is a means to an end so if it were possible to clone syntactically rather than semantically then that would be my preference. The example was an illustration of how common clones are with channels and threads, not necessarily as a way to subvert current syntax.

Your alternatives to my implementation are probably better than mine, no doubt, but my point was that in my opinion, this should be a problem solved with syntax, not semantics. That's why I'm particularly fond of .use or any other form of syntax which makes my implementation completely redundant.

@kennytm
Copy link
Member

kennytm commented Sep 23, 2024

@marziply Sorry. I thought "that specific syntax" meant ".duplicate()" 😅

In any case, as other mentioned this RFC is conflating two features (capturing by clone i.e. use || x + special syntax for shallow clone i.e. x.use) into a single one. To solve your problem we need the capture-by-clone part, not the .use part.

Using #3680 (comment) syntax,

let foo: Arc<i32> = Arc::default();

spawn(use(clone foo) || {
    println!("clone1: {foo}"); // *not* foo.use
});

println!("clone2: {foo}");

@marziply
Copy link

@marziply Sorry. I thought "that specific syntax" meant ".duplicate()" 😅

In any case, as other mentioned this RFC is conflating two features (capturing by clone i.e. use || x + special syntax for shallow clone i.e. x.use) into a single one. To solve your problem we need the capture-by-clone part, not the .use part.

Using #3680 (comment) syntax,

let foo: Arc<i32> = Arc::default();

spawn(use(clone foo) || {
    println!("clone1: {foo}"); // *not* foo.use
});

println!("clone2: {foo}");

I do agree that this RFC represents more than one problem as others have mentioned. I just wanted to chime into the usefulness I personally would find with a feature like this!

@dev-ardi
Copy link

dev-ardi commented Sep 23, 2024

I really do like the approach of #![autoclone], however I think that in the way that it was preseted it would greatly increase the scope of the feature.

I'd be in favour of having the Use (or Claim) trait enabled for the library's ref counted types, and have its behaviour opt-out by using the #![no_autoclone(std::sync::Arc)] inner/outer attribute.

All types implementing Use that haven't been marked as no_autoclone act the same as Copy, you can pass them around, move them into closures, assign them to another variable...

This Use trait would then have a use fn in order to distinguish it from clone as Niko suggested. The issue with having to import that trait I don't think is an issue because the use for wanting to have explicit calls to use is rare, and it can be added to the prelude in the next edition.

This snippet should work, the compiler should insert all of the calls to Use::use as it needs to.

let x = Arc::new(3);
somethig_async(async move { x; }).await;
let y = x;
consume_arc(x);

I believe that this solution would satisfy the two groups of users: The ones like me that want implicit cloning of refcounted types and the ones that consider cloning Arcs expensive, since there is still an opt-out.

I'm not in favour of the #![autoclone(...)] attribute because it has many issues with exported types, you would just #[derive(Clone, Use)] for any of your types.

@Dirbaio
Copy link

Dirbaio commented Sep 23, 2024

I'm not in favour of the #![autoclone(...)] attribute because it has many issues with exported types, you would just #[derive(Clone, Use)] for any of your types.

could you expand on this? the way I was imagining it #[autoclone] would make Clone types behave like Copy types (by automatically inserting .clone() as you mention), but it wouldn't force all your types to implement Clone automatically. You still choose which impl Clone and which don't.

So if you use #[autoclone] in your lib, your exported types are unaffected. You decide which types are Clone, you decide whether you use autoclone in your lib, the users decide whether to use autoclone in their crates independently of that.

@dev-ardi
Copy link

This is off topic for this discussion (ergonomic ref counting) as that is a much different feature with different implications and additional design:

fn f() {
    mod m {
        #![autoclone(Foo)]
        pub struct Foo(Vec<u8>);
    }
    let x: m::Foo;
}
  • Is x autoclone? (It really shouldn't)
  • How is this documented in rustdoc?
#![autoclone(Rc<_>)]
struct Foo(Rc<()>);
struct Bar((Rc<()>, u8));
  • Is Foo autoclone?
  • Is Bar autoclone?
  • If they aren't, do you need to add ALL of the types you want to be autoclone to the autoclone attr?
  • Is this better in any way to having a Use marker trait that you can opt out of?

Even if that was a good solution, I think that it is a very significant feature creep.

@Dirbaio
Copy link

Dirbaio commented Sep 24, 2024

With my proposal there's no such thing as "a type being autoclone". Autoclone is NOT a property of types, it's a property of code, ie functions. You're telling the compiler "within this piece of code, instead of raising use of moved value errors, just implicitly clone."

// No autoclone enabled for the `blah` function.
fn blah() {
    let x = Arc::new(());
    let y = x;
    drop(x);  // error: use of moved value: `x`
    drop(y);
}

#[autoclone(*)] // enable autoclone for the `meh` function
fn meh() {
    let x = Arc::new(());
    let y = x; // implicitly clones here.
    drop(x);  // no error!
    drop(y);
}

#[autoclone(*)] // enable autoclone for all functions within this module
mod foo { ... }

Similarly, within lib.rs you can use the inner attribute syntax #![autoclone{*)] to apply the attribute to the crate itself, which means it applies to all functions within the crate.

If you do #![autoclone{Foo)] you're just changing how your crate's code is compiled, you're not changing any property of the type Foo. If Foo is exported, other crates can observe whether Foo is Clone or not, and nothing else.

So there's no need to expose autoclone in rustdoc at all. Autoclone is not a property of types, it's a setting that changes how code is compiled. if I enable autoclone in a lib crate, it has no effect in other crates. It's not public API.

@ssokolow
Copy link

So there's no need to expose autoclone in rustdoc at all. Autoclone is not a property of types, it's a setting that changes how code is compiled. if I enable autoclone in a lib crate, it has no effect in other crates. It's not public API.

Honestly, that still feels like it poses a risk of creeping toward "I don't want to contribute to projects written in this dialect of C++" but for Rust.

(Yes, rust-analyzer can claw back some ease of understanding of what is cloning when, but Rust has historically been designed on the principle that an IDE or similar should not be required to paper over weaknesses in the language's design.)

In short, it feels like it's breaking from the "code is read much more often than it's written" facet of Rust's design philosophy.

@Dirbaio
Copy link

Dirbaio commented Sep 24, 2024

Honestly, that still feels like it poses a risk of creeping toward "I don't want to contribute to projects written in this dialect of C++" but for Rust.

Yes, that's the tradeoff. I'm not 100% convinced we should do autoclone or some other form of implicit cloning. The upside is much better ergonomics, the downside is now you have two Rust dialects.

The main point I wanted to make in this thread is that IMO any solution that uses a trait CheapClone/Claim/Use to denote "this is cheap to clone" is fundamentally misguided, since "cheap to clone" is subjective as I said in my first comment. This applies both for "lighter syntax" (like the use being proposed here) and "no syntax" (fully implicit cloning).

So if we do some form of light/implicit syntax for cloning, it should be controlled by an attribute (puts the choice in the crate writing the code), not a trait (puts the choice in the crate defining the type).

@ssokolow
Copy link

ssokolow commented Sep 24, 2024

So if we do some form of light/implicit syntax for cloning, it should be controlled by an attribute (puts the choice in the crate writing the code), not a trait (puts the choice in the crate defining the type).

I can certainly agree with that. If it's trait-based, as a downstream consumer, I want something I can #![forbid(...)] in my crates to turn it off.

NOTE: I say this as an application developer, not a low-level developer. I came to Rust for the compile-time correctness... but I really do like being able to stave off the creeping resource bloat that comes from so much on my desktop migrating to garbage-collected languages and webtech frameworks, and part of that is remaining aware of the dynamics of my code... such as when reference counts are getting incremented.

@workingjubilee
Copy link
Member

workingjubilee commented Sep 24, 2024

based on the numbers people are pulling out for Arc<str> vs. String, it seems very much that making the call based on "but this constructor is defined as cheap to clone" is not correct. that seems purely circumstantial. the reality is just that sometimes we want it to be less overbearing to write or not, and sometimes it's not gonna be that cheap. the same issue plagues Copy. it's not really cheap to copy a several-page struct of bytes, but you can sure slap Copy on it, because a single u8 is cheap to copy, and [u8; 64] is also pretty cheap to copy (esp. if you align it!) but uhhh that stops being true pretty fast as the N increases further on [u8; N].

@ssokolow
Copy link

ssokolow commented Sep 24, 2024

it's not really cheap to copy a several-page struct of bytes [...]

Personally, I'd be very much in favour of, at minimum, a warn-by-default lint in rustc (not clippy) that triggers on Copy data types above a certain size and some kind of means of !Copy-ing specific array instances.

To me, this whole Use-as-implicitly-copyable-non-memcpy idea lands as "This tiny bit of the existing language design is broken. Therefore, we can use it to justify breaking more things." (There's that fundamental disconnect that withoutboats mentioned.)

EDIT: To clarify, I've been on the "Programmers just focus on their goals and I pay the 'de facto memory leak' price." side of things like garbage collection and automatic reference counting far too often. I like that Rust puts road bumps in the way of things that make it easy to fall off the fast/efficient path. It helps me to not burn out when I'm refusing to be a hypocrite on my own projects.

@lolbinarycat
Copy link
Contributor

aside from anything else: too much keyword overloading!!

we already added the + use<'a> syntax, and that's questionable enough. that, at least, is in a weird already confusing edge case unlikely to be encountered by new programmers.

refcounting is frequently used by beginners that either can't figure out the borrow checker, or can't figure out how to restructure their code into something the borrow checker will understand.

adding a third meaning of use, expecially in part of the language so frequently used by newcomers, is something i am opposed to.

@bitwalker
Copy link

One thought I've had reading through the proposal and thread, is that perhaps part of the difficulty here is that an attempt is being made to generalize the rather specific problem of the ergonomics of Clone-ing into closures (which isn't even universally agreed upon in discussions I've seen on the topic), to all code that contains calls to clone. I think it is debatable whether, even if everyone agrees about the specific problem with closures, that there is a meaningful generalization from that.

I think an effort to try and discover a precise notion of a shallow/lightweight Clone trait is useful, but it is a very broad solution, to a seemingly narrow problem, and maybe puts the cart before the horse a bit. To me, none of the proposed solutions here "feel right", which isn't exactly quantifiable, but seems like an indication that the idea that there is something principled between Copy and Clone, is at best, elusive. My impression is that a more targeted ergonomic improvement focusing specifically on smart pointer types, would be a better solution space to explore first - given the original problem description - and perhaps from that something more general will fall out.

For instance, there are various unstable features which go a long way towards making custom smart pointer types easier to write, more ergonomic, and able to be used in all the ways that the standard library types are used. This feels like it could fit in that space relatively easily. A solution that allows smart pointer types to be automatically cloned improves the ergonomics both for user-defined types and those provided by the standard library. That feels like a much safer space in which to enable "auto-Clone", as opposed to enabling that kind of magic for any type in general (assuming implementation of some kind of marker trait). I think an argument could be made that pointers are pretty unambiguously "cheap" to clone, although I would normally consider myself in the camp that finds auto-cloning Arc questionable at best.

As stated above, while it doesn't solve for those that would argue that Arc isn't "cheap" (which could be debated ad infinitum), I do think it is reasonable to draw a line at "we consider auto-Clone of smart pointer types to be cheap". It's narrow enough in scope that it can't really be meaningfully abused, and might provide a way to feel out a more generalized solution without it being all-or-nothing. Allowing arbitrary types to be auto-cloned feels like a much riskier change IMO.

Anyway, I don't know if this is helpful commentary or not, but figured I would try and suggest an alternative approach that might be more palatable to everyone, even if I'm not proposing something concrete myself. Personally, I imagine something like a marker(?) trait specifically for smart pointer types, that is used not only for the topic of this discussion, but for things like determining whether or not something can be used as a method receiver, etc., but I have no idea if that is the right way to approach it or not, but it does feel sensible at least.


As an aside, a more general solution to how a given capture is, well, captured by a closure, would be nice. I'd put myself in the camp with those that would like more explicit capture syntax e.g. move(foo) clone(bar) || { .. }, since that feels like a strict improvement over what we have today, both in expressive power and clarity. That said, that feels like a completely separate discussion IMO.

@ssokolow
Copy link

ssokolow commented Nov 11, 2024

A solution that allows smart pointer types to be automatically cloned improves the ergonomics both for user-defined types and those provided by the standard library. That feels like a much safer space in which to enable "auto-Clone", as opposed to enabling that kind of magic for any type in general (assuming implementation of some kind of marker trait)

I remember when Deref's documentation was phrased more in a "This is only intended for making smart pointers" way and that didn't stop it from being necessary to repeatedly tell people "No, Deref isn't to be used to 'simplify' the creation of a half-baked inheritance mechanism".

It's not useful to propose "Just for smart pointers?" as the question to address when there's no enforcement mechanism (aside from going back to the bad old days of what happened to actix-web, if you call that an "enforcement mechanism") and, even if there were, Rust's famously low memory footprint is a holistic thing. The kind of thing where you can't really put your finger on a defined place to be fixed if you stare at the other languages in a memory profiler.

There's no way to slap a #![forbid(auto_clone_in_my_transitive_dependencies)] on a codebase and I don't want to see a creeping tide of "costs not being made explicit" and the resultant "Memory bloat or NIH all the things" dilemma that could easily emerge one auto-clone at a time. It reminds me of how D's optional garbage collector became de facto mandatory because too much of the ecosystem depended on it. (Or, for that matter, how GCed languages are famous for trading a lot of memory leaks for a smaller number of "permanent caches" because there's no magic replacement for "Hey, programmer, are you sure you meant to do that?")

Humans are lazy as a rule, and I think, on this point, Rust's existing design strikes a very good "as simple as possible but no simpler" balance for making people "take their medicine" without transitioning into a degree of onerousness that's unreasonable.

Rust's explicitness is part of its niche and, to me at least, this feels like one of those "Can I just have this one feature I want, please?" RFCs that leads toward death by a thousand cuts. (The same category I'd put fehler-style Ok()-wrapping syntactic sugar into as a "Can I please just have exception throwing-style syntax?".)

As someone who came from Python, I have to remind people that Rust isn't a scripting language. Languages like Swift already exist to occupy other points on the performance-vs-convenience continuum and I don't want Rust to fall off the "just enough impurity of design vision to achieve mainstream practicality" balance point it's occupied thus far, which has made it so special. Some people want inheritance, but that would draw Rust away from the niche it's found. Some people want exceptions for various reasons, but having that built into the language would draw Rust away from the niche it's found. etc. etc. etc.

This is another "but that would draw Rust away from the niche it's found" and I see it as being exactly the same kind of "I want my one feature" as fehler without the degree of benefit that a justifiable example like "async puts a hidden Future in the return type" brings. It's just "I don't like seeing/typing a few extra Arc::clones and I trust that everyone else cares about efficiency enough that this new power (and drop in the complexity budget bucket) won't get abused". In the vast majority of situations,

Arc::clone having some innate friction is a feature in the same way that the borrow-checker's innate friction is a feature, and requiring at least an .into() or as to do type conversion is a feature. It's not to Rust's benefit to reduce that friction any more than replicating C's integer promotion hierarchy would be.

If anything, I think this should receive the same "Make it a macro and we'll see if there's enough demand to justify it" treatment that fehler, delegate, and lazy_static/once_cell got. That seems to have a proven track record, even if I'll admit that I'm suggesting it because I firmly believe the general community is on my side and a claim crate will see uptake more in the vein of fehler (or, at most, delegate) than once_cell. (Without that pressure relief valve, I'd be arguing vehemently to reject it outright instead of just arguing vehemently to not deny the ecosystem the chance to "vote with their wallets".)

EDIT: ...I also think that making it a third-party macro crate would help to "push the externalities back into being a line item on the balance sheet", similar to what regulations are supposed to do for businesses when they're working properly. "You think it's cheaper to throw your waste into shared resources and/or poor peoples' neighbourhoods than to dispose of your waste properly? It looks like regulations are needed to increase the cost of choosing that option until it's cheaper to clean up your messes." (But then I am one of those "this shouldn't need client-side JavaScript" people who has fantasized about climate change initiatives taxing corporate revenue to discourage designs where moving costs to the customers has a disproportionate amplifying effect on watts spent and PCs and phones prematurely obsolesced when externalities are taken into account.)

@ssokolow
Copy link

...OK, I'm done. Consider this message a "commit-squashed" e-mail notification for all the stuff I added to my comment.

@bitwalker
Copy link

bitwalker commented Nov 11, 2024

@ssokolow Like I mentioned, I absolutely share your opinion that the explicitness of Rust is one of its strengths, and therefore making things less explicit is undercutting something core to the language. I'd be perfectly content if auto-cloning never materialized. That said, value judgements (even if they resonate with me) are fundamentally not going to win the debate, because if everyone shared the same perspective on what Rust's strengths are, and how to preserve them/expand on them, there would be no debate. I'd rather at least try and figure out whether a more targeted solution is (or isn't) a preferable alternative.

On that note, the Deref example - and it has been brought up before on this topic elsewhere - falls flat IMO. You can probably write a Deref impl that does something insane, but I don't think I've ever seen one that does anything other than basically what you'd expect. If anything, Deref strikes me as an example of why a solution targeting smart pointers may well be a better approach. If Deref was designed solely for smart pointers, then it was wildly successful at being useful beyond its intended use case, while managing to avoid going off the rails. There are no compiler-enforced rules around what can be in a Deref impl (that I know of anyway), and it does suffer from being magical, but I think it is an example of where a little magic, applied in a targeted fashion, provides significant ergonomic benefits without giving up too much in exchange. I think there are sufficient good reasons not to implement this proposal, but if anything, Deref is a counter-example to the idea that the concept in general is a slippery slope.

I do agree though, that if all one has to do to avoid typing .clone in their code, is add a marker trait impl to their type, a non-trivial number of people will do that out of some misguided pursuit of aesthetics, particularly library authors who want to eke out as many ergonomic wins as possible. One of the core flaws with the idea of auto-cloning arbitrary types IMO, aside from the value judgment stuff, is that what constitutes "cheaply cloneable" is not really specifiable. At least with smart pointers, defining "cheap" becomes tractable, albeit not completely free of debate.

Anyway, I don't want to derail the conversation about this specific proposal any more than I already have, but figured it might be worth elaborating a bit on those points. Feel free to delete/hide this comment if it is unhelpful!

@ssokolow
Copy link

ssokolow commented Nov 11, 2024

Fair point on Deref. My perspective on it may have been skewed by how much stuff on it I saw pass through the places I hung out back in the early days and, though I remain unconvinced that narrowing the proposal will meaningfully change the value calculus, I agree with the rest of what you're saying.

(I think making Rc/Arc less off-putting is, in itself, an anti-feature because Rc/Arc being ugly is one of the design decisions that form the "pit of success" for writing memory-efficient code. If it were more pleasant to use, people would be more likely to use it to "just make things compile" and I firmly believe that wanting cheap clones is a "want to have your cake and eat it too" situation, even in the most conservative hypothetical where they were implemeted via a perma-unstable trait with standard-library implementations on Rc and Arc.)

@ssokolow
Copy link

ssokolow commented Nov 15, 2024

Having just run across I am a SQL Injection Attack, I realized that, in a sense, the ugliness of Rc/Arc is a feature, not a bug, because it functions as a metaphor, bridging the "It doesn't cease to be ugly, even when it's the best solution"-ness of shared ownership into the user's intuition.

From that perspective, the "claim" concept is akin to drinking coffee instead of improving your sleep habits. Praying for people and believing you're done instead of actually helping, or playing euphemism musical chairs with uncomfortable words instead of going out and campaigning against the injustice they evoke. It's the invention of artificially fatty/salty/sugary foods or pornography or other inventions which please the metrics our instincts use without achieving the aims they were evolved for.

Shared ownership is inherently a code smell and nothing will change that... we just have to recognize that, sometimes, the least bad solution to a problem is still a smelly one, rather than trying to abolish the concept of a sense of smell.

I'm reminded of something Maurice Krafft said in an interview in a National Geographic video I watched as a kid. (I think it was just called "Volcano!") Hazy memory aside...

The noise on an explosive volcano is generally terrible. It's so strong, that sometimes you cannot talk to each other. [...] Sometimes people ask why don't you put earplugs... Never. It's a good way to be killed. Because sometimes, a very small noise... whistle and so on, it means a lot. It means it's time to go.

While I would never say it's that disastrous, I think "using bog-standard passive hearing protection in an industrial environment where you need to retain your situational awareness" is an excellent metaphor for claim. (For those unaware, there are special hearing protection systems like these for situations like that.)

As with writing correct C or C++ code, there are certainly people who will be able to use claim responsibly... but the raison d'etre of Rust, it's purpose for existing, is to acknowledge what C and C++ learned the hard way... it doesn't matter how skilled you are if you have to work on a large codebase as part of a team or rely on dependencies written by someone else.

You don't get to sunset the lockout-tagout system because you are cautious enough that you personally don't believe you need it, no matter how inconvenient you find it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.