Conversation
Phantomical
left a comment
There was a problem hiding this comment.
I've gone and approved CI for this. Not sure why it needed approval - unless you haven't accepted the org invite yet? I'm also not sure if I'll have to approve again if you add more commits
I think you have the right idea here but I think that creating traits for these is not necessarily the right approach for this. I don't foresee anyone having a need to generically convert various raw xed types to their corresponding wrappers and there is a real cost to putting core methods in traits where they aren't immediately visible in the docs or, to a lesser extent, in autocomplete.
My suggestion is that we keep the wrapper type macro, but just change it to stamp out some inherent methods instead of having them be in traits. Something like this (you may need to reformat it to match):
/// Declare accessors converting between `$name` and `$raw`.
///
/// # Safety
/// `$name` must be a `#[repr(transparent)]` wrapper around `$raw`.
macro_rules! wrapper_type_accessors {
($name:type => $raw:type) => {
impl $name {
pub fn from_raw(raw: $raw) -> Self {
Self(raw)
}
pub fn from_ref(raw: &$raw) -> Self {
// SAFETY: The user of this macro asserts that $name is a #[repr(transparent)]
// wrapper around $raw
unsafe { &*(raw as *const $raw as *const Self) }
}
pub fn into_raw(self) -> $raw {
self.0
}
pub fn as_raw(&self) -> &$raw {
&self.0
}
pub fn as_raw_mut(&mut self) -> &mut $raw {
&mut self.0
}
}
}and we could also create some doc comments for the various methods as opposed to them being empty.
If we do still need generic conversions on top of that they should probably be From impls for the relevant types.
| "Create a `", stringify!($name), "` from the underlying enum value." | ||
| )] | ||
| pub const fn from_raw(value: u32) -> Option<Self> { | ||
| pub(crate) const fn from_raw(value: u32) -> Option<Self> { |
There was a problem hiding this comment.
That this is const is actually rather important. It means that users of xed can create their own constants for new variants introduced by xed-sys before we get around to adding them here.
No description provided.