Skip to content

borrow checking not transparent to inlining #16594

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
bfops opened this issue Aug 19, 2014 · 6 comments
Closed

borrow checking not transparent to inlining #16594

bfops opened this issue Aug 19, 2014 · 6 comments

Comments

@bfops
Copy link
Contributor

bfops commented Aug 19, 2014

The borrow checker gives different results when functions are manually inlined, giving confusing or annoying results.

The following compiles:

struct Foo {
    x: int,
    y: int,
}

impl Foo {
    pub fn foo(&mut self) {
        let y = &mut self.y;
        self.x = self.x + *y;
    }
}

The following does not:

struct Foo {
    x: int,
    y: int,
}

impl Foo {
    pub fn foo(&mut self) {
        let y = self.bar();
        self.x = self.x + *y;
    }

    pub fn bar(&mut self) -> &mut int {
        &mut self.y
    }
}

I don't believe this to be a duplicate of #6393, because this has not only to do with scoping, but also the fact that a member function always borrows all of self.

@Gankra
Copy link
Contributor

Gankra commented Aug 19, 2014

This strikes me as a bit of a feature. I would be unhappy if changing the implementation details of a function (or a function called by a function... etc), suddenly caused a user of that function to fail borrow-checking. Especially since they'd only be passing borrow-checking because they were being clever (or, much more likely: lucky).

As it stands now, the signature of the function is sufficient to identify all the ownership consequences of calling the function, which is nice.

@bfops
Copy link
Contributor Author

bfops commented Aug 19, 2014

It means there are patterns of common behavior that can't be factored out without aggravating the borrow checker. I have code like this:

for (id, mob) in self.mobs.mut_iter() {
    translate_mob(mob, &self.physics, &self.gl_context, &self.mob_gl_buffers, ..., Vec3::new(1.0, 2.0, 3.0));
}

If I do this in more than a few places, it would be nice to factor this out into a function, but this isn't valid, regardless of how I define self.translate_mob.

for (id, mob) in self.mobs.mut_iter() {
    self.translate_mob(mob, Vec3::new(1.0, 2.0, 3.0));
}

I could use a macro to call the real function:

macro_rules! translate_mob(
  ($world:expr, $mob:expr, $v:expr) => (
    App::translate_mob(
      &$world.gl,
      &mut $world.physics,
      $world.mob_buffers.get_mut_ref(),
      ...
      $mob,
      $v
    );
  );
)

But that's kind of cumbersome. Maybe this only makes sense within the context of implementing class internals, but in that case, I don't think it's really clever or lucky for me to rely on some underlying principle of how another function was implemented, and I certainly want the build breaking if those assumptions change. I'd at least like the option of writing functions with "deep" borrow checking, to avoid the impending boilerplate of writing "static" functions and then writing macros to call them.

@huonw
Copy link
Member

huonw commented Aug 19, 2014

"refinement types" would address this (i.e. a method could dictate exactly which fields it touched). Another approach is factoring out the functionality into distinct structs, so it would be self.mob.mut_iter() and self.world.translate_mob(...).

@bfops
Copy link
Contributor Author

bfops commented Aug 19, 2014

refinement types sound like what I want! The common factoring approach makes sense in some scenarios (and this pressure from the borrow checker has definitely caused me to factor into smaller structs in some places), but it's not universally applicable - in the mob example above, it doesn't really make immediate sense to further couple any of those things together (except maybe the gl buffers into the gl context). Ultimately a struct is composed of some fundamental and distinct units that are going to be used together, like mobs, physics, and gl above.

I think it makes sense to have "deeper" borrow checking as an opt-in (e.g. via explicit typing) - again, I've definitely liked the push back from the borrow checker as a default.

@bfops
Copy link
Contributor Author

bfops commented Aug 19, 2014

It's especially strange-looking with closures:

struct MutThing;

impl MutThing {
  pub fn mut_thing(&mut self, _: ||) {}
}

struct Foo {
    x: MutThing,
    y: int,
}

impl Foo {
    pub fn foo(&mut self) {
        self.x.mut_thing(|| {
          self.y = 1;
        });
    }
}

Especially when the solution is manually borrowing the individual parts of the struct:

struct MutThing;

impl MutThing {
  pub fn mut_thing(&mut self, _: ||) {}
}

struct Foo {
    x: MutThing,
    y: int,
}

impl Foo {
    pub fn foo(&mut self) {
        let y = &mut self.y;
        self.x.mut_thing(|| {
          *y = 1;
        });
    }
}

@steveklabnik
Copy link
Member

Closing in favor of the refinement types RFC issue: rust-lang/rfcs#671

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants