Safety of OperationRef::to_ref()

[tanmay-bakshi]

We hit some interesting lifetime quirks when trying to loop through the operations in a block, so I was digging through the code for OperationRef, and found this:

melior/melior/src/ir/operation.rs

Line 375 in 0b2a9bd

pub unsafe fn to_ref(&self) -> &'a Operation<'c> {

My question is why this function is marked unsafe. In theory, isn't this safe because the &'a Operation<'c> is still bounded by the borrow checker to be live as long as the OperationRef<'c, 'a> would?

It makes sense that the Deref trait doesn't allow us to express that reference lifetime, but doesn't that make the Deref lifetime (the "safe" one) actually incorrect and unnecessarily strict/short? As far as I can tell, the Operation should be bounded in lifetime by the lifetime of the value that the OperationRef was itself accessed from (in our case, a Block), and that's better described by &'a Operation<'c>.

[raviqqe]

In theory, isn't this safe because the &'a Operation<'c> is still bounded by the borrow checker to be live as long as the OperationRef<'c, 'a> would?

It makes sense that the Deref trait doesn't allow us to express that reference lifetime, but doesn't that make the Deref lifetime (the "safe" one) actually incorrect and unnecessarily strict/short?

In Rust, references correspond to pointers. So when we mark it safe, we can drop the OperationRef safely (wrongly,) which invalidates references to it.

As far as I can tell, the Operation should be bounded in lifetime by the lifetime of the value that the OperationRef was itself accessed from (in our case, a Block), and that's better described by &'a Operation<'c>.

We can't do this because the C API doesn't allow this... MlirOperation's internal representation in the C API is void*. So we can assume it to be &'a Operation<'c>. But then that might break the entire design of Operation and OperationRef in the Rust API in the future when they change the representation.

https://github.com/llvm/llvm-project/blob/a309c07ad3fc4a9b0bf41a4e66e812b54338aa02/mlir/include/mlir-c/IR.h#L45

[Moxinilian]

I think it would be nice if OperationRef<'c, 'a> had more documentation on what the lifetimes model. I work with MLIR in C++ a lot yet I am not super sure what they represent. I assume 'c refers to the lifetime of the MLIRContext, but I'm not sure what 'a is about.