diff --git a/CHANGELOG.md b/CHANGELOG.md index 6040fc67..571ebc47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +- Add `IsEnum` constraint for `FieldWriter`s (fix `variant` safety) +- Make field writer `bits` always `unsafe` add `set` for safe writing - Fix bit writer type for `ModifiedWriteValues::ZeroToSet` ## [v0.32.0] - 2024-02-26 diff --git a/src/generate/generic.rs b/src/generate/generic.rs index 1932f97a..563065eb 100644 --- a/src/generate/generic.rs +++ b/src/generate/generic.rs @@ -56,6 +56,9 @@ pub trait FieldSpec: Sized { type Ux: Copy + PartialEq + From; } +/// Marker for fields with fixed values +pub trait IsEnum: FieldSpec {} + /// Trait implemented by readable registers to enable the `read` method. /// /// Registers marked with `Writable` can be also be `modify`'ed. @@ -474,16 +477,13 @@ pub struct Safe; /// You should check that value is allowed to pass to register/field writer marked with this pub struct Unsafe; -/// Write field Proxy with unsafe `bits` -pub type FieldWriter<'a, REG, const WI: u8, FI = u8> = raw::FieldWriter<'a, REG, WI, FI, Unsafe>; -/// Write field Proxy with safe `bits` -pub type FieldWriterSafe<'a, REG, const WI: u8, FI = u8> = raw::FieldWriter<'a, REG, WI, FI, Safe>; +/// Write field Proxy +pub type FieldWriter<'a, REG, const WI: u8, FI = u8, Safety = Unsafe> = raw::FieldWriter<'a, REG, WI, FI, Safety>; -impl<'a, REG, const WI: u8, FI> FieldWriter<'a, REG, WI, FI> +impl<'a, REG, const WI: u8, FI, Safety> FieldWriter<'a, REG, WI, FI, Safety> where REG: Writable + RegisterSpec, FI: FieldSpec, - REG::Ux: From, { /// Field width pub const WIDTH: u8 = WI; @@ -499,7 +499,14 @@ where pub const fn offset(&self) -> u8 { self.o } +} +impl<'a, REG, const WI: u8, FI, Safety> FieldWriter<'a, REG, WI, FI, Safety> +where + REG: Writable + RegisterSpec, + FI: FieldSpec, + REG::Ux: From, +{ /// Writes raw bits to the field /// /// # Safety @@ -511,45 +518,31 @@ where self.w.bits |= (REG::Ux::from(value) & REG::Ux::mask::()) << self.o; self.w } - /// Writes `variant` to the field - #[inline(always)] - pub fn variant(self, variant: FI) -> &'a mut W { - unsafe { self.bits(FI::Ux::from(variant)) } - } } -impl<'a, REG, const WI: u8, FI> FieldWriterSafe<'a, REG, WI, FI> +impl<'a, REG, const WI: u8, FI> FieldWriter<'a, REG, WI, FI, Safe> where REG: Writable + RegisterSpec, FI: FieldSpec, REG::Ux: From, { - /// Field width - pub const WIDTH: u8 = WI; - - /// Field width - #[inline(always)] - pub const fn width(&self) -> u8 { - WI - } - - /// Field offset - #[inline(always)] - pub const fn offset(&self) -> u8 { - self.o - } - /// Writes raw bits to the field #[inline(always)] - pub fn bits(self, value: FI::Ux) -> &'a mut W { - self.w.bits &= !(REG::Ux::mask::() << self.o); - self.w.bits |= (REG::Ux::from(value) & REG::Ux::mask::()) << self.o; - self.w + pub fn set(self, value: FI::Ux) -> &'a mut W { + unsafe { self.bits(value) } } +} + +impl<'a, REG, const WI: u8, FI, Safety> FieldWriter<'a, REG, WI, FI, Safety> +where + REG: Writable + RegisterSpec, + FI: IsEnum, + REG::Ux: From, +{ /// Writes `variant` to the field #[inline(always)] pub fn variant(self, variant: FI) -> &'a mut W { - self.bits(FI::Ux::from(variant)) + unsafe { self.bits(FI::Ux::from(variant)) } } } diff --git a/src/generate/register.rs b/src/generate/register.rs index d21f11f9..614d3c21 100644 --- a/src/generate/register.rs +++ b/src/generate/register.rs @@ -393,7 +393,7 @@ pub fn render_register_mod( // * there is a single field that covers the entire register // * that field can represent all values // * the write constraints of the register allow full range of values - let can_write_safe = !unsafety( + let safe_ty = if let Safety::Safe = Safety::get( register .fields .as_ref() @@ -401,9 +401,14 @@ pub fn render_register_mod( .and_then(|field| field.write_constraint) .as_ref(), rsize, - ) || !unsafety(register.write_constraint.as_ref(), rsize); - let safe_ty = if can_write_safe { "Safe" } else { "Unsafe" }; - let safe_ty = Ident::new(safe_ty, span); + ) { + Safety::Safe + } else if let Safety::Safe = Safety::get(register.write_constraint.as_ref(), rsize) { + Safety::Safe + } else { + Safety::Unsafe + }; + let safe_ty = safe_ty.ident(span); let doc = format!("`write(|w| ..)` method takes [`{mod_ty}::W`](W) writer structure",); @@ -1097,7 +1102,7 @@ pub fn fields( // the read value, or else we reuse read value. if can_write { let mut proxy_items = TokenStream::new(); - let mut unsafety = unsafety(f.write_constraint.as_ref(), width); + let mut safety = Safety::get(f.write_constraint.as_ref(), width); // if we writes to enumeratedValues, generate its structure if it differs from read structure. let value_write_ty = if let Some(ev) = rwenum.write_enum() { @@ -1136,12 +1141,12 @@ pub fn fields( if variants.len() == 1 << width { } else if let Some(def) = def.take() { variants.push(def); - unsafety = false; + safety = Safety::Safe; } // if the write structure is finite, it can be safely written. if variants.len() == 1 << width { - unsafety = false; + safety = Safety::Safe; } // generate write value structure and From conversation if we can't reuse read value structure. @@ -1235,19 +1240,15 @@ pub fn fields( quote! { crate::#wproxy<'a, REG, #value_write_ty> } } } else { - let wproxy = Ident::new( - if unsafety { - "FieldWriter" - } else { - "FieldWriterSafe" - }, - span, - ); + let wproxy = Ident::new("FieldWriter", span); let width = &unsuffixed(width); - if value_write_ty == "u8" { + if value_write_ty == "u8" && safety != Safety::Safe { quote! { crate::#wproxy<'a, REG, #width> } - } else { + } else if safety != Safety::Safe { quote! { crate::#wproxy<'a, REG, #width, #value_write_ty> } + } else { + let safe_ty = safety.ident(span); + quote! { crate::#wproxy<'a, REG, #width, #value_write_ty, crate::#safe_ty> } } }; mod_items.extend(quote! { @@ -1370,22 +1371,40 @@ pub fn fields( )) } -fn unsafety(write_constraint: Option<&WriteConstraint>, width: u32) -> bool { - match &write_constraint { - Some(&WriteConstraint::Range(range)) - if range.min == 0 && range.max == u64::MAX >> (64 - width) => - { - // the SVD has acknowledged that it's safe to write - // any value that can fit in the field - false - } - None if width == 1 => { - // the field is one bit wide, so we assume it's legal to write - // either value into it or it wouldn't exist; despite that - // if a writeConstraint exists then respect it - false +#[derive(Clone, Debug, PartialEq, Eq)] +enum Safety { + Unsafe, + Safe, +} + +impl Safety { + fn get(write_constraint: Option<&WriteConstraint>, width: u32) -> Self { + match &write_constraint { + Some(&WriteConstraint::Range(range)) + if range.min == 0 && range.max == u64::MAX >> (64 - width) => + { + // the SVD has acknowledged that it's safe to write + // any value that can fit in the field + Self::Safe + } + None if width == 1 => { + // the field is one bit wide, so we assume it's legal to write + // either value into it or it wouldn't exist; despite that + // if a writeConstraint exists then respect it + Self::Safe + } + _ => Self::Unsafe, } - _ => true, + } + fn ident(&self, span: Span) -> Ident { + Ident::new( + if let Self::Safe = self { + "Safe" + } else { + "Unsafe" + }, + span, + ) } } @@ -1425,7 +1444,7 @@ impl Variant { span, ); let sc = case.sanitize(&ev.name); - const INTERNALS: [&str; 4] = ["set_bit", "clear_bit", "bit", "bits"]; + const INTERNALS: [&str; 6] = ["bit", "bits", "clear_bit", "set", "set_bit", "variant"]; let sc = Ident::new( &(if INTERNALS.contains(&sc.as_ref()) { sc + "_" @@ -1552,6 +1571,7 @@ fn add_from_variants<'a>( impl crate::FieldSpec for #pc { type Ux = #fty; } + impl crate::IsEnum for #pc {} }); } }