yield has a problem with lifetimes

[manuels]

When working with futures-await I ran into a problem with lifetimes. @Arnavion was able to pin it down to the implementation of generators.

I simplified the code and got rid of futures completely. My current minimal example to reproduce this bug reads:

#![feature(conservative_impl_trait, generators, generator_trait)]

use std::ops::{ Generator, GeneratorState };

struct Foo {
    i: u8,
}

impl Foo {
    fn i<'a>(&'a mut self) -> u8 {
        self.i += 1;
        self.i
    }
}

fn run<'a>(foo: &'a mut Foo) -> impl Generator<Yield = (), Return = u8> + 'a {
    move || {
        if false {
            yield
        }

        let _ = {
            loop {
                match foo.i() {
                    i if i == 0 => break i,
                    i if i == 1 => yield, // Error disappears when removing this line!
                    _ => unreachable!(),
                }
            }
        };

        {
            match foo.i() {
                i if i == 0 => i,
                _ => unreachable!(),
            }
        }
    }
}

fn main() {
    let mut foo = Foo { i: 0 };
    let mut g = run(&mut foo);
    match g.resume() {
        GeneratorState::Complete(i) => println!("{}", i),
        _ => unreachable!(),
    }
}

For some reason the compiler complains:

error[E0499]: cannot borrow `*foo` as mutable more than once at a time
  --> test.rs:24:23
   |
24 |                 match foo.i() {
   |                       ^^^ mutable borrow starts here in previous iteration of loop
...
38 |     }
   |     - mutable borrow ends here

error[E0499]: cannot borrow `*foo` as mutable more than once at a time
  --> test.rs:33:19
   |
24 |                 match foo.i() {
   |                       --- first mutable borrow occurs here
...
33 |             match foo.i() {
   |                   ^^^ second mutable borrow occurs here
...
38 |     }
   |     - first borrow ends here

error: aborting due to 2 previous errors

@Arnavion's diagnosis is

It appears to be a bug in generators arising because Foo::i() returns a value bounded by self's lifetime and there is a yield point while it's borrowed.

I tried to have a look at the rust compiler's implementation of std::ops::Generator by I don't know enough to debug this problem.
The fact that the compiler complains about the lifetime, however, is strange.

[manuels]

Well, this is a bit awkward: After looking at issue 44197 I tried to replace

-match foo.i() {
+let i = foo.i();
+match i {

in the code above, and the compilation suddenly worked fine.
However, if you go back to @Arnavion's code this fix is not applicable anymore and compilation fails again.
@Arnavion's code read:

#![feature(conservative_impl_trait, generators, generator_trait)]

extern crate futures;

use std::ops::{ Generator, GeneratorState };
use futures::{ Async, future, Future };

struct Foo {
    i: u8,
}

impl Foo {
    fn i<'a>(&'a mut self) -> impl Future<Item = u8, Error = ()> + 'a {
        future::ok(self.i)
    }
}

fn run<'a>(foo: &'a mut Foo) -> impl Generator<Yield = (), Return = Result<u8, ()>> + 'a {
    move || {
        if false {
            yield
        }

        let _ = {
            let mut f = foo.i();
            loop {
                let poll = f.poll();
                match poll {
                    Ok(Async::Ready(i)) => break i,
                    Ok(Async::NotReady) => yield,
                    _ => unreachable!(),
                }
            }
        };

        {
            let mut f = foo.i();
            let poll = f.poll();
            match poll {
                Ok(Async::Ready(i)) => Ok(i),
                _ => unreachable!(),
            }
        }
    }
}

fn main() {
    let mut foo = Foo { i: 0 };
    let mut g = run(&mut foo);
    match g.resume() {
        GeneratorState::Complete(Ok(i)) => println!("{}", i),
        _ => unreachable!(),
    }
}

[Arnavion]

Reduced test case:

#![feature(conservative_impl_trait, generators, generator_trait)]

use std::ops::Generator;

trait Tr { }

struct Foo;
impl Tr for Foo { }

struct Bar;
impl Bar {
    fn baz<'a>(&'a mut self) -> impl Tr + 'a {
        Foo
    }
}

fn run<'a>(bar: &'a mut Bar) -> impl Generator<Yield = (), Return = ()> + 'a {
    move || {
        if false {
            yield;
        }

        {
            let _baz = bar.baz();
            if false { yield; }
        }

        {
            bar.baz();
        }
    }
}

fn main() {
}

[Zoxc]

These are all due to borrows being considered live at a suspension point.
It should give error[E0626]: borrow may still be in use when generator yields.
For match foo.i() {, foo is being considered borrowed for the entire match. In @Arnavion's reduced test case, _baz is a borrow which is live across the yield.

@nikomatsakis You wrote the code that should detect these cases. @arielb1 also had some comments about it you should look at.

[manuels]

@Zoxc, Are you saying that this is just the wrong error message or do you agree that this is a 'real' bug?

[Zoxc]

It is just a wrong error message.

[manuels]

I do not see why this code should not work. The mutual reference of self is only used for the baz() call and baz is dropped.
Can you elaborate on why this code is invalid?

[Zoxc]

There actually seem to be something interesting going on here. Here is a reduced test case:

#![feature(generators)]

fn _run(bar: &mut i32) {
    || {
        {
            let _baz = &*bar;
            yield;
        }
        
        *bar = 2;
    };
}

fn main() {}

_baz now stores a reborrow of bar, but because of the yield expression, this reference gets stored in the generator, and because of that, the borrow lasts for the entire generator. Now we get an error when we try to mutate bar because it's borrowed immutably for the entire duration of the generator. This is no longer an error if we move the assignment after the generator, or change it to only dereference bar.

This program is not fundamentally unsafe and would compile if region inference had found a better lifetime for the reborrow. So I guess this is more unfortunate semantics than a bug, but the error message should still be improved.

[manuels]

Thanks, @Zoxc for your explanation. I see now why this code won't compile.
But this seems to be a limitation of the current implementation of generators.
From a user's point of view, I think, there should be no reason why this shouldn't be valid code.
Do you see any possibility to modify the current implementation of generators to make the code work?

I don't know if it's possible to make the compiler inject some code after the yield to drop the reference again?

[Zoxc]

@manuels I'm working on a way to make this work.

[arielb1]

@Zoxc

This is basically non-lexical lifetimes. The borrow is inferred to be potentially live over a yield point, and therefore it's potentially live while the generator is used.

[arielb1]

Or you could attack the "both 'a and 'b can live as long as the generator, but both can be live at the same time" angle:

fn foo(x: &mut u32) {
    // both `s` and `t` are live for the generator's lifetime, but within
    // the generator they have distinct lifetimes.
    move || {
        {
            let s = &mut *x;
            yield;
            *s += 1;
        }

        let t = &mut *x;
        yield;
        *t += 1;
    };
}