Multiple borrows in InstrSeqBuilder.if_else()

[icefoxen]

Summary

I feel awfully silly about this but I seem to have a trivial problem with InstrSeqBuilder.if_else() that seems bonkers but I can't figure out a way around it. ...well, that's a lie, I'm pretty sure Rc<RefCell<T>> would fix it but that shouldn't be necessary here so it Feels Wrong(tm).

I'm writing a compiler that outputs wasm using walrus. I'm in output_expr(bcx: &mut Backend, locals: &Locals, ir: &Ir) which is just a big match statement over the intermediate representation, which is currently just a slightly-lowered AST. Backend contains the walrus::Module, which has to get passed in so that, at the very least, local variables can get looked up and added as necessary. Locals is an index helping keep track of what local variable is where, as well as debugging info like names.

The guilty code is just this:

           fn compile_ifcase(
                bcx: &mut Backend,
                locals: &mut Locals,
                instrs: &mut walrus::InstrSeqBuilder,
                testblock: &Expr,
                trueblock: &[Expr],
                falseblock: &[Expr],
            ) {
                compile_expr(bcx, locals, instrs, &testblock);
                let locals1 = &mut Locals::new();
                let locals2 = &mut Locals::new();
                instrs.if_else(
                    ..., 
                    |then| {
                        compile_exprs(bcx, locals1, then, trueblock);
                    },
                    |else_| {
                        compile_exprs(bcx, locals2, else_, falseblock);
                    },
                );
                locals.add_other(locals1);
                locals.add_other(locals2);
           }

The PROBLEM is that Backend gets borrowed mutably in each the then and else_ closures. There's no way to pass it to the closures as an argument, and there's no way I'm aware of to really explain that the lifetimes of the closures don't really overlap. As you can see I already have to split the Locals index apart into several ones, let each get populated, and then merge them back together, and now that I think of it that's probably the wrong approach.

...but I don't see what other approach I can do, and so I'm feeling like I'm missing something stupid, some option or trick or method in walrus. Because as it is I think you'd have the same problem in anything that might ever have to mutate state during the closures passed to if_else().

Additional Details

I've simplified the code to its essence, or tried to at least. Full code is here: https://hg.sr.ht/~icefox/garnet/browse/src/backend.rs?rev=default#L185 . Very incomplete though.

[fitzgen]

With the API as it currently is, there is no easy workaround other than RefCell.

I think if we had an if_else_builder() method that returned an IfElseBuilder that had an r#if method and an r#else method, then rustc would be able to determine that the two closures passed into the builder's methods are called one after the other, and therefore would borrow check. This is the nicest API I can think of to support this pattern. If you want to play around with it a little and make a PR, I'm happy to review. Relevant sources are over here.

[fitzgen]

Or maybe if_else_builder takes a generic context parameter that is then passed in as an additional parameter to each r#if and r#else function.

[icefoxen]

It's okay as long as I'm not missing something obvious; after some fooling around it turns out the RefCell can contain a &mut Backend and only has to live the length of the .if_else() call. I was tearing my hair crying "now I have to refcell EVERYTHING and it's still going to double-borrow at runtime if the call recurses", but it appears that is not in fact the case. So it's a local nuisance instead of a global redesign. A generic context parameter is the obvious solution that I was thinking of but does make the walrus API arbitrarily a bit more narsty for what is actually just an edge case. A simple note in the function docs might be more my speed.

Having an if_else_builder() does sound interesting though, I might play around with that. Thanks!