Skip to content

autoderef: DerefMut doesn't work with Box #26205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
japaric opened this issue Jun 11, 2015 · 10 comments
Closed

autoderef: DerefMut doesn't work with Box #26205

japaric opened this issue Jun 11, 2015 · 10 comments
Labels
A-type-system Area: Type system

Comments

@japaric
Copy link
Member

japaric commented Jun 11, 2015

STR

use std::ops::{Deref, DerefMut};

struct Foo;

impl Foo {
    fn foo(&self) {}
    fn foo_mut(&mut self) {}
}

struct Bar(Foo);

impl Deref for Bar {
    type Target = Foo;

    fn deref(&self) -> &Foo {
        &self.0
    }
}

impl DerefMut for Bar {
    fn deref_mut(&mut self) -> &mut Foo {
        &mut self.0
    }
}

#[cfg(not(works))]
fn test(mut bar: Box<Bar>) {
    bar.foo();  // OK
    bar.foo_mut();
    //~^ error: cannot borrow immutable borrowed content as mutable (WRONG)
}

#[cfg(works)]
fn test(mut bar: Box<Bar>) {
    bar.deref_mut().foo_mut();
    //~^ but this actually works!
}

fn main() {}

Version

rustc 1.2.0-nightly (613e57b44 2015-06-01) (built 2015-06-02)

cc @nikomatsakis @eddyb

@steveklabnik steveklabnik added the A-type-system Area: Type system label Jun 15, 2015
@apasel422
Copy link
Contributor

I'd be interested in fixing this if someone can point me in the right direction.

@eddyb
Copy link
Member

eddyb commented Sep 1, 2015

There's some type-checking code that fixes up auto-deref calls on the receiver after the actual method being called is found.
Presumably it works with one level of overloaded DerefMut, but in this case, you have built-in Box<Bar> dereference followed by overloaded Bar dereference, and somehow it chokes on that.

@apasel422
Copy link
Contributor

src/librustc/middle/ty.rs:

// Returns the type and mutability of *ty.
//
// The parameter `explicit` indicates if this is an *explicit* dereference.
// Some types---notably unsafe ptrs---can only be dereferenced explicitly.
pub fn builtin_deref(&self, explicit: bool) -> Option<TypeAndMut<'tcx>> {
    match self.sty {
        TyBox(ty) => {
            Some(TypeAndMut {
                ty: ty,
                mutbl: ast::MutImmutable,
            })
        },
        TyRef(_, mt) => Some(mt),
        TyRawPtr(mt) if explicit => Some(mt),
        _ => None
    }
}

I assume that ast::MutImmutable is the cause of this issue.

@eddyb
Copy link
Member

eddyb commented Sep 2, 2015

@apasel422 ah, yes, indeed, that would make most code treat it as &T instead of being transitive.
This is a bit tricky, because it needs to be integrated with the typeck's LvaluePreference (or whatever it's called nowadays) that controls whether the context requires mutable access.
Maybe that enum could be moved to middle::ty - there was a PR where renaming it and its variants was suggested, but I forget where.

@eddyb
Copy link
Member

eddyb commented Sep 2, 2015

Ah, there it is, @Kimundi's fix for ref mut patterns: #26058.

@apasel422
Copy link
Contributor

@eddyb Thanks for the help, but if you have a better idea how to address this, go ahead. I'm not entirely sure how to approach this.

@apasel422
Copy link
Contributor

Selected log output from compiling the original snippet:

DEBUG:rustc_typeck::check: autoderef(base_ty=&Bar, opt_expr=Some(expr(37: self)), lvalue_pref=NoPreference)
DEBUG:rustc_typeck::check: autoderef(base_ty=&Foo, opt_expr=Some(expr(35: &self.0)), lvalue_pref=NoPreference)
DEBUG:rustc_typeck::check: autoderef(base_ty=&mut Bar, opt_expr=Some(expr(52: self)), lvalue_pref=PreferMutLvalue)
DEBUG:rustc_typeck::check: autoderef(base_ty=&mut Foo, opt_expr=Some(expr(50: &mut self.0)), lvalue_pref=PreferMutLvalue)
DEBUG:rustc_typeck::check: autoderef(base_ty=Box<Bar>, opt_expr=None, lvalue_pref=NoPreference)
DEBUG:rustc_typeck::check: autoderef(base_ty=Box<Bar>, opt_expr=Some(expr(65: bar)), lvalue_pref=NoPreference)
DEBUG:rustc_typeck::check: autoderef(base_ty=Box<Bar>, opt_expr=Some(expr(65: bar)), lvalue_pref=PreferMutLvalue)

@eddyb
Copy link
Member

eddyb commented Sep 3, 2015

@apasel422 What I'm saying is, more or less, to pass that lvalue_pref value to Ty::builtin_deref and pick the mutability of the TyBox case based on that, instead of always having it immutable.
You could try with just a bool and see if that changes anything.

@apasel422
Copy link
Contributor

@eddyb But based on the logs, it seems like it eventually does use PreferMutLvalue. It just doesn't actually succeed.

@apasel422
Copy link
Contributor

All right, I think I understand what needs to be done now. PR incoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system
Projects
None yet
Development

No branches or pull requests

4 participants