Async/await and references in pattern match

[earthengine]

I want to share an issue that took me a lot of time to trace the root course.

This is not a new issue of simular things, however, although I have been used to fix things like that without async/await, async/await obstacles the error message and makes the reason very hard to point out in the first place.

Here is a simplified version of the code (Playground):

#![feature(
    await_macro,
    async_await,
    futures_api,
    optin_builtin_traits
)]
use std::future::Future;

struct Foo;
impl Foo {
    fn foo(&self) -> Option<()> { Some(()) }
}
impl !Sync for Foo{}

async fn bar() {
    let f = Foo;
    if let Some(v) = f.foo() {
        await!(async{})
    }
}

async fn buz(f: impl Future<Output=()> + Send) {
    await!(f)
}

fn main(){
    buz(bar());
}

error message:

error[E0277]: `Foo` cannot be shared between threads safely
  --> src/main.rs:27:5
   |
27 |     buz(bar());
   |     ^^^ `Foo` cannot be shared between threads safely
   |

Even when the code simplified like the above, I believe it is not clear to many people to guess the reason. After all, we do not attempt to send a &Foo to anywhere, or do we?

Actually the if let Some(v) = f.foo() line betrayed us. It looks so innocent that it just call foo on f, isn’t it? But it actually keeps the reference of f though the whole block, so the reference will have to go through an await boundry, and therefore the returning Future of the outer async block have to keep a reference of it. As we require the Future to be Send, Foo thus have to be Sync.

The fix, of cause is to use a temporary variable for the pattern match source.

let tmp = f.foo();
if let Some(v) = tmp {...

(There will be warnings but I am only demostrating and at least it will compile)

This kind of error is not new to me - I fixed countless such error before, mostly when I attempt to move things out of a struct when pattern matching an expression using a reference of the struct. However in such cases the error message clearly tell me what I attempted to do and therefore I can quickly spot the error and apply the solution above.

---

Now, as I know that the strategy to create a temporary variable would always work, and the cases that requires a reference to live though out the match block are

1. relatively rare;
2. even in such a case the temporary variable is still good enough to catch the lifetime of the borrow,

Why do we not make this the default behaviour? Am I right to assume that match exp {...} should always behave the same as let tmp=exp; match tmp {...}?

[withoutboats]

This is an interesting intersection of async/await with NLL work. Certainly it isn’t necessary that f be borrowed across the yield point in that function. cc @nikomatsakis

[earthengine]

I think the problem is there before and after NLL - NLL have nothing to do with this, it just another step of checking the code and figured out “Oh, this is never used again, so let’s stop the borrow here”.

However, the borrow is still theoretically across the block, this is why it comes back in the async/await case above.

The proper solution I think, is to confirm the let tmp=exp; match exp {} desugaring. This prevents similar problems forever.

[phaylon]

A simple temporary variable desugaring solution might run into the problem that match doesn’t perform a move unless the patterns move.

[earthengine]

Are there any examples you can show me?

[phaylon]

Sure:

fn main() {
    let foo = String::from("something that's not Copy");
    match foo { ref x => println!("{}", x) }
    let _bar = foo; // not moved yet
}

This will fail to compile if you remove the ref in the match.

Same applies even if the pattern has non-Copy parts but only Copy ones are bound:

fn main() {
    let foo = (String::from("something that's not Copy"), 23);
    match foo { (_, x) => println!("{}", x) }
    let _bar = foo; // not moved yet
}

Both would fail if the value is moved from foo prior to the match.

[earthengine]

I see. So how about the “tmp variable” rule only apply when the match target is an expression?

In your cases they are all variables and variables have a name to refer to. However expressions do not have a name, and so they don’t have this problem and can be moved immidiately.

[phaylon]

I mean, they are expressions. I think if you want such a rule you’d have to say something like “don’t apply it for things that can be moved out of”, which also includes like struct field or tuple access.

Then there is the category of things you can match against but can’t move out of, like *foo.bar().

[earthengine]

That makes sense. We can improve it a little bit to say: “apply if the things matching cannot be invalidated”, this is therefor includes Copy things.

And even the match *foo.bar() should be written let tmp=foo.bar(); match *tmp {...}. So there is still space to improve the rule.

One way to simplify the rule, is to always desugar like let tmp = &mut exp; match *tmp {...}. In other words, the expression always executed before the match, and the result’s unique address was taken and dereferenced immidiately in the match. These extra operations should be optimised out later on, but NLL is now able to eliminate unnessory borrows in the expression evaluation. This will work even when exp is a simple variable.

[phaylon]

That would require the variable to be mut. It also makes it impossible to move out of the variable:

fn main() {
    let mut foo = String::from("something not Copy");
    let tmp = &mut foo;
    // cannot move out of borrowed content
    match *tmp { x => drop(x) }
}

I’m just not sure a simple rewrite to a temporary will be a solution here.

[earthengine]

In theory the code you shown can be allowed under some advanced rules - the variable foo didn’t used after assignment to tmp, and the tmp is a unique reference, therefore *tmp should be the recover of foo.

In short, moving out should be allowed as long as foo is never used again, in theory. If it is not the case, of cause there should be an error. I am a bit supprised that NLL didn’t made this happen.

[earthengine]

Being carefully looked into the issue that causes the &mut tmp not movable, I found myself back to the situation of the very beginning: we cannot move out of *tmp because tmp is considered “live” throughout the match block. This is the same as my f variable being borrowed throughout the block.

So, a simple rewrite would not be enough to solve it. Instead, we have to consider, although if cond {} else {} or match exp {} are all expressions, is it necessory to keep every intermediate result and varialbe alive through the whole expression? We could at least, introduce a “block” boundry, such that every intermediate results to not live across a block subexpression, unless being explicitly referenced.

This is to say we are talking about the lifetime of temporaries, and this is a big debating topic and I believe the Rust team have already debated with this. So I would like to hear from them.