rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.9k stars 1.56k forks source link

Redesign `Arc` API #3601

Closed IoaNNUwU closed 6 months ago

IoaNNUwU commented 6 months ago

My suggestion is to redesign Arc, Rc and similar structures' API to be more obvious to the user. Let's look at Arc as an example.

Arc<T> stands for Atomically Reference Counted T.

Current implementation consists of 2 parts:

In current implementation Arc is reference-like structure and ArcInner type is hidden from the user. This is not ideal because it may be not obvious to the user what Arc actually is.

Coming from C or C++, I would assume Arc stands for Atomic Reference Counter, which is heap-allocated container, which wraps T and manages pointers to itself through special means, which is the same as ArcInner in Rust.

In C you would also usually use ArcRef structure, which is wrapper around a pointer to heap-allocated counter. So instead of free(arc_ref.ptr) you would call arc_decrement(&arc_ref).

I think this style of API, where Arc<T> is heap-allocated object which wraps T and atomic counter is more obvious. So we can have something like:

// you don't need to import ArcRef if you're not explicitly stating the type.
use std::sync::{Arc, ArcRef};
#[derive(Debug)]
struct User { id: u32 }

fn main() {
    let user: ArcRef<User> = Arc::new(User { id: 18 }); // Arc::new returns ArcRef instead of Arc.
    let handles = (0..10).map(|_| {
        let user = user.clone(); // it's clear we're copying the reference, because `user` is `ArcRef<User>`
        std::thread::spawn(move || {
            println!("{:?}", user); // ArcRef<T> implements Deref<Item = T>
        });
    }).collect::<Vec<_>>();

    for handle in handles {
        handle.join().unwrap();
    }
}

To make this possible we need:

Diggsey commented 6 months ago

In current implementation Arc is reference-like structure and ArcInner type is hidden from the user. This is not ideal because it may be not obvious to the user what Arc actually is.

Renaming Arc -> ArcRef is not going to make people suddenly understand what it does if they didn't before. Furthermore, Ref has specific meaning in Rust, so calling it a Ref would be more misleading.

Coming from C or C++, I would assume Arc stands for Atomic Reference Counter

Language decisions shouldn't be made based on what people coming from C/C++ might assume. (Speaking as someone who came to Rust from C/C++)

SOF3 commented 6 months ago

what is the point of exposing ArcInner? why would it increase any clarity to introduce a new type that nobody uses directly?

kennytm commented 6 months ago

Coming from C or C++, I would assume Arc stands for Atomic Reference Counter

The Rust doc clearly stated that Arc stands for "Atomically Reference Counted" (not Counter), an adjective not a noun describing the pointer structure, as "a thread-safe reference-counting pointer".

And I don't think there are any standard C or C++ structure using the name "RC" or "ARC" (the standard C++ reference-counting pointer is called std::shared_ptr<T>). So I don't know how "coming from C or C++" caused the whole misunderstanding.


Independent of the naming, I can't think of any reason that needs exposing ArcInner.

IoaNNUwU commented 6 months ago

Thanks for your opinions. I still think this new API would'be made a lot of sense, especially for the new users.

But as this proposal is getting no support and has obvious flaws, I think it should be closed.

Best thing to do now is to improve docs on such structures.

SOF3 commented 6 months ago

@kennytm just a NIT, stack variables are not necessarily thread-local (they can be leased out with scoped threads), and it might actually be possible (for whatever reason) to borrow an Option<ArcInner<T>> that calls option.take() when all references are lost.

But I have no idea how that would be useful at all, except in microprocessors where borrowing all the possible stack is desirable (but then that wouldn't be atomics anyway? and a dedicated stack-based allocator sounds applicable)