rust-lang/rfcs

Redesign `Arc` API

IoaNNUwU opened this issue · 5 comments

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:

  • heap-allocated ArcInner<T>, which holds T as well as 2 AtomicUsize which are used for strong- and weak-reference counting.
  • Arc<T> which wraps pointer to ArcInner<T>, allows shared access through this pointer, and is used for reference counting (Clone and Drop implementations).

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:

  • Rename Arc to ArcRef
  • Rename ArcInner to Arc and make it public.
  • Make Arc::new return ArcRef
  • Remove ArcRef::new in flavor of Arc::new

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

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

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.

  • If you don't like the default heap allocation, you can supply a custom allocator with Arc::new_in.
  • If you want to put the ArcInner on the stack (or actually RcInner since it must be thread-local)... why you need ref-counting in the first place rather than letting the destructor run at the end of its lifetime
  • If you want to make it a global variable... the ref-count can never reach zero anyway so why not just refer to the inner object directly.

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

@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)