magnus
magnus copied to clipboard
Safety improvements to `rb-sys-interop`
This PR is a safety + ergonomics revamp to the rb-sys-interop
feature. I want to provide some basic sanity checks around when converting from raw ID
and VALUE
. Previously, the methods id_from_raw
and value_from_raw
were infallible, but I think we should at least attempt to provide some safety around these functions when we can. There may be more cases we can check as well (such as checking that the value % 40 == 0
?), but this is a good start.
Quick feedback: sorry this is brief and potentially full of mistakes, but hopefully it’s helpful getting this feedback sooner.
I think I’d like this to be more explicitly not part of the regular Magnus api at the call site in end user code. I’d be happy with something like AsRawValue
and AsRawId
traits that would need to be explicitly use
d in a scope to get the as_raw
/from_raw
methods. Hopefully that’d be a nice balance of improved ergonomics while keeping it clear in end user code and published examples that these functions aren’t for regular use.
I think any error checking should be debug asserts, that only run for debug builds and panic rather than returning an error. Passing an non-VALUE
value to from_raw
isn’t an error that can be handled, it’s something so completely wrong it invalidates the whole program and should crash. But also these functions should optimise away to nothing, but error checks would stop that, so it should be for debug builds only.
Done @matsadler
@matsadler Finally got around to updating this. I also added a PartialEq
derivation for ID which is useful to have, but happy to remove if you do not want it.
This is great, thanks!
Sorry for taking such a long time to merge it.