Add support for signed arbitrary-ints (danlehmann#90)

Author: danlehmann

bitbybit-tests/Cargo.toml

@@ -9,7 +9,7 @@ publish = false
 
 [dependencies]
 bitbybit = { path = "../bitbybit" }
-arbitrary-int = "1.3"
+arbitrary-int = "2.0"
 defmt = "1"
 
 [features]

bitbybit-tests/src/bitfield_tests.rs

@@ -1,10 +1,7 @@
-use arbitrary_int::u6;
-use arbitrary_int::Number;
-use std::fmt::Debug;
-
-use arbitrary_int::{u1, u12, u13, u14, u2, u24, u3, u30, u4, u48, u5, u57, u7};
+use arbitrary_int::prelude::*;
 use bitbybit::bitenum;
 use bitbybit::bitfield;
+use std::fmt::Debug;
 
 #[test]
 fn test_construction() {
@@ -69,6 +66,58 @@ fn test_getter_and_with() {
     assert_eq!(0x0122, t.raw_value);
 }
 
+#[test]
+fn test_getter_and_with_signed() {
+    #[bitfield(u128, default = 0)]
+    struct Test2 {
+        #[bits(98..=127, rw)]
+        val30: i30,
+
+        #[bits(41..=97, rw)]
+        val57: i57,
+
+        #[bits(28..=40, rw)]
+        val13: i13,
+        #[bits(12..=27, rw)]
+        val16: i16,
+
+        #[bits(4..=11, rw)]
+        baudrate: i8,
+
+        #[bits(0..=3, rw)]
+        some_other_bits: i4,
+    }
+
+    let t = Test2::new_with_raw_value(0xAE42_315A_2134_FE06_3412_345A_2134_FE06);
+    assert_eq!(i30::from_bits(0x2B908C56), t.val30());
+    assert_eq!(i57::from_bits(0x0110_9A7F_031A_091A), t.val57());
+    assert_eq!(i13::new(0x5A2), t.val13());
+    assert_eq!(0x134F, t.val16());
+    assert_eq!(0xE0u8 as i8, t.baudrate());
+    assert_eq!(i4::new(0x6), t.some_other_bits());
+
+    let t = Test2::DEFAULT
+        .with_baudrate(0x12)
+        .with_some_other_bits(i4::new(2));
+    assert_eq!(0x12, t.baudrate());
+    assert_eq!(i4::new(2), t.some_other_bits());
+    assert_eq!(0x0122, t.raw_value);
+
+    // Ensure lower fields aren't accidentally sign extending into higher ones
+    assert_eq!(
+        0,
+        Test2::DEFAULT
+            .with_some_other_bits(i4::from_bits(0b1111))
+            .baudrate()
+    );
+
+    assert_eq!(0, {
+        let mut a = Test2::DEFAULT;
+        a.set_some_other_bits(i4::from_bits(0b1111));
+        a.baudrate()
+    });
+}
+
 #[test]
 fn test_getter_and_with_arbitrary_uint() {
     #[bitfield(u128, default = 0)]
@@ -193,6 +242,42 @@ fn test_u1() {
     assert_eq!(t3.raw_value, 0b10);
 }
 
+#[test]
+fn test_i1() {
+    #[bitfield(u16, default = 0)]
+    struct Test {
+        #[bit(0, rw)]
+        bit0: i1,
+
+        #[bit(1, rw)]
+        bit1: i1,
+
+        #[bits(2..=9, r)]
+        n: u8,
+
+        #[bits(10..=12, w)]
+        m: i3,
+
+        #[bits(13..=15, r)]
+        o: i3,
+    }
+
+    let t = Test::DEFAULT;
+    assert_eq!(t.bit0(), i1::new(0));
+    assert_eq!(t.bit1(), i1::new(0));
+    assert_eq!(t.raw_value, 0b00);
+
+    let t2 = t.with_bit0(i1::from_bits(1));
+    assert_eq!(t2.bit0(), i1::from_bits(1));
+    assert_eq!(t2.bit1(), i1::from_bits(0));
+    assert_eq!(t2.raw_value, 0b01);
+
+    let t3 = t.with_bit1(i1::new(-1));
+    assert_eq!(t3.bit0(), i1::from_bits(0));
+    assert_eq!(t3.bit1(), i1::from_bits(1));
+    assert_eq!(t3.raw_value, 0b10);
+}
+
 #[test]
 fn signed_vs_unsigned() {
     #[bitfield(u32)]

bitbybit/CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## bitbybit 2.0.0
+
+This version expects arbitrary-int 2.x.
+
+### Added
+
+- Support for signed arbitrary-int integers as field types, e.g. `i3`, `i24`, etc.
+
 ## bitbybit 1.4.0
 
 This is the final version to support arbitrary-int 1.x. Future versions will require arbitrary-int 2.x.

bitbybit/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "bitbybit"
-version = "1.4.0"
+version = "2.0.0"
 authors = ["Daniel Lehmann <danlehmannmuc@gmail.com>"]
 edition = "2021"
 description = "Efficient implementation of bit-fields where several numbers are packed within a larger number and bit-enums. Useful for drivers, so it works in no_std environments"
@@ -23,7 +23,7 @@ examples = ["arbitrary-int/defmt"]
 syn = { version = "2.0", features = ["full"] }
 quote = "1.0"
 proc-macro2 = "1.0"
-arbitrary-int = "1.3.0"
+arbitrary-int = "2.0"
 
 [dev-dependencies]
 defmt = "1"

bitbybit/README.md

@@ -284,10 +284,11 @@ struct GICD_TYPER {
 
 ## Dependencies
 
-Arbitrary bit widths like u5 or u67 do not exist in Rust at the moment. Therefore, the following dependency is required:
+Arbitrary bit widths like u5, u67 or i81 do not exist in Rust at the moment. Therefore, the following dependency is
+required:
 
 ```toml
-arbitrary-int = "1.3.0"
+arbitrary-int = "2.0"
 ```
 
 ## Usage

bitbybit/src/bitfield/codegen.rs

@@ -134,8 +134,14 @@ pub fn generate(
                                 quote! { field_value }
                             }
                         } else {
-                            // Once signed arbitrary-ints (e.g. i7) are a thing, we'll need to pay special attention to sign extension here
-                            quote! { field_value.value() }
+                            // For arbitrary-ints, signed numbers provide to_bits() which return an
+                            // unsigned, non-sign extended number. Using value() would be incorrect
+                            // here as the sign bit would pollute fields defined higher up.
+                            if field_definition.is_signed {
+                                quote! { field_value.to_bits() }
+                            } else {
+                                quote! { field_value.value() }
+                            }
                         }
                     }
                     CustomType::Yes(_) => {
@@ -260,8 +266,9 @@ fn extracted_bits(
             quote! {  #packed as #primitive_type }
         }
     } else {
+        let prefix = if field_definition.is_signed { "i" } else { "u" };
         let custom_type =
-            TokenStream2::from_str(format!("arbitrary_int::u{}", total_number_bits).as_str())
+            TokenStream2::from_str(format!("arbitrary_int::{prefix}{total_number_bits}").as_str())
                 .unwrap();
         let extract =
             TokenStream2::from_str(format!("extract_u{}", base_data_size.internal).as_str())

bitbybit/src/bitfield/mod.rs

@@ -23,21 +23,32 @@ const fn is_int_size_regular_type(size: usize) -> bool {
     size == BITCOUNT_BOOL || size == 8 || size == 16 || size == 32 || size == 64 || size == 128
 }
 
-fn try_parse_arbitrary_int_type(s: &str) -> Option<usize> {
-    if !s.starts_with('u') || s.len() < 2 {
+/// Parses an integer string, like "u8" or "i24".
+/// Allows "i" types if allow_signed is true.
+///
+/// Returns Some((size, is_signed)) if successful, None if not.
+fn try_parse_arbitrary_int_type(s: &str, allow_signed: bool) -> Option<(usize, bool)> {
+    if s.len() < 2 {
         return None;
     }
 
-    let size = usize::from_str(s.split_at(1).1);
-    match size {
-        Ok(size) => {
-            if (1..128).contains(&size) && !is_int_size_regular_type(size) {
-                Some(size)
-            } else {
-                None
+    let is_unsigned = s.starts_with('u');
+    let is_signed = s.starts_with('i');
+
+    if is_unsigned || (is_signed && allow_signed) {
+        let size = usize::from_str(s.split_at(1).1);
+        match size {
+            Ok(size) => {
+                if (1..128).contains(&size) && !is_int_size_regular_type(size) {
+                    Some((size, is_signed))
+                } else {
+                    None
+                }
             }
+            Err(_) => None,
         }
-        Err(_) => None,
+    } else {
+        None
     }
 }
 
@@ -51,6 +62,7 @@ struct FieldDefinition {
     getter_type: Option<Type>,
     setter_type: Option<Type>,
     field_type_size_from_data_type: Option<usize>,
+    is_signed: bool,
     /// If non-null: (count, stride)
     use_regular_int: bool,
     primitive_type: TokenStream2,
@@ -238,8 +250,8 @@ pub fn bitfield(args: TokenStream, input: TokenStream) -> TokenStream {
         "u32" => BaseDataSize::new(32),
         "u64" => BaseDataSize::new(64),
         "u128" => BaseDataSize::new(128),
-        s if try_parse_arbitrary_int_type(s).is_some() => {
-            BaseDataSize::new(try_parse_arbitrary_int_type(s).unwrap())
+        s if try_parse_arbitrary_int_type(s, false).is_some() => {
+            BaseDataSize::new(try_parse_arbitrary_int_type(s, false).unwrap().0)
         }
         _ => {
             return syn::Error::new_spanned(

bitbybit/src/bitfield/parsing.rs

@@ -23,22 +23,22 @@ pub fn parse(fields: &Fields, base_data_size: BaseDataSize) -> Result<Vec<FieldD
     Ok(field_definitions)
 }
 
-fn parse_scalar_field(ty: &Type) -> Result<(Option<usize>, bool)> {
+fn parse_scalar_field(ty: &Type) -> Result<Option<(usize, bool)>> {
     match ty {
         Type::Path(path) => {
             let type_str = path.to_token_stream().to_string();
             let result = match type_str.as_str() {
-                "bool" => Some((Some(BITCOUNT_BOOL), false)),
-                "u8" => Some((Some(8), false)),
-                "i8" => Some((Some(8), true)),
-                "u16" => Some((Some(16), false)),
-                "i16" => Some((Some(16), true)),
-                "u32" => Some((Some(32), false)),
-                "i32" => Some((Some(32), true)),
-                "u64" => Some((Some(64), false)),
-                "i64" => Some((Some(64), true)),
-                "u128" => Some((Some(128), false)),
-                "i128" => Some((Some(128), true)),
+                "bool" => Some(Some((BITCOUNT_BOOL, false))),
+                "u8" => Some(Some((8, false))),
+                "i8" => Some(Some((8, true))),
+                "u16" => Some(Some((16, false))),
+                "i16" => Some(Some((16, true))),
+                "u32" => Some(Some((32, false))),
+                "i32" => Some(Some((32, true))),
+                "u64" => Some(Some((64, false))),
+                "i64" => Some(Some((64, true))),
+                "u128" => Some(Some((128, false))),
+                "i128" => Some(Some((128, true))),
                 _ => None,
             };
 
@@ -47,15 +47,15 @@ fn parse_scalar_field(ty: &Type) -> Result<(Option<usize>, bool)> {
             }
 
             if let Some(last_segment) = path.path.segments.last() {
-                return Ok((try_parse_arbitrary_int_type(&last_segment.ident.to_string()), false));
+                return Ok(try_parse_arbitrary_int_type(&last_segment.ident.to_string(), true));
             }
 
             Err(Error::new(path.span(), "invalid path for bitfield field"))
         }
         _ => Err(Error::new(
             ty.span(),
             format!(
-                "bitfield!: Field type {} not valid. Supported types: bool, u8, i8, u16, i16, u32, i32, u64, i64, u128, i128, arbitrary int (e.g., u1, u3, u62). Their arrays are also supported.",
+                "bitfield!: Field type {} not valid. Supported types: bool, u8, i8, u16, i16, u32, i32, u64, i64, u128, i128, arbitrary int (e.g., u1, u3, u62, i81). Their arrays are also supported.",
                 ty.into_token_stream()
             ),
         )),
@@ -132,14 +132,16 @@ fn parse_field(base_data_size: usize, field: &Field) -> Result<FieldDefinition>
             _ => (&field.ty, None),
         }
     };
-    let (field_type_size_from_data_type, field_type_is_signed) = parse_scalar_field(ty)?;
-    let unsigned_field_type = if field_type_is_signed {
-        Some(
-            syn::parse_str::<Type>(
-                format!("u{}", field_type_size_from_data_type.unwrap()).as_str(),
+    let field_type_size_from_data_type = parse_scalar_field(ty)?;
+    let unsigned_field_type = if let Some((bits, is_signed)) = field_type_size_from_data_type {
+        if is_signed {
+            Some(
+                syn::parse_str::<Type>(format!("u{}", bits).as_str())
+                    .unwrap_or_else(|_| panic!("bitfield!: Error parsing unsigned_field_type")),
             )
-            .unwrap_or_else(|_| panic!("bitfield!: Error parsing unsigned_field_type")),
-        )
+        } else {
+            None
+        }
     } else {
         None
     };
@@ -323,7 +325,7 @@ fn parse_field(base_data_size: usize, field: &Field) -> Result<FieldDefinition>
                 panic!("bitfield!: number_of_bits is too large!")
             }
         }),
-        Some(b) => (b, quote! { #ty }),
+        Some((b, _is_signed)) => (b, quote! { #ty }),
     };
 
     if field_type_size == BITCOUNT_BOOL {
@@ -401,7 +403,7 @@ fn parse_field(base_data_size: usize, field: &Field) -> Result<FieldDefinition>
     };
 
     let use_regular_int = match field_type_size_from_data_type {
-        Some(i) => is_int_size_regular_type(i),
+        Some((i, _is_signed)) => is_int_size_regular_type(i),
         None => {
             // For CustomTypes (e.g. enums), prefer u1 over bool
             number_of_bits != 1 && is_int_size_regular_type(number_of_bits)
@@ -427,7 +429,8 @@ fn parse_field(base_data_size: usize, field: &Field) -> Result<FieldDefinition>
         custom_type,
         doc_comment,
         array: indexed_count.map(|count| (count, indexed_stride.unwrap())),
-        field_type_size_from_data_type,
+        field_type_size_from_data_type: field_type_size_from_data_type.map(|v| v.0),
+        is_signed: field_type_size_from_data_type.map_or(false, |v| v.1),
         unsigned_field_type,
     })
 }