Closure idea: Relax capture rules for closures

language design
1

This is an older issue that has been iterator before, but it pretty much boils down to the following snippet:

let mut value = 0;
let mut incr = || value += 1;
let mut decr = || value -= 1;
incr();
decr();

which currently (rightfully) complains about value being borrowed twice. This is mostly because the code above (somewhat) desugars to

fn main() {
    struct Incr<'a>(&'a mut i32);
    impl Incr<'_> {
        fn call(&mut self) { *self.0 += 1; }
    }
    
    struct Decr<'a>(&'a mut i32);
    impl Decr<'_> { 
        fn call(&mut self) { *self.0 -= 1; }
    }
    let mut value = 0;
    let mut incr = Incr(&mut value);
    let mut decr = Decr(&mut value);

    incr.call();
    decr.call();
}

The desugaring borrow checker error always makes sense because Incr could be holding an internal reference that would be invalidated by whatever Decr is doing

An alternative desugaring for (non-move) closures could be:

fn main() {
    struct Incr<'a>(&'a mut i32);
    impl Incr<'_> {
        fn call(&mut self) { *self.0 += 1; }
    }
    
    struct Decr<'a>(&'a mut i32);
    impl Decr<'_> { 
        fn call(&mut self) { *self.0 -= 1; }
    }
    let mut value = 0;
    let mut incr = Incr(&mut value);
    let mut decr = Decr(&mut value);

    {incr = Incr(&mut value); incr}.call();
    {decr = Decr(&mut value); decr}.call();
}

Translated to closures this rule would sound like:

  1. Every use of a non-move FnMut(or Fn) closure within its definition scope (and by scope I mean a scope in which drop-rules apply) gets replaced by another instance of it with all it's upvars recaptured at that point.
    • I'm limiting this to non move || traits that implement FnMut because those might have non-copy fields which would need to be moved between instances. There is a desugaring that works for that case as well, but it's cumbersome and it should probably be a follow up discussion
    • The advantage with this approach is that it doesn't circumvent the borrow checker, so it shouldn't introduce any security issues, but it would increase MIR size (and likely LLVM IR size) so it would have an impact on compile time.

1. has an unfortunate limitation in that it wouldn't be useful for things like:

let value;
let closure = {
   // some code
  || value.foo();
};

because the rule is too strict. The rule could probably be relaxed so the closure gets instantiated even in higher scopes as long as all it's upvars are available there.

Because of this disadvantage and the (probably) increased cost to compilation due to increased MIR size I propose an alternative (but in my opinion equivalent) solution that is just MIR based.

  1. In the function where the closure is defined the upvars captured by the closure are not considered live between the points of definition/use

The MIR from the first example looks (somewhat, it doesn't compile so I had to edit it by hand) like:

let mut _0: ();
let mut _1: i32; // value
let mut _3: &mut i32; // value upvar captured in incr
let _5: ();
let mut _6: &mut {closure@src/main.rs:4:16: 4:18}; // incr
let mut _7: &mut i32; // value upvar captured in decr
let _8: ();
let mut _9: &mut {closure@src/main.rs:7:8: 7:10}; //decr
scope 1 {
    debug value => _1;
    let mut _2: {closure@src/main.rs:4:16: 4:18};
    scope 2 {
        debug incr => _2;
        let mut _4: {closure@src/main.rs:7:8: 7:10};
        scope 3 {
            debug decr => _4;
        }
    }
}

bb0: {
    _1 = const 0_i32;
    _3 = &mut _1;
    _2 = {closure@src/main.rs:4:16: 4:18} { value: move _3 };
    _6 = &mut _2;
    _7 = &mut _1;
    _4 = {closure@src/main.rs:7:8: 7:10} { value: move _7 };
    _5 = <{closure@src/main.rs:4:16: 4:18} as FnMut<()>>::call_mut(move _6, const ()) -> [return: bb1, unwind continue];
}

bb1: {
    _9 = &mut _4;
    _8 = <{closure@src/main.rs:7:8: 7:10} as FnMut<()>>::call_mut(move _9, const ()) -> [return: bb2, unwind continue];
}

bb2: {
    return;
}

The reason this (rightfully) fails to compile is because at the point of calling incr (result is _5) the reference to _1 (value) is still alive through _4 which needs to be alive because it is being used at call site of decr (result is _8)

For non closure aggregates, calling a method might save a reference to an internal field in the closure but for closures that does not hapen. NLL with this extra rule would allow the code to work with no extra code (MIR or LLVM) generated.

I am somewhat sure about solution 1. being practical and working (with it's limitations), but I'd be more interested to know if solution 2. is safe and sane or am I missing something there?

Thank you!

5 Likes
2

This seems useful, but I'd prefer if it was opt-in because I've written code like this, and it's nice to have the compiler checking for me that I don't try to read count before I'm done reading count_value (except the code is more complicated, so it's not as trivial to see manually):

fn count_values(value: &[Value]) -> u32 {
    let mut count = 0;
    let mut count_value = |_value| { count += 1; };
    for value in values {
        if value.should_be_counted() {
            count_value(value);
        }
    }
    count
}
1 Like