Skip to content
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

method call fails to upcast trait objects, resulting in overlong borrows #24950

Open
nikomatsakis opened this Issue Apr 29, 2015 · 8 comments

Comments

Projects
None yet
8 participants
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 29, 2015

@SimonSapin encountered an error in servo that I reduced to the following test case http://is.gd/w7HPbJ:

pub trait Flow {
    fn foo(&self);
}

pub trait LayoutDamageComputation {
    fn compute_layout_damage(self);
}

impl<'a> LayoutDamageComputation for &'a mut (Flow + 'a) {
    fn compute_layout_damage(self) {
        self.compute_layout_damage();
        self.compute_layout_damage();
    }
}

fn main() { }

The errors you get are:



<anon>:12:9: 12:13 error: cannot borrow `*self` as mutable more than once at a time
<anon>:12         self.compute_layout_damage();
                  ^~~~
<anon>:11:9: 11:13 note: previous borrow of `*self` occurs here; the mutable borrow prevents subsequent moves, borrows, or modification of `*self` until the borrow ends
<anon>:11         self.compute_layout_damage();
                  ^~~~
<anon>:13:6: 13:6 note: previous borrow ends here
<anon>:10     fn compute_layout_damage(self) {
<anon>:11         self.compute_layout_damage();
<anon>:12         self.compute_layout_damage();
<anon>:13     }
              ^
error: aborting due to previous error
playpen: application terminated with error code 101
Program ended.

There are various possible workarounds:

  1. http://is.gd/frmXsT
  2. http://is.gd/FgrEfZ

but the gist (no pun intended, I kill me) is to ensure a coercion from self to &mut Flow, which allows us to upcast the lifetime in the trait object. The easiest way to do that is to convert the recursive call self.compute_layout_damage() to UFCS form <&mut Flow as ComputeLayoutDamage>::compute_layout_damage(self). Method dispatch ought to automatically performing this upcast, I think, must as we auto-reborrow here and there.

cc @pnkfelix who experimented on this with me

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Apr 29, 2015

triage: P-medium -- annoying, but there exists a workaround

@nikomatsakis nikomatsakis self-assigned this Apr 29, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Apr 29, 2015

I assigned myself but have not yet actively started working on this... it occurs to me it is not entirely clear when this upcast should be done in method dispatch. Perhaps just whenever the (fully transformed) receiver is of the form &Foo or &mut Foo trait object? And this kind of relates to the general desire to do upcasting to allow for calls implemented on supertraits.

The most obvious thing to do would be to wait until we've fully autoderefered to a trait object type (Flow+'a, in this case), but that would not fix this example. It would fix the example if the impl were implemented Flow and the method were &mut self, though.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Apr 29, 2015

Note for future people triaging this bug: there are easy modifications of @nikomatsakis 's test that exhibit the same compile failure that will not infinite loop after this is fixed; for example, this:

pub trait Flow {
    fn foo();
}

pub trait LayoutDamageComputation {
    fn compute_layout_damage(self);
    fn children(&self) -> Vec<Self>;
}

impl<'a> LayoutDamageComputation for &'a mut (Flow + 'a) {
    fn compute_layout_damage(self) {
        for k in self.children() {
            k.compute_layout_damage();
            k.compute_layout_damage();
        }
    }
    fn children(&self) -> Vec<&'a mut (Flow + 'a)> { vec![] }
}

fn main() { }
@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Apr 29, 2015

triage: P-medium -- annoying, but there exists a workaround

The work around exists, but is hardly discoverable… unless “every one ask Niko about their borrowck errors” is acceptable ;)

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented May 1, 2015

@SimonSapin indeed. I intend to fix this soon -- it's just that I don't think it's a "stop everything" emergency (which is what we are trying to use P-High for).

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented May 1, 2015

Ok, fair enough.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 14, 2016

triage: P-low

We've gone this long w/o a fix, probably not P-medium anymore (esp b/c there's a workaround). Also removing @nikomatsakis as an assignee as I don't think this is actively being worked on.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Oct 31, 2018

Triage: i now get

error[E0038]: the trait `Flow` cannot be made into an object
  --> src/main.rs:11:10
   |
11 | impl<'a> LayoutDamageComputation for &'a mut (Flow + 'a) {
   |          ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Flow` cannot be made into an object
   |
   = note: method `foo` has no receiver

error[E0038]: the trait `Flow` cannot be made into an object
  --> src/main.rs:12:5
   |
12 |     fn compute_layout_damage(self) {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Flow` cannot be made into an object
   |
   = note: method `foo` has no receiver

for all cases, including the "fixed" ones. Was this bug fixed in the last two years?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.