Surprising/"exciting" behaviour of inner move closures

So it seems move closures propagate. That is, something like:

fn main() {
    let a = ::std::cell::Cell::new(1);
    let b = ::std::cell::Cell::new(2);
    let c = ::std::cell::Cell::new(3);
    let x = |a: ::std::cell::Cell<_>, b: ::std::cell::Cell<_>| {
        move || {
            a.set(4);
            b.set(5);
            c.set(6);
        }
    };
    let x = x(a, b);
}

will actually move c despite the closure around it not being itself move. This is surprising because we'd never expect a move closure to move things through a non-move closure!

We mean we guess it is consistent with non-move closures being able to move things, but eh. This just feels like a giant footgun...

Being able to be explicit about moves and non-moves seems more useful. After all, moving into a closure has very different semantics and runtime cost than borrowing (mutably or sharedly). While we doubt our existing code has any bugs related to this, we can definitely imagine ways of getting this wrong, particularly with unsafe...

And we can definitely imagine using something like this to create malicious code that passes code review.

That's true, but perfectly fine and normal, since non-move closure also move values if they are used by value inside the closure (as is the case when there is an inner move closure capturing the value).

5 Likes

Yeah, non-move closures are not "do not move" closures, they are "decide implicitly" closures. E.g. this code is perfectly valid:

let a = vec![1];
let consume_a = || drop(a);

It's good to be cautious with implicit behavior, but I don't think this one is any kind of footgun. It's hard for me to imagine a scenario where this behavior would cause a bug, because moving into the inner closure is maximally restrictive – if you "accidentally reuse the same value outside the closure", that's a hard error, so you notice it. And if you make the opposite mistake, where you "accidentally don't reuse the value outside the closure", then not-moving the value won't catch your mistake anyway.

The closure rules are, at worst, a code-writing annoyance – once I convince the compiler to accept the code, I've never ended up having a mistake that resulted from them.

5 Likes

Where this would potentially cause issues is interacting with @Soni's other proposal, Closure ergonomics/movable whole bindings, where "move closuring" a binding removes it from scope and reëxposes any binding that it shadowed.

(This is effectively in the wrong thread but) this is why it's important that this isn't the case. Your motivating examples all have the shadowing binding directly before the move closure that removes it, but a) it's an implicit whether a binding gets moved, and b) any binding, no matter how lexically far it is from the closure, can be moved.

This is the reason we have the borrow checker in the first place. It's too onerous to expect the programmer to track exactly where bindings are borrowed and moved, so the borrow checker exists to make sure that what you've written makes sense. If moving a binding reëxposes the shadowed binding, then while it's not a use-after-move, you're still using a binding different from the one you expected to use, which is a change in program semantics from what the programmer thought and what the machine understood.

1 Like

I'd be more surprised if it didn't move c, since it's used in the move closure. And indeed, this "use a check to be explicit about what moved" code fails (i.e. the explicit move overrides implicit conditions):

    let x = |a: ::std::cell::Cell<_>, b: ::std::cell::Cell<_>| {
        move || {
            a.set(4);
            b.set(5);
            c.set(6);
        }
    };
    let _ = &c;

But you can get what is apparently more intuitive to you with a bit more explicitness and shadowing:

    let x = |a: ::std::cell::Cell<_>, b: ::std::cell::Cell<_>| {
        let c = &c;
        move || {
            a.set(4);
            b.set(5);
            c.set(6);
        }
    };
    // works
    let _ = &c;

Playground.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.