Borrow check still has problems

[GuillaumeGomez]

From the current PR, it appears than unsafe isn't considered as a block/scope. Is it normal?

[durka]

That's weird, because it definitely is a scope.

[GuillaumeGomez]

cc @EPashkin

[durka]

I looked around in that PR but it seems to be a confusing web of generated code. Could you paste an entire function that shows the issue? You have some lifetime parameters flying around (in to_glib_none) so I don't think it's likely to turn out to be a rustc bug, but it could be.

On Fri, Mar 17, 2017 at 8:49 PM, Guillaume Gomez notifications@github.com wrote:

cc @EPashkin https://github.com/EPashkin

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/40613#issuecomment-287503962, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC3n1V3xf8PbVDV7eF9rcX3I_EYRHoVks5rmyoLgaJpZM4MhDCJ .

[EPashkin]

Failing function looks like this (moved it inside unsafe block)

--- a/src/auto/application.rs
+++ b/src/auto/application.rs
@@ -94,9 +94,11 @@ impl Application {
         }
     }

-    pub fn inhibit<'a, T: IsA<Window>, U: Into<Option<&'a str>>>(&self, window: Option<&T>, flags: ApplicationInhibitFlags, reason: U) -> u32 {
+    pub fn inhibit<'a, 'b, P: IsA<Window> + 'a, Q: Into<Option<&'a P>>, R: Into<Option<&'b str>>>(&self, window: Q, flags: ApplicationInhibitFlags, reason: R) -> u32 {
         unsafe {
-            ffi::gtk_application_inhibit(self.to_glib_none().0, window.to_glib_none().0, flags.to_glib(), reason.into().to_glib_none().0)
+            let window_i = window.into();
+            let reason = reason.into();
+            ffi::gtk_application_inhibit(self.to_glib_none().0, window_i.to_glib_none().0, flags.to_glib(), reason.to_glib_none().0)
         }
     }

to_glib_none has long dependencies https://github.com/gtk-rs/glib/blob/master/src/translate.rs#L276 https://github.com/gtk-rs/glib/blob/master/src/translate.rs#L305 https://github.com/gtk-rs/glib/blob/master/src/object.rs#L164

Error produced:

error: `window_i` does not live long enough
   --> D:/eap/rust/0/gtk\src\auto\application.rs:102:9
    |
101 |             ffi::gtk_application_inhibit(self.to_glib_none().0, window_i.to_glib_none().0, flags.to_glib(), reason.to_glib_none().0)
    |                                                                 -------- borrow occurs here
102 |         }
    |         ^ `window_i` dropped here while still borrowed
103 |     }
    |     - borrowed value needs to live until here

[EPashkin]

But if I move call to_glib_none to separate variable all builds fine.

--- a/src/auto/application.rs
+++ b/src/auto/application.rs
@@ -97,8 +97,9 @@ impl Application {
     pub fn inhibit<'a, 'b, P: IsA<Window> + 'a, Q: Into<Option<&'a P>>, R: Into<Option<&'b str>>>(&self, window: Q, flags: ApplicationInhibitFlags, reason: R) -> u32 {
         unsafe {
             let window_i = window.into();
+            let window_s = window_i.to_glib_none();
             let reason = reason.into();
-            ffi::gtk_application_inhibit(self.to_glib_none().0, window_i.to_glib_none().0, flags.to_glib(), reason.to_glib_none().0)
+            ffi::gtk_application_inhibit(self.to_glib_none().0, window_s.0, flags.to_glib(), reason.to_glib_none().0)
         }
     }

[Mark-Simulacrum]

I believe this is not a bug, or at least a minimal example doesn't reproduce -- I feel like the scoping issues you've encountered are likely due to an accidentally extended lifetime in the function signature or something like that, that is, not a rust bug. It'd be great if you could provide a more minimal example (even if it is an extraction of said crate).

fn main() {
    let mut x = 10;
    unsafe {
        let y = &mut x;
    }
    let z = &x;
}

[EPashkin]

I failed removing glib dependence but here gio example with only one function: https://github.com/EPashkin/gio/tree/minimal_rust_40613. Strange that separate call from_glib2 works too.

[Mark-Simulacrum]

That appears to compile today, so I'm not sure...

[EPashkin]

@Mark-Simulacrum Sorry I uncommented good case. Fixed now.

[Mark-Simulacrum]

Ideally, we'd either inline or get rid of to_glib_none, however, I think I may know the problem. let parent1 = parent.into() doesn't borrow parent, it moves it -- which means that it will go out of scope at the end of the unsafe block. However, parent1.to_glib_none().0 (as far as I can tell, and the types here are somewhat hairy) is &Pwhich means that, since this is the last expression in the block, all temporaries will be "returned" from the block (that is, they must live as long as the block's outer scope); therefore, we're trying to drop parent1 while parent1.to_glib_none() borrowed from it. This is why moving that to a separate statement/expression (not the final one) fixes this problem.

[EPashkin]

Thanks for explanation. I though that borrow checker don't count pointers as references: parent1.to_glib_none().0 is *mut GFile.

[Mark-Simulacrum]

Hm, it doesn't. I think the problem might be that borrowck isn't as smart as you think it is -- it's seeing the parent1.to_glib_none() expression and requiring that that value live as long as the block; independent of the fact that it doesn't really need to.

[EPashkin]

Then why moving parent1 = parent.into() out unsafe block also produce error: block ended, so all borrows must ended too?

[Mark-Simulacrum]

Because the last expression of the block currently must live as long as the block; which means that the borrow isn't ended yet. (poorly indented code which demonstrates this follows), this works:

impl File {
    pub fn has_parent<'a, P: IsA<File> + 'a, Q: Into<Option<&'a P>>>(&self, parent: Q) -> bool {
            let parent1 = parent.into();
let a = {
        unsafe {
            //This works
            //let parent2 = parent1.to_glib_none();
            //from_glib2(ffi::g_file_has_parent(self.to_glib_none().0, parent2.0))

            //This fails
            from_glib2(ffi::g_file_has_parent(self.to_glib_none().0, parent1.to_glib_none().0))

            //This works too
            //let v=ffi::g_file_has_parent(self.to_glib_none().0, parent1.to_glib_none().0);
            //from_glib2(v)
        }
};
a
    }
}

[EPashkin]

So it just limitation of current version of borrowck. Thanks

[steveklabnik]

It's been two years, and the borrow checker has been replaced entirely. I cannot compile the old minimal example due to a glib dependency mismatch.

I think I'm going to give this a close, as it seems like this is known/intended behavior, and doesn't seem to be tracking some sort of huge adjustment we want to make to the borrow checker. If we get a new minimal example (preferably one with no external dependencies) that compiles today, we could re-open this if it's important :)