Types implementing Deref{,Mut} shouldn't have any other methods or public fields

[sfackler]

For example, sync::RwLockWriteGuard implements Deref and DerefMut, implements the downgrade method, and has a public cond field. Since Rust doesn't have distinct . and -> operators, this makes for an API that can be confusing and ambiguous.

Adding an extra layer of indirection seems like a reasonable solution. It feels a bit strange to call methods of the wrapped type on an RAII lock guard anyways:

let locked_foo: RwLock<Foo> = ...;
let mut guard = locked_foo.write();
while !guard.get().is_foo() {
    guard.cond.wait();
}

[Kimundi]

This is not a problem isolated to Deref:

Because of the auto(de)ref behavior of dot, you can get in this situation in plenty different ways. Prominent example: Clone is implemented for &T to return &T, independent of any type that might implement Clone for T.

While implementors of Deref should definitely be careful to not make the API to confusing, restricting it to only types that don't have a public method/field APIs to restrictive.

Future example: With DST, Vec will implement Deref<[T]>, to make slicing it easy. This is definitely a case where the limitation spoken of here would not be useful.

[alexcrichton]

This is something that we're inevitably buying into with the "smart pointer" concept. If we go down this road, all of these need to be removed:

• {Arc, Rc}::clone
• {Arc, Rc}::downgrade
• MutexGuard.cond
• RWLockWriteGuard.cond
• RWLockWriteGuard::downgrade

This is what I think are just the start of the Deref implementations. I believe that many more will come. I feel that this is just a fact of smart pointers rather than a hazard that should be avoided. When designing a smart pointer API it should be kept in mind, but just as a mild concern.

[huonw]

If we go down this road, all of these need to be removed:

• {Arc, Rc}::clone
• {Arc, Rc}::downgrade

Surely removing those makes the types essentially useless?

[alexcrichton]

Surely removing those makes the types essentially useless?

That's what I was getting at, I don't think that this is an actionable issue.

[sfackler]

It's totally actionable, potentially with a slightly more verbose API:

impl<T> Rc<T> {
    pub fn downgrade(&mut self) { ... }
    pub fn get(&mut self) -> &'a T { ... }
}

Maybe get would return a smart pointer instead, I'm not sure what would work better.

[sfackler]

Alternatively we can keep the Deref impls and move the other stuff. For example:

let foo = Rc<Foo>;
foo.downgrade(); // calls Foo's downgrade
let weak = Rc::downgrade(foo); // the only way to call Rc's downgrade

[nikomatsakis]

I was assuming we would move these operations to functions, which seems like best practice. You could keep them---users can always explicitly dereference---but you couldn't add new such operations backwards compatibly.

[nikomatsakis]

I guess we could establish a convention that you write "smart pointer.methods().foo()" or something -- that is, there is one designated name (methods, in this example) that then lets you invoke methods on the smartpointer itself. To implement, methods would do something like:

struct RcMethods<'a, T> {
    rc: &'a mut Rc<T>
}

But it all seems awkward.

[aturon]

cc me

[bluss]

@alexcrichton is shifting the convention to static methods over free functions. I think it seems like a great idea.

Also, practice has been that some essential methods (comparison, clone etc) are shadowed by the smart pointer, while more specialized methods are not. (For example try_unwrap for Rc).

[huonw]

I think this is now as done (enough?) in the standard library, and is probably not really actionable anyway, due to stability. Closing.

(Don't miss rust-lang/rfcs#279 either.)