From f33d7752f81dd8952a0214fb63d41a8e4e59bc18 Mon Sep 17 00:00:00 2001 From: Paul Bergmann Date: Thu, 7 Dec 2023 12:30:50 +0100 Subject: [PATCH 1/3] :sparkles: Implement read-only and write-only fields This is useful when implement drivers for memory mapped I/O. Some of the device registers should only be read/written, but not the opposite. Fixes #27 --- src/lib.rs | 161 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 121 insertions(+), 40 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 07f54ce..0c44f1e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -410,6 +410,7 @@ struct Member { bits: usize, base_ty: syn::Type, default: TokenStream, + access: AccessMode, inner: Option, } @@ -449,6 +450,7 @@ impl Member { mut default, into, from, + access, } = parse_field(&base_ty, &attrs, &ty, ignore)?; if bits > 0 && !ignore { @@ -485,6 +487,7 @@ impl Member { bits, base_ty, default, + access, inner: Some(MemberInner { ident, ty, @@ -504,31 +507,39 @@ impl Member { bits, base_ty, default, + access, inner: None, }) } } fn debug(&self) -> TokenStream { - if let Some(inner) = &self.inner { - let ident_str = inner.ident.to_string(); - let ident = &inner.ident; - quote!(.field(#ident_str, &self.#ident())) - } else { - quote!() + match (&self.inner, self.access) { + (Some(inner), AccessMode::ReadWrite | AccessMode::ReadOnly) => { + let ident_str = inner.ident.to_string(); + let ident = &inner.ident; + quote!(.field(#ident_str, &self.#ident())) + }, + _ => { + quote!() + } } } fn default(&self) -> TokenStream { let default = &self.default; - if let Some(inner) = &self.inner { - let ident = &inner.ident; - let with_ident = format_ident!("with_{ident}"); - quote!(this = this.#with_ident(#default);) - } else { - let offset = self.offset; - let base_ty = &self.base_ty; - quote!(this.0 |= (#default as #base_ty) << #offset;) + + match (&self.inner, self.access) { + (Some(inner), AccessMode::ReadWrite | AccessMode::WriteOnly) => { + let ident = &inner.ident; + let with_ident = format_ident!("with_{ident}"); + quote!(this = this.#with_ident(#default);) + } + _ => { + let offset = self.offset; + let base_ty = &self.base_ty; + quote!(this.0 |= (#default as #base_ty) << #offset;) + } } } } @@ -540,6 +551,7 @@ impl ToTokens for Member { bits, base_ty, default: _, + access, inner: Some(MemberInner { ident, @@ -572,36 +584,46 @@ impl ToTokens for Member { let mask = u128::MAX >> (u128::BITS - *bits as u32); let mask = syn::LitInt::new(&format!("0x{mask:x}"), Span::mixed_site()); - let code = quote! { + let mut code = quote! { const #bits_ident: usize = #bits; const #offset_ident: usize = #offset; + }; - #doc - #[doc = #location] - #[cfg_attr(debug_assertions, track_caller)] - #vis const fn #with_ident(self, value: #ty) -> Self { - let value: #base_ty = { - let this = value; - #into - }; - #[allow(unused_comparisons)] - debug_assert!(value <= #mask, "value out of bounds"); - Self(self.0 & !(#mask << #offset) | (value & #mask) << #offset) - } - #doc - #[doc = #location] - #vis const fn #ident(&self) -> #ty { - let this = (self.0 >> #offset) & #mask; - #from - } - #doc - #[doc = #location] - #[cfg_attr(debug_assertions, track_caller)] - #vis fn #set_ident(&mut self, value: #ty) { - *self = self.#with_ident(value); - } + if matches!(access, AccessMode::ReadWrite | AccessMode::ReadOnly) { + code.extend(quote! { + #doc + #[doc = #location] + #vis const fn #ident(&self) -> #ty { + let this = (self.0 >> #offset) & #mask; + #from + } + }); + } + + if matches!(access, AccessMode::ReadWrite | AccessMode::WriteOnly) { + code.extend(quote! { + #doc + #[doc = #location] + #[cfg_attr(debug_assertions, track_caller)] + #vis const fn #with_ident(self, value: #ty) -> Self { + let value: #base_ty = { + let this = value; + #into + }; + #[allow(unused_comparisons)] + debug_assert!(value <= #mask, "value out of bounds"); + Self(self.0 & !(#mask << #offset) | (value & #mask) << #offset) + } + + #doc + #[doc = #location] + #[cfg_attr(debug_assertions, track_caller)] + #vis fn #set_ident(&mut self, value: #ty) { + *self = self.#with_ident(value); + } + }); + } - }; tokens.extend(code); } } @@ -627,6 +649,8 @@ struct Field { default: TokenStream, into: TokenStream, from: TokenStream, + + access: AccessMode, } /// Parses the `bits` attribute that allows specifying a custom number of bits. @@ -641,6 +665,12 @@ fn parse_field( e } + let default_access = if ignore { + AccessMode::None + } else { + AccessMode::ReadWrite + }; + // Defaults for the different types let (class, ty_bits) = type_bits(ty); let mut ret = match class { @@ -650,6 +680,7 @@ fn parse_field( default: quote!(false), into: quote!(this as _), from: quote!(this != 0), + access: default_access, }, TypeClass::SInt => Field { bits: ty_bits, @@ -657,6 +688,7 @@ fn parse_field( default: quote!(0), into: quote!(), from: quote!(), + access: default_access, }, TypeClass::UInt => Field { bits: ty_bits, @@ -664,6 +696,7 @@ fn parse_field( default: quote!(0), into: quote!(this as _), from: quote!(this as _), + access: default_access, }, TypeClass::Other => Field { bits: ty_bits, @@ -671,6 +704,7 @@ fn parse_field( default: quote!(), into: quote!(#ty::into_bits(this)), from: quote!(#ty::from_bits(this)), + access: default_access, }, }; @@ -691,6 +725,7 @@ fn parse_field( default, into, from, + access, } = syn::parse2(tokens.clone()).map_err(|e| malformed(e, attr))?; // bit size @@ -711,6 +746,14 @@ fn parse_field( )); } + // Ensure padding fields remain inaccesible + if ignore && access.is_some_and(|mode| mode != AccessMode::None) { + return Err(syn::Error::new( + default.span(), + "'access' may only be 'None' (or unset) for padding", + )); + } + // conversion if let Some(into) = into { ret.into = quote!(#into(this)); @@ -726,6 +769,11 @@ fn parse_field( if let Some(default) = default { ret.default = default.into_token_stream(); } + + // access mode + if let Some(access) = access { + ret.access = access; + } } } @@ -769,6 +817,7 @@ struct BitsAttr { default: Option, into: Option, from: Option, + access: Option, } impl Parse for BitsAttr { @@ -778,6 +827,7 @@ impl Parse for BitsAttr { default: None, into: None, from: None, + access: None, }; if let Ok(bits) = syn::LitInt::parse(input) { attr.bits = Some(bits.base10_parse()?); @@ -798,6 +848,8 @@ impl Parse for BitsAttr { attr.into = Some(input.parse()?); } else if ident == "from" { attr.from = Some(input.parse()?); + } else if ident == "access" { + attr.access = Some(input.parse()?); } if input.is_empty() { @@ -811,6 +863,35 @@ impl Parse for BitsAttr { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AccessMode { + ReadWrite, + ReadOnly, + WriteOnly, + None, +} + +impl Parse for AccessMode { + fn parse(input: ParseStream) -> syn::Result { + let mode = input.parse::()?; + + if mode == "RW" { + Ok(AccessMode::ReadWrite) + } else if mode == "RO" { + Ok(AccessMode::ReadOnly) + } else if mode == "WO" { + Ok(AccessMode::WriteOnly) + } else if mode == "None" { + Ok(AccessMode::None) + } else { + Err(syn::Error::new( + mode.span(), + "Invalid access mode, only RW/RO/WO/None are allowed", + )) + } + } +} + #[derive(Clone, Copy, PartialEq)] enum Order { Lsb, From 59e2ba61ea5ccef6b95f684ecbbff1eb889965d4 Mon Sep 17 00:00:00 2001 From: Paul Bergmann Date: Thu, 7 Dec 2023 12:02:34 +0100 Subject: [PATCH 2/3] :white_check_mark: Add tests for access field on bit attribute --- src/lib.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- tests/test.rs | 19 +++++++++++++++++-- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0c44f1e..6877766 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -986,7 +986,7 @@ fn type_bits(ty: &syn::Type) -> (TypeClass, usize) { mod test { use quote::quote; - use crate::{BitsAttr, Order, Params}; + use crate::{AccessMode, BitsAttr, Order, Params}; #[test] fn parse_args() { @@ -1011,26 +1011,67 @@ mod test { assert!(attr.default.is_none()); assert!(attr.into.is_none()); assert!(attr.from.is_none()); + assert!(attr.access.is_none()); - let args = quote!(8, default = 8); + let args = quote!(8, default = 8, access = RW); let attr = syn::parse2::(args).unwrap(); assert_eq!(attr.bits, Some(8)); assert!(attr.default.is_some()); assert!(attr.into.is_none()); assert!(attr.from.is_none()); + assert_eq!(attr.access, Some(AccessMode::ReadWrite)); - let args = quote!(default = 8); + let args = quote!(access = RO); let attr = syn::parse2::(args).unwrap(); assert_eq!(attr.bits, None); - assert!(attr.default.is_some()); + assert!(attr.default.is_none()); assert!(attr.into.is_none()); assert!(attr.from.is_none()); + assert_eq!(attr.access, Some(AccessMode::ReadOnly)); - let args = quote!(3, into = into_something, default = 1, from = from_something); + let args = quote!(default = 8, access = WO); + let attr = syn::parse2::(args).unwrap(); + assert_eq!(attr.bits, None); + assert!(attr.default.is_some()); + assert!(attr.into.is_none()); + assert!(attr.from.is_none()); + assert_eq!(attr.access, Some(AccessMode::WriteOnly)); + + let args = quote!( + 3, + into = into_something, + default = 1, + from = from_something, + access = None + ); let attr = syn::parse2::(args).unwrap(); assert_eq!(attr.bits, Some(3)); assert!(attr.default.is_some()); assert!(attr.into.is_some()); assert!(attr.from.is_some()); + assert_eq!(attr.access, Some(AccessMode::None)); + } + + #[test] + fn parse_access_mode() { + let args = quote!(RW); + let mode = syn::parse2::(args).unwrap(); + assert_eq!(mode, AccessMode::ReadWrite); + + let args = quote!(RO); + let mode = syn::parse2::(args).unwrap(); + assert_eq!(mode, AccessMode::ReadOnly); + + let args = quote!(WO); + let mode = syn::parse2::(args).unwrap(); + assert_eq!(mode, AccessMode::WriteOnly); + + let args = quote!(None); + let mode = syn::parse2::(args).unwrap(); + assert_eq!(mode, AccessMode::None); + + let args = quote!(garbage); + let mode = syn::parse2::(args); + assert!(mode.is_err()); } } diff --git a/tests/test.rs b/tests/test.rs index fba5130..7ed43d6 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -25,8 +25,17 @@ fn members() { #[bits(16)] custom: CustomEnum, /// public field -> public accessor functions - #[bits(12)] + #[bits(9)] pub public: usize, + /// Can specify the access mode for fields, Read Write being the default + #[bits(1, access = RW)] + read_write: bool, + /// Can also specify read only fields... + #[bits(1, access = RO)] + read_only: bool, + /// ...and write only fields + #[bits(1, access = WO)] + write_only: bool, /// padding #[bits(5)] __: (), @@ -57,7 +66,11 @@ fn members() { .with_int(3 << 15) .with_tiny(1) .with_negative(-3) - .with_public(2); + .with_public(2) + .with_read_write(true) + // Would not compile + // .with_read_only(true) + .with_write_only(false); println!("{val:?}"); @@ -73,6 +86,8 @@ fn members() { assert_eq!(val.tiny(), 1); assert_eq!(val.custom(), CustomEnum::B); assert_eq!(val.public(), 2); + assert_eq!(val.read_write(), true); + assert_eq!(val.read_only(), false); // const members assert_eq!(MyBitfield::FLAG_BITS, 1); From 4af40a001f4bc30277861a0447148223841809ec Mon Sep 17 00:00:00 2001 From: Paul Bergmann Date: Thu, 7 Dec 2023 12:42:24 +0100 Subject: [PATCH 3/3] :memo: Add documentation for field access mode --- README.md | 19 +++++++++++++++++-- src/lib.rs | 21 ++++++++++++++++++--- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a0404b1..96ed985 100644 --- a/README.md +++ b/README.md @@ -104,8 +104,17 @@ struct MyBitfield { #[bits(16)] custom: CustomEnum, /// public field -> public accessor functions - #[bits(12)] + #[bits(9)] pub public: usize, + /// Can specify the access mode for fields, Read Write being the default + #[bits(1, access = RW)] + read_write: bool, + /// Can also specify read only fields... + #[bits(1, access = RO)] + read_only: bool, + /// ...and write only fields + #[bits(1, access = WO)] + write_only: bool, /// padding #[bits(5)] __: u8, @@ -139,7 +148,11 @@ let mut val = MyBitfield::new() .with_tiny(1) .with_negative(-3) .with_custom(CustomEnum::B) - .with_public(2); + .with_public(2) + .with_read_write(true) + // Would not compile + // .with_read_only(true) + .with_write_only(false); println!("{val:?}"); let raw: u64 = val.into(); @@ -151,6 +164,8 @@ assert_eq!(val.negative(), -3); assert_eq!(val.tiny(), 1); assert_eq!(val.custom(), CustomEnum::B); assert_eq!(val.public(), 2); +assert_eq!(val.read_write(), true); +assert_eq!(val.read_only(), false); // const members assert_eq!(MyBitfield::FLAG_BITS, 1); diff --git a/src/lib.rs b/src/lib.rs index 6877766..6abd3d7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,8 +95,17 @@ //! #[bits(16)] //! custom: CustomEnum, //! /// public field -> public accessor functions -//! #[bits(12)] +//! #[bits(9)] //! pub public: usize, +//! /// Can specify the access mode for fields, Read Write being the default +//! #[bits(1, access = RW)] +//! read_write: bool, +//! /// Can also specify read only fields... +//! #[bits(1, access = RO)] +//! read_only: bool, +//! /// ...and write only fields +//! #[bits(1, access = WO)] +//! write_only: bool, //! /// padding //! #[bits(5)] //! __: u8, @@ -130,7 +139,11 @@ //! .with_tiny(1) //! .with_negative(-3) //! .with_custom(CustomEnum::B) -//! .with_public(2); +//! .with_public(2) +//! .with_read_write(true) +//! // Would not compile +//! // .with_read_only(true) +//! .with_write_only(false); //! //! println!("{val:?}"); //! let raw: u64 = val.into(); @@ -142,6 +155,8 @@ //! assert_eq!(val.tiny(), 1); //! assert_eq!(val.custom(), CustomEnum::B); //! assert_eq!(val.public(), 2); +//! assert_eq!(val.read_write(), true); +//! assert_eq!(val.read_only(), false); //! //! // const members //! assert_eq!(MyBitfield::FLAG_BITS, 1); @@ -519,7 +534,7 @@ impl Member { let ident_str = inner.ident.to_string(); let ident = &inner.ident; quote!(.field(#ident_str, &self.#ident())) - }, + } _ => { quote!() }