implement exclusive non-overlapping field support

Author: robamu

bitbybit-tests/src/bitfield_tests.rs

@@ -1861,6 +1861,66 @@ fn test_defmt_impl_bitfields() {
     defmt_impl_check(&test);
 }
 
+#[test]
+fn test_forbidden_overlaps_okay_u8() {
+    #[bitfield(u8, forbid_overlaps)]
+    struct Test {
+        #[bits(4..=7, rw)]
+        bit_upper: u4,
+        #[bit(3, rw)]
+        bit_3: bool,
+        #[bit(2, rw)]
+        bit_2: bool,
+        #[bit(1, rw)]
+        bit_1: bool,
+        #[bit(0, rw)]
+        bit_0: bool,
+    }
+    Test::new_with_raw_value(0xFF);
+}
+
+#[test]
+fn test_forbidden_overlaps_okay_u16() {
+    #[bitfield(u16, forbid_overlaps)]
+    struct Test {
+        #[bits(12..=15, rw)]
+        bit_upper: u4,
+        #[bit(11, rw)]
+        bit_11: bool,
+        #[bit(10, rw)]
+        bit_10: bool,
+        #[bit(9, rw)]
+        bit_9: bool,
+        #[bit(8, rw)]
+        bit_8: bool,
+        #[bits(0..=7, rw)]
+        lower_bits: u8,
+    }
+    Test::new_with_raw_value(0x1F1F);
+}
+
+#[test]
+fn test_forbidden_overlaps_okay_u32() {
+    #[bitfield(u32, forbid_overlaps)]
+    struct Test {
+        #[bits(16..=31, rw)]
+        bits_upper_2: u16,
+        #[bits(12..=15, rw)]
+        bits_upper_1: u4,
+        #[bit(11, rw)]
+        bit_11: bool,
+        #[bit(10, rw)]
+        bit_10: bool,
+        #[bit(9, rw)]
+        bit_9: bool,
+        #[bit(8, rw)]
+        bit_8: bool,
+        #[bits(0..=7, rw)]
+        lower_bits: u8,
+    }
+    Test::new_with_raw_value(0x1F1FF1F1);
+}
+
 #[test]
 fn test_defmt_impl_bitfields_on_arbitrary_int_fields_u8_primitive_val() {
     #[bitfield(u6, defmt_bitfields)]

bitbybit/CHANGELOG.md

@@ -7,6 +7,7 @@ This version expects arbitrary-int 2.x.
 ### Added
 
 - Support for signed arbitrary-int integers as field types, e.g. `i3`, `i24`, etc.
+- Added `forbid_overlaps` attribute argument which checks and denies overlaps of `bitfield` fields.
 
 ## bitbybit 1.4.0
 

bitbybit/README.md

@@ -37,7 +37,7 @@ struct GICD_TYPER {
 
 How this works:
 
-- #[bitfield(u32)] specifies that this is a bitfield in which u32 is the underlying data type. This means that all the
+- `#[bitfield(u32)]` specifies that this is a bitfield in which u32 is the underlying data type. This means that all the
   bits inside of the bitfield
   have to fit within 32 bits. Built-in Rust types (u8, u16, u32, u64, u128) as well as arbitrary-ints (u17, u48 etc) are
   supported.
@@ -187,6 +187,23 @@ immediates in a way that they have to be reassembled. This can be achieved like
   }
 ```
 
+## Field overlap checking
+
+In some situations, it can be useful to check for and deny overlapping bits.
+The macro code generator can be instructed to do this using the `forbid_overlaps` specifier.
+
+For example, the following code will not compile:
+
+```rs
+#[bitfield(u32, forbid_overlaps)]
+struct NoOverlaps {
+    #[bits(4..=11, r)]
+    overlapping_portion: u8,
+    #[bits(0..=7, r)]
+    lower_8_bits: u8,
+}
+```
+
 ## Debug
 
 The `bitfield` macro can generate a `Debug` implementation for you which prints

bitbybit/src/bitfield/codegen.rs

@@ -1,6 +1,6 @@
 use crate::bitfield::{
-    const_name, mask_name, setter_name, with_name, BaseDataSize, BitfieldAttributes, CustomType,
-    DefmtVariant, FieldDefinition, BITCOUNT_BOOL,
+    const_name, mask_name, setter_name, with_name, ArrayInfo, BaseDataSize, BitfieldAttributes,
+    CustomType, DefmtVariant, FieldDefinition, BITCOUNT_BOOL,
 };
 use proc_macro2::{Ident, TokenStream as TokenStream2, TokenStream, TokenTree};
 use quote::{quote, TokenStreamExt as _};
@@ -52,7 +52,7 @@ pub fn generate(
             let mask_name = mask_name(field_name);
             let mask = setter_mask(&one, field_definition);
 
-            if let Some((count, stride)) = field_definition.array {
+            if let Some(ArrayInfo { count, indexed_stride: stride }) = field_definition.array {
                 let count_name = const_name(field_name, "COUNT");
                 let stride_name = const_name(field_name, "STRIDE");
                 quote! {
@@ -99,7 +99,7 @@ pub fn generate(
                 };
 
                 if let Some(array) = field_definition.array {
-                    let indexed_count = array.0;
+                    let indexed_count = array.count;
                     quote! {
                         #(#doc_comment)*
                         #[inline]
@@ -160,7 +160,7 @@ pub fn generate(
             let with_name = with_name(field_name);
 
             if let Some(array) = field_definition.array {
-                let indexed_count = array.0;
+                let indexed_count = array.count;
                 quote! {
                     #(#doc_comment)*
                     #[inline]
@@ -235,7 +235,7 @@ fn extracted_bits(
     base_data_size: BaseDataSize,
     total_number_bits: usize,
 ) -> TokenStream {
-    let indexed_stride = field_definition.array.map(|(_, stride)| stride);
+    let indexed_stride = field_definition.array.map(|info| info.indexed_stride);
     let array_shift = indexed_stride.map_or_else(
         || quote! {},
         |indexed_stride| quote! { + index * #indexed_stride },
@@ -296,8 +296,8 @@ fn setter_new_raw_value(
     base_data_size: BaseDataSize,
     internal_base_data_type: &Type,
 ) -> TokenStream {
-    if let Some(array) = field_definition.array {
-        let indexed_stride = array.1;
+    if let Some(array) = &field_definition.array {
+        let indexed_stride = array.indexed_stride;
         // bool?
         if field_definition.field_type_size_from_data_type == Some(BITCOUNT_BOOL) {
             assert_eq!(field_definition.ranges.len(), 1);
@@ -449,8 +449,8 @@ pub fn make_builder(
                 //   .with_a(1, value[1])
                 //   .with_a(2, value[2])
 
-                let array_count = array.0;
-                let array_stride = array.1;
+                let array_count = array.count;
+                let array_stride = array.indexed_stride;
                 if ranges_have_self_overlap(&field_definition.ranges, array_stride, array_count) {
                     return (quote! {}, Vec::new());
                 }

bitbybit/src/bitfield/mod.rs

@@ -52,12 +52,18 @@ fn try_parse_arbitrary_int_type(s: &str, allow_signed: bool) -> Option<(usize, b
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+pub struct ArrayInfo {
+    count: usize,
+    indexed_stride: usize,
+}
+
 #[cfg_attr(feature = "extra-traits", derive(Debug))]
 struct FieldDefinition {
     field_name: Ident,
     ranges: Vec<Range<usize>>,
     unsigned_field_type: Option<Type>,
-    array: Option<(usize, usize)>,
+    array: Option<ArrayInfo>,
     field_type_size: usize,
     getter_type: Option<Type>,
     setter_type: Option<Type>,
@@ -141,6 +147,7 @@ pub enum DefmtVariant {
 struct BitfieldAttributes {
     pub base_type: Option<Ident>,
     pub default_val: Option<DefaultVal>,
+    pub forbid_overlaps: bool,
     pub debug_trait: bool,
     pub introspect: bool,
     pub defmt_trait: Option<DefmtTrait>,
@@ -174,6 +181,10 @@ impl BitfieldAttributes {
             }
             return Ok(());
         }
+        if meta.path.is_ident("forbid_overlaps") {
+            self.forbid_overlaps = true;
+            return Ok(());
+        }
         if meta.path.is_ident("debug") {
             self.debug_trait = true;
             return Ok(());
@@ -265,19 +276,25 @@ pub fn bitfield(args: TokenStream, input: TokenStream) -> TokenStream {
             .unwrap_or_else(|_| panic!("bitfield!: Error parsing internal base data type"));
 
     let input = syn::parse_macro_input!(input as DeriveInput);
-    let struct_name = input.ident;
-    let struct_vis = input.vis;
-    let struct_attrs = input.attrs;
+    let struct_name = &input.ident;
+    let struct_vis = &input.vis;
+    let struct_attrs = &input.attrs;
 
-    let fields = match input.data {
-        Data::Struct(struct_data) => struct_data.fields,
+    let fields = match &input.data {
+        Data::Struct(struct_data) => &struct_data.fields,
         _ => panic!("bitfield!: Must be used on struct"),
     };
 
-    let field_definitions = match parsing::parse(&fields, base_data_size) {
+    let field_definitions = match parsing::parse(fields, base_data_size) {
         Ok(definitions) => definitions,
         Err(token_stream) => return token_stream.into_compile_error().into(),
     };
+
+    if bitfield_attrs.forbid_overlaps {
+        if let Err(e) = check_for_overlaps(&field_definitions, &input) {
+            return e.into_compile_error().into();
+        }
+    }
     let accessors = codegen::generate(
         &field_definitions,
         base_data_size,
@@ -351,16 +368,16 @@ pub fn bitfield(args: TokenStream, input: TokenStream) -> TokenStream {
     }
 
     let defmt_trait = codegen::generate_defmt_trait_impl(
-        &struct_name,
+        struct_name,
         &bitfield_attrs,
         &field_definitions,
         base_data_size,
     );
 
     let (new_with_constructor, new_with_builder_chain) = codegen::make_builder(
-        &struct_name,
+        struct_name,
         bitfield_attrs.default_val.is_some(),
-        &struct_vis,
+        struct_vis,
         &internal_base_data_type,
         base_data_type,
         base_data_size,
@@ -435,6 +452,47 @@ pub fn bitfield(args: TokenStream, input: TokenStream) -> TokenStream {
     TokenStream::from(expanded)
 }
 
+fn check_for_overlaps(
+    field_definitions: &[FieldDefinition],
+    input: &DeriveInput,
+) -> syn::Result<()> {
+    let mut current_bitmask: u128 = 0;
+
+    for field in field_definitions {
+        let mut check_and_update_bitmask = |offset: usize, width: usize| {
+            // Create an all-ones bitmask for the given range, e.g. 0b1110 for range (1..=3).
+            let mask = ((1_u128 << width) - 1) << offset;
+            if (current_bitmask & mask) != 0 {
+                return Err(syn::Error::new_spanned(
+                    input,
+                    format!(
+                        "bitfield!: Detected overlap of field `{}` with other field",
+                        field.field_name
+                    ),
+                ));
+            }
+            current_bitmask |= mask;
+            Ok(())
+        };
+        for range in &field.ranges {
+            let width = range.end - range.start;
+            match field.array {
+                Some(info) => {
+                    let mut current_offset = range.start;
+                    for _ in 0..info.count {
+                        check_and_update_bitmask(current_offset, width)?;
+                        current_offset += info.indexed_stride;
+                    }
+                }
+                None => {
+                    check_and_update_bitmask(range.start, width)?;
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
 fn with_name(field_name: &Ident) -> Ident {
     // The field might have started with r#. If so, it was likely used for a keyword. This can be dropped here
     let field_name_without_prefix = {

bitbybit/src/bitfield/parsing.rs

@@ -1,5 +1,5 @@
 use crate::bitfield::{
-    is_int_size_regular_type, try_parse_arbitrary_int_type, BaseDataSize, CustomType,
+    is_int_size_regular_type, try_parse_arbitrary_int_type, ArrayInfo, BaseDataSize, CustomType,
     FieldDefinition, BITCOUNT_BOOL,
 };
 use proc_macro2::{Ident, Literal, Punct, TokenStream as TokenStream2, TokenTree};
@@ -428,7 +428,10 @@ fn parse_field(base_data_size: usize, field: &Field) -> Result<FieldDefinition>
         primitive_type,
         custom_type,
         doc_comment,
-        array: indexed_count.map(|count| (count, indexed_stride.unwrap())),
+        array: indexed_count.map(|count| ArrayInfo {
+            count,
+            indexed_stride: indexed_stride.unwrap(),
+        }),
         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,

bitbybit/tests/no_compile/overlapping_bitfield_arrays.rs

@@ -0,0 +1,11 @@
+use bitbybit::bitfield;
+
+#[bitfield(u32, default = 0, forbid_overlaps)]
+struct Test {
+    #[bits(8..=15, rw)]
+    overlapping_bits: u8,
+    #[bits(0..=7, rw)]
+    u8_array: [u8; 2],
+}
+
+fn main() {}

bitbybit/tests/no_compile/overlapping_bitfield_arrays.stderr

@@ -0,0 +1,10 @@
+error: bitfield!: Detected overlap of field `u8_array` with other field
+ --> tests/no_compile/overlapping_bitfield_arrays.rs:4:1
+  |
+4 | / struct Test {
+5 | |     #[bits(8..=15, rw)]
+6 | |     overlapping_bits: u8,
+7 | |     #[bits(0..=7, rw)]
+8 | |     u8_array: [u8; 2],
+9 | | }
+  | |_^

bitbybit/tests/no_compile/overlapping_bitfield_arrays_with_stride.rs

@@ -0,0 +1,11 @@
+use bitbybit::bitfield;
+
+#[bitfield(u32, default = 0, forbid_overlaps)]
+struct Test {
+    #[bits(16..=23, rw)]
+    overlapping_bits: u8,
+    #[bits(0..=7, rw, stride = 16)]
+    u8_array: [u8; 2],
+}
+
+fn main() {}

bitbybit/tests/no_compile/overlapping_bitfield_arrays_with_stride.stderr

@@ -0,0 +1,10 @@
+error: bitfield!: Detected overlap of field `u8_array` with other field
+ --> tests/no_compile/overlapping_bitfield_arrays_with_stride.rs:4:1
+  |
+4 | / struct Test {
+5 | |     #[bits(16..=23, rw)]
+6 | |     overlapping_bits: u8,
+7 | |     #[bits(0..=7, rw, stride = 16)]
+8 | |     u8_array: [u8; 2],
+9 | | }
+  | |_^