Should body.append_operation require a mutable ref?

[jmintb]

Hi, I was a bit surpised that module.body().append_operation does not require a mutable reference. I am not that familiar with MLIR internals yet but the unsafe block inside append_operation does appear to be mutating the body.

Can we make this safer by switching to a mutable reference or am I missing something?

[raviqqe]

Yes. It's a problem of the current design. I've decided to do so originally following the design of Inkwell so that we can represent a graph of operations just like a graph at the level of Rust codes.

For example, when we have mutable references instead of immutable ones in those places, we need to write:

let value = block.append_operation(operation).result(0).unwrap().into();
block.append_operation(func::r#return(&[value], location));

rather than:

block.append_operation(func::r#return(
    &[block.append_operation(operation).result(0).unwrap().into()],
    location,
));

because Rust doesn't allow referencing mutable references at two locations at the same time for linearity.

You can see previous notes and discussions about it in those tickets:

• Roadmap to v1 #24
• Draft: Dialect generation from ODS #274
• Mutable operations vs dynamic checks with RefCell #347

As described in #24, we haven't decided the best solution. But if you have any ideas or opinions, we would like to hear that!