Rust: BorrowMut can cause confusion with Rc<RefCell<X>>

[mitsuhiko]

When BorrowMut is in scope and one has a rc: Rc<RefCell<X>> a call to rc.borrow_mut() does not deref into the inner RefCell but uses the generic call returning a &mut Rc<...>.

Something like this:

   = note: expected type `&mut std::rc::Rc<std::cell::RefCell<X>>`
   = note:    found type `std::option::Option<_>`

The compiler should ideally give feedback that the trait was used instead instead of the deref and how to fix this issue.

[shiMusa]

I just encountered the same issue.
Any ideas on giving feedback from the compiler?
I'm on 1.25.0-nightly (bd98fe0c0 2018-02-06)

[Mark-Simulacrum]

@estebank @GuillaumeGomez Any ideas on what we could do to improve the situation here? It feels somewhat hard to detect...

[GuillaumeGomez]

A bit yes...

[kristate]

Ran into this one.
rust version 1.26.0-nightly (259e4a678 2018-03-04)

[estebank]

I feel that we should just make it so that typeck resolves to the RefCell, but that would probably be a (slight) breaking change. cc @nikomatsakis what do you think about this?

The diagnostic should still be improved regardless. @mitsuhiko could we get a minimized repro case?

[mitsuhiko]

Minimized repro case:

Works:

use std::cell::RefCell;
use std::rc::Rc;

fn main() {
    let rc = Rc::new(RefCell::new(true));
    *rc.borrow_mut() = false;
}

Fails:

use std::borrow::BorrowMut;
use std::cell::RefCell;
use std::rc::Rc;

fn main() {
    let rc = Rc::new(RefCell::new(true));
    *rc.borrow_mut() = false;
}

Error:

  |
7 |     *rc.borrow_mut() = false;
  |                        ^^^^^ expected struct `std::rc::Rc`, found bool
  |
  = note: expected type `std::rc::Rc<std::cell::RefCell<bool>>`
             found type `bool`

[nikomatsakis]

I feel that we should just make it so that typeck resolves to the RefCell, but that would probably be a (slight) breaking change.

That'd be nice, but I fear that the impact might be larger than you are anticipating. I don't think that such a change is really on the table.

Possibly the BorrowMut trait should not have permitted . notation (i.e., by avoiding &mut self shorthand), but that's water under the bridge. (Though -- plausibly -- we could do something around the epoch here, where BorrowMut kind of "opts out" of being used with . notation in the newer epoch.)

I'm trying to decide how a suggestion here ought to work: there are definitely a few cases where method calls resolve to surprising places and the resulting errors can be confused. I wonder if we can find some way to capture those too.

For example, I am frequently hit by this problem, caused by a missing Clone:

fn foo<T>(x: &T) {
    let y = x.clone(); // results in `Clone` being invoked on the `&T`, rather than *failing*
}

In that case -- as here -- method resolution succeeds, but the error comes much later. It feels like we would need some sort of "tracer" that we can apply to the type so that we can see that it was derived via a method dispatch, and then trace back to see if that method dispatch may have had other possibilities.

I've been wanting to change how our type checker works -- it wouldn't necessarily have to be a massive change, basically just avoiding the refresh_type_vars_if_possible helper -- to enable this sort of tracking. Maybe a good project.

[shiMusa]

I'm not a pro on the interior workings of rust, but it seems to me to be an issue of function-overloading.

Why not use the as keyword to specify the origin of the function, e.g.
(x as Clone).clone(); or *(rc as RefCell).borrow_mut()?

PS: this would also help enormously with overloading in derived traits btw.

[nikomatsakis]

@shiMusa we have an existing notation for that, you can write (for example) Clone::clone(&x), but it's tedious.

[shiMusa]

Thanks @nikomatsakis , I forgot about that. Yeah, it's not something you'd easily think of...

[mitsuhiko]

I think the fact that a trait can shadow the methods of a type are particularly weird when working with trait objects. I came across this again in a different form when working with failure. https://github.com/rust-lang-nursery/failure/issues/211

failure implements a method on a trait with the same name as the default trait impl. I cannot find a way to disambiguate the call since I can't take the trait out of scope.

use std::fmt::Debug;

trait Foo: Debug + 'static {
    fn debug(&self) -> String where Self: Sized { format!("{:?}", self) }
}

impl Foo {
    fn debug(&self) -> String { format!("{:?}", self) }
}

trait AsFoo {
    fn as_foo(&self) -> &Foo;
}
impl<T: Bar> AsFoo for T {
    fn as_foo(&self) -> &Foo { self }
}

trait Bar: Foo + AsFoo {}
impl Bar {
    fn debug_via_bar(&self) { println!("debug: {}", self.as_foo().debug()) }
}

impl Foo for i32 {}
impl Bar for i32 {}

fn main() {
    let x: &Bar = &42i32;
    x.debug_via_bar();
}

[1414C]

Encountered the same issue as OP.
rust version stable-x86_64-apple-darwin unchanged - rustc 1.34.0 (91856ed52 2019-04-10)

works:

use std::collections::HashMap;
use std::cell::RefCell;
use std::rc::Rc;

let mut hm: HashMap<&str, i32> = HashMap::new();
hm.insert("myKey1", 100);
hm.insert("myKey2", 200);

let rc = Rc::new(RefCell::new(hm));
let v1 = rc.borrow_mut().get("myKey1").unwrap().clone();
println!("read {} into v1!", v1);

error:

use std::borrow::BorrowMut;
use std::collections::HashMap;
use std::cell::RefCell;
use std::rc::Rc;

let mut hm: HashMap<&str, i32> = HashMap::new();
hm.insert("myKey1", 100);
hm.insert("myKey2", 200);

let rc = Rc::new(RefCell::new(hm));
let v1 = rc.borrow_mut().get("myKey1").unwrap().clone();
println!("read {} into v1!", v1);

error[E0599]: no method named `get` found for type `&mut std::rc::Rc<std::cell::RefCell<std::collections::HashMap<&str, i32>>>` in the current scope
  --> src/main.rs:34:30
   |
34 |     let v1 = rc.borrow_mut().get("myKey1").unwrap().clone();
   |                              ^^^

As a newcomer to Rust working through the book, this error puzzled me for a few minutes. My efforts to code a Rcuse std::borrow_BorrowMut to the scope of the Rc