Merge #649

649: max cluster size r=adamgreig a=burrbull



Co-authored-by: Andrey Zgarbul <[email protected]>

Author: bors[bot]

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+- Fixed parentheses in RegisterBlock field accessors
+- Check cluster size, add `max_cluster_size` option
+
 ## [v0.25.0] - 2022-08-02
 
 - Add `feature_peripheral` option which generates cfg features for each peripheral

src/generate/peripheral.rs

@@ -222,13 +222,40 @@ pub fn render(p_original: &Peripheral, index: &Index, config: &Config) -> Result
     Ok(out)
 }
 
+#[derive(Clone, Debug)]
+pub struct ArrayAccessor {
+    pub doc: String,
+    pub name: Ident,
+    pub ty: syn::Type,
+    pub basename: Ident,
+    pub i: syn::LitInt,
+}
+
+impl ArrayAccessor {
+    pub fn to_tokens(&self, method: bool) -> TokenStream {
+        let parens = method.then(|| quote! {()});
+        let doc = &self.doc;
+        let name = &self.name;
+        let ty = &self.ty;
+        let basename = &self.basename;
+        let i = &self.i;
+        quote! {
+            #[doc = #doc]
+            #[inline(always)]
+            pub fn #name(&self) -> &#ty {
+                &self.#basename#parens[#i]
+            }
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 struct RegisterBlockField {
     syn_field: syn::Field,
     description: String,
     offset: u32,
     size: u32,
-    accessors: Option<TokenStream>,
+    accessors: Vec<ArrayAccessor>,
 }
 
 #[derive(Clone, Debug)]
@@ -471,9 +498,6 @@ fn register_or_cluster_block(
 
     for reg_block_field in &ercs_expanded {
         regions.add(reg_block_field)?;
-        if let Some(ts) = &reg_block_field.accessors {
-            accessors.extend(ts.clone());
-        }
     }
 
     // We need to compute the idents of each register/union block first to make sure no conflicts exists.
@@ -524,6 +548,12 @@ fn register_or_cluster_block(
                 reg_block_field.syn_field.to_tokens(&mut region_rbfs);
                 Punct::new(',', Spacing::Alone).to_tokens(&mut region_rbfs);
             }
+            accessors.extend(
+                reg_block_field
+                    .accessors
+                    .iter()
+                    .map(|a| a.to_tokens(is_region_a_union)),
+            );
         }
 
         if !is_region_a_union {
@@ -670,7 +700,7 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
     let mut cluster_expanded = vec![];
 
     let cluster_size = cluster_info_size_in_bits(cluster, config)
-        .with_context(|| format!("Cluster {} has no determinable `size` field", cluster.name))?;
+        .with_context(|| format!("Can't calculate cluster {} size", cluster.name))?;
     let description = cluster
         .description
         .as_ref()
@@ -692,12 +722,21 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                 description,
                 offset: info.address_offset,
                 size: cluster_size,
-                accessors: None,
+                accessors: Vec::new(),
             })
         }
         Cluster::Array(info, array_info) => {
-            let sequential_addresses =
-                (array_info.dim == 1) || (cluster_size == array_info.dim_increment * BITS_PER_BYTE);
+            let increment_bits = array_info.dim_increment * BITS_PER_BYTE;
+            if cluster_size > increment_bits {
+                let cname = &cluster.name;
+                return Err(anyhow!("Cluster {cname} has size {cluster_size} bits that is more then array increment {increment_bits} bits"));
+            }
+            let cluster_size = if config.max_cluster_size {
+                increment_bits
+            } else {
+                cluster_size
+            };
+            let sequential_addresses = (array_info.dim == 1) || (cluster_size == increment_bits);
 
             // if dimIndex exists, test if it is a sequence of numbers from 0 to dim
             let sequential_indexes_from0 = array_info
@@ -717,10 +756,10 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
 
             if array_convertible {
                 let accessors = if sequential_indexes_from0 {
-                    None
+                    Vec::new()
                 } else {
                     let span = Span::call_site();
-                    let mut accessors = TokenStream::new();
+                    let mut accessors = Vec::new();
                     let nb_name_cs = ty_name.to_snake_case_ident(span);
                     for (i, idx) in array_info.indexes().enumerate() {
                         let idx_name =
@@ -731,15 +770,15 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                             &description,
                         );
                         let i = unsuffixed(i as _);
-                        accessors.extend(quote! {
-                            #[doc = #comment]
-                            #[inline(always)]
-                            pub fn #idx_name(&self) -> &#ty {
-                                &self.#nb_name_cs[#i]
-                            }
+                        accessors.push(ArrayAccessor {
+                            doc: comment,
+                            name: idx_name,
+                            ty: ty.clone(),
+                            basename: nb_name_cs.clone(),
+                            i,
                         });
                     }
-                    Some(accessors)
+                    accessors
                 };
                 let array_ty = new_syn_array(ty, array_info.dim);
                 cluster_expanded.push(RegisterBlockField {
@@ -763,7 +802,7 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                     description: info.description.as_ref().unwrap_or(&info.name).into(),
                     offset: info.address_offset,
                     size: 0,
-                    accessors: None,
+                    accessors: Vec::new(),
                 });
             } else {
                 for (field_num, idx) in array_info.indexes().enumerate() {
@@ -776,7 +815,7 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                         description: description.clone(),
                         offset: info.address_offset + field_num as u32 * array_info.dim_increment,
                         size: cluster_size,
-                        accessors: None,
+                        accessors: Vec::new(),
                     });
                 }
             }
