WUFFS

Posted Aug 16, 2024 13:07 UTC (Fri) by mb (subscriber, #50428)
In reply to: WUFFS by excors
Parent article: Standards for use of unsafe Rust in the kernel

>I think it has made a good choice.

Yeah, it's ok-ish most of the time.

Except for the case of move-capturing blocks. Passing an Arc clone to multiple async-move blocks is awkward.
It basically means cloning the Arc to a different name and then moving that name into the async block.
Which typically results in names such as foo2 or foo_cloned.
(Name shadowing is also possible in some cases.)

I think this is on the current list of things to be improved. But I'm not sure what an elegant solution would look like.

WUFFS

Posted Aug 16, 2024 19:12 UTC (Fri) by mathstuf (subscriber, #69389) [Link] (2 responses)

I think that Claim[1] looks ok. It makes a different cut to help understand "cheap to Clone" because `Rc::clone` is (almost certainly) cheaper than `[u8; 16536]` despite the latter being `Copy`.

[1] https://smallcultfollowing.com/babysteps/blog/2024/06/21/...

WUFFS

Posted Aug 16, 2024 20:49 UTC (Fri) by khim (subscriber, #9252) [Link] (1 responses)

I actually dislike that proposal. Because it's conceptually wrong. It asserts that the only difference between copy and clone is efficiency. But in reality difference is on another level: Copy types are just chunks of memory without internal structure and their copy is entirely independent from the original while clone means something else.

To me, on the congnitive level, clone of Rc<Vec<u32>> means that we want to split ownership of that [unmodifiable] vector… which maybe is kinda-sorta-Ok (because it's fixed and unmodifiable), but I would definitely not want to see Rc<RefCell<Vec<u32>>> automatically clones and, if understand correctly, this proposal would make it automatically clone'able, too.

The whole thing is solving entirely wrong problem: if the desire is to replace this:

tokio::spawn({
    let io = cx.io.clone():
    let disk = cx.disk.clone():
    let health_check = cx.health_check.clone():
    async move {
        do_something(io, disk, health_check)
    }
})

with this:

tokio::spawn(async move {
    do_something(cx.io, cx.disk, cx.health_check)
})

then it can be fixed with addition of just one keyword (it may even not be a keyword since it always comes after async):

tokio::spawn(async clone {
    do_something(cx.io, cx.disk, cx.health_check)
})

This would make it possible for a people like me to enable a clippy lint which rejects such code (I very much do want to see when I'm sharing ownership with some other task), although I can see why some people would want to add some magic to their programs.

Thankfully this proposal also comes with #![deny(automatic_claims)] which is good enough for me: while complexity of that whole madness is a bit larger than I would like with existence of #![deny(automatic_claims)] I don't care all that much about what other people are doing about Claim.

IOW: IMNSHO this proposal is a net negative, but very mild one, not enough to throw a temper tantrum if it would be accepted.

WUFFS

Posted Aug 16, 2024 21:28 UTC (Fri) by mathstuf (subscriber, #69389) [Link]

> then it can be fixed with addition of just one keyword (it may even not be a keyword since it always comes after async):
>
> tokio::spawn(async clone {
> do_something(cx.io, cx.disk, cx.health_check)
> })

Except I want it for plain closures too, not just `async` blocks.