@@ -813,7 +852,7 @@ fn expand_register(register: &Register, config: &Config) -> Result<Vec<RegisterB
                 description,
                 offset: info.address_offset,
                 size: register_size,
-                accessors: None,
+                accessors: Vec::new(),
             })
         }
         Register::Array(info, array_info) => {
@@ -838,10 +877,10 @@ fn expand_register(register: &Register, config: &Config) -> Result<Vec<RegisterB
                     .is_some();
 
                 let accessors = if sequential_indexes_from0 {
-                    None
+                    Vec::new()
                 } else {
                     let span = Span::call_site();
-                    let mut accessors = TokenStream::new();
+                    let mut accessors = Vec::new();
                     let nb_name_cs = ty_name.to_snake_case_ident(span);
                     for (i, idx) in array_info.indexes().enumerate() {
                         let idx_name =
@@ -852,15 +891,15 @@ fn expand_register(register: &Register, config: &Config) -> Result<Vec<RegisterB
                             &description,
                         );
                         let i = unsuffixed(i as _);
-                        accessors.extend(quote! {
-                            #[doc = #comment]
-                            #[inline(always)]
-                            pub fn #idx_name(&self) -> &#ty {
-                                &self.#nb_name_cs[#i]
-                            }
+                        accessors.push(ArrayAccessor {
+                            doc: comment,
+                            name: idx_name,
+                            ty: ty.clone(),
+                            basename: nb_name_cs.clone(),
+                            i,
                         });
                     }
-                    Some(accessors)
+                    accessors
                 };
                 let array_ty = new_syn_array(ty, array_info.dim);
                 let syn_field =
@@ -883,7 +922,7 @@ fn expand_register(register: &Register, config: &Config) -> Result<Vec<RegisterB
                         description: description.clone(),
                         offset: info.address_offset + field_num as u32 * array_info.dim_increment,
                         size: register_size,
-                        accessors: None,
+                        accessors: Vec::new(),
                     });
                 }
             }

src/main.rs

@@ -85,6 +85,11 @@ fn run() -> Result<()> {
                 .long("feature_peripheral")
                 .help("Use independent cfg feature flags for each peripheral"),
         )
+        .arg(
+            Arg::with_name("max_cluster_size")
+                .long("max_cluster_size")
+                .help("Use array increment for cluster size"),
+        )
         .arg(
             Arg::with_name("make_mod")
                 .long("make_mod")
@@ -183,6 +188,8 @@ fn run() -> Result<()> {
         cfg.bool_flag("feature_group", Filter::Arg) || cfg.bool_flag("feature_group", Filter::Conf);
     let feature_peripheral = cfg.bool_flag("feature_peripheral", Filter::Arg)
         || cfg.bool_flag("feature_peripheral", Filter::Conf);
+    let max_cluster_size = cfg.bool_flag("max_cluster_size", Filter::Arg)
+        || cfg.bool_flag("max_cluster_size", Filter::Conf);
 
     let mut source_type = cfg
         .grab()
@@ -209,6 +216,7 @@ fn run() -> Result<()> {
         derive_more,
         feature_group,
         feature_peripheral,
+        max_cluster_size,
         output_dir: path.clone(),
         source_type,
     };

src/util.rs

@@ -22,7 +22,7 @@ pub const BITS_PER_BYTE: u32 = 8;
 /// that are not valid in Rust ident
 const BLACKLIST_CHARS: &[char] = &['(', ')', '[', ']', '/', ' ', '-'];
 
-#[derive(Clone, PartialEq, Debug)]
+#[derive(Clone, PartialEq, Eq, Debug)]
 pub struct Config {
     pub target: Target,
     pub nightly: bool,
@@ -36,6 +36,7 @@ pub struct Config {
     pub derive_more: bool,
     pub feature_group: bool,
     pub feature_peripheral: bool,
+    pub max_cluster_size: bool,
     pub output_dir: PathBuf,
     pub source_type: SourceType,
 }
@@ -55,6 +56,7 @@ impl Default for Config {
             derive_more: false,
             feature_group: false,
             feature_peripheral: false,
+            max_cluster_size: false,
             output_dir: PathBuf::from("."),
             source_type: SourceType::default(),
         }
@@ -63,7 +65,7 @@ impl Default for Config {
 
 #[allow(clippy::upper_case_acronyms)]
 #[allow(non_camel_case_types)]
-#[derive(Clone, Copy, PartialEq, Debug)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum Target {
     CortexM,
     Msp430,
@@ -93,7 +95,7 @@ impl Default for Target {
     }
 }
 
-#[derive(Clone, Copy, PartialEq, Debug)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum SourceType {
     Xml,
     #[cfg(feature = "yaml")]