From 0832b9b9f1fadb32709594043b5d60093157aebb Mon Sep 17 00:00:00 2001 From: n8tlarsen Date: Wed, 5 Oct 2022 11:36:00 -0500 Subject: [PATCH 1/3] Identify disjoint arrays and expand them --- CHANGELOG.md | 1 + Cargo.toml | 1 + src/generate/peripheral.rs | 681 ++++++++++++++++++++++++++++--------- 3 files changed, 515 insertions(+), 168 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfb2e766..4992308a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Bring documentation on how to generate MSP430 PACs up to date (in line with [msp430_svd](https://github.com/pftbest/msp430_svd)). - Prefix submodule path with self:: when reexporting submodules to avoid ambiguity in crate path. +- Add handling for disjoint arrays ## [v0.25.1] - 2022-08-22 diff --git a/Cargo.toml b/Cargo.toml index af3be6b6..7a1608fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,6 +55,7 @@ thiserror = "1.0" serde = { version = "1.0", optional = true } serde_json = { version = "1.0.85", optional = true } serde_yaml = { version = "0.9.11", optional = true } +regex = "1.6.0" [dependencies.svd-parser] features = ["expand"] diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs index fba9b431..f29f5701 100644 --- a/src/generate/peripheral.rs +++ b/src/generate/peripheral.rs @@ -1,5 +1,7 @@ +use regex::Regex; use std::borrow::Cow; use std::cmp::Ordering; +use std::fmt; use svd_parser::expand::{derive_cluster, derive_peripheral, derive_register, BlockPath, Index}; use crate::svd::{array::names, Cluster, ClusterInfo, Peripheral, Register, RegisterCluster}; @@ -16,6 +18,31 @@ use anyhow::{anyhow, bail, Context, Result}; use crate::generate::register; +/// An enum defining how to represent registers in the rendered outputs, binding a bool +/// which describes whether or not renderers should include type information, +/// which allows for disjoint arrays to share the same type. +#[derive(Debug)] +enum Repr { + Array { include_info: bool }, + Expand { include_info: bool }, + // String to store the common type when the erc is part of a disjoint array + Single { include_info: bool, common: String }, +} + +impl Default for Repr { + fn default() -> Self { + Repr::Expand { + include_info: false, + } + } +} + +impl fmt::Display for Repr { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self:?}") + } +} + pub fn render(p_original: &Peripheral, index: &Index, config: &Config) -> Result { let mut out = TokenStream::new(); @@ -191,17 +218,20 @@ pub fn render(p_original: &Peripheral, index: &Index, config: &Config) -> Result return Ok(TokenStream::new()); } + debug!("Checking array representation information"); + let reprs = check_erc_reprs(&ercs, config)?; + debug!("Pushing cluster & register information into output"); // Push all cluster & register related information into the peripheral module - let mod_items = render_ercs(&mut ercs, &path, index, config)?; + let mod_items = render_ercs(&mut ercs, &reprs, &path, index, config)?; // Push any register or cluster blocks into the output debug!( "Pushing {} register or cluster blocks into output", ercs.len() ); - let reg_block = register_or_cluster_block(&ercs, None, None, config)?; + let reg_block = register_or_cluster_block(&ercs, &reprs, None, None, config)?; let open = Punct::new('{', Spacing::Alone); let close = Punct::new('}', Spacing::Alone); @@ -484,6 +514,7 @@ fn make_comment(size: u32, offset: u32, description: &str) -> String { fn register_or_cluster_block( ercs: &[RegisterCluster], + reprs: &[Repr], name: Option<&str>, size: Option, config: &Config, @@ -491,8 +522,8 @@ fn register_or_cluster_block( let mut rbfs = TokenStream::new(); let mut accessors = TokenStream::new(); - let ercs_expanded = - expand(ercs, config).with_context(|| "Could not expand register or cluster block")?; + let ercs_expanded = expand(ercs, reprs, config) + .with_context(|| "Could not expand register or cluster block")?; // Locate conflicting regions; we'll need to use unions to represent them. let mut regions = FieldRegions::default(); @@ -631,15 +662,20 @@ fn register_or_cluster_block( /// Expand a list of parsed `Register`s or `Cluster`s, and render them to /// `RegisterBlockField`s containing `Field`s. -fn expand(ercs: &[RegisterCluster], config: &Config) -> Result> { +fn expand( + ercs: &[RegisterCluster], + reprs: &[Repr], + config: &Config, +) -> Result> { let mut ercs_expanded = vec![]; - debug!("Expanding registers or clusters into Register Block Fields"); - for erc in ercs { - match &erc { + let zipped = ercs.iter().zip(reprs.iter()); + + for (erc, repr) in zipped { + match erc { RegisterCluster::Register(register) => { let reg_name = ®ister.name; - let expanded_reg = expand_register(register, config).with_context(|| { + let expanded_reg = expand_register(register, repr, config).with_context(|| { let descrip = register.description.as_deref().unwrap_or("No description"); format!("Error expanding register\nName: {reg_name}\nDescription: {descrip}") })?; @@ -648,10 +684,13 @@ fn expand(ercs: &[RegisterCluster], config: &Config) -> Result { let cluster_name = &cluster.name; - let expanded_cluster = expand_cluster(cluster, config).with_context(|| { - let descrip = cluster.description.as_deref().unwrap_or("No description"); - format!("Error expanding cluster\nName: {cluster_name}\nDescription: {descrip}") - })?; + let expanded_cluster = + expand_cluster(cluster, repr, config).with_context(|| { + let descrip = cluster.description.as_deref().unwrap_or("No description"); + format!( + "Error expanding cluster\nName: {cluster_name}\nDescription: {descrip}" + ) + })?; trace!("Cluster: {cluster_name}"); ercs_expanded.extend(expanded_cluster); } @@ -663,12 +702,251 @@ fn expand(ercs: &[RegisterCluster], config: &Config) -> Result Result> { + let mut ercs_type_info: Vec<(String, Option, &RegisterCluster, &mut Repr)> = vec![]; + let mut reprs: Vec = vec![]; + // fill the array so we can slice it (without implementing Clone) + for _i in 0..ercs.len() { + reprs.push(Repr::default()); + } + let reprs_slice = &mut reprs[0..ercs.len()]; + let zipped = ercs.iter().zip(reprs_slice.iter_mut()); + for (erc, repr) in zipped { + match erc { + RegisterCluster::Register(register) => { + let info_name = register.fullname(config.ignore_groups); + match register { + Register::Single(_) => { + let mut ty_name = info_name.to_string(); + let mut prev_match = false; + if ercs_type_info.len() > 0 { + // Check this type against previous regexs + for prev in &ercs_type_info { + let (prev_name, prev_regex, prev_erc, _prev_repr) = prev; + if let RegisterCluster::Register(_) = prev_erc { + if let Some(prev_re) = prev_regex { + // if matched adopt the previous type name + if prev_re.is_match(&ty_name) { + ty_name = prev_name.to_string(); + prev_match = true; + debug!( + "Register {} matched to array {}, expanding array", + register.name, prev_name + ); + break; + } + } + } + } + } + // don't render info if we matched a previous erc + *repr = Repr::Single { + include_info: !prev_match, + common: ty_name.clone(), + }; + ercs_type_info.push((ty_name, None, erc, repr)); + } + + Register::Array(info, array_info) => { + // Only match integer indeces when searching for disjoint arrays + let re_string = util::replace_suffix(&info_name, "([0-9]+|%s)"); + let re = Regex::new(format!("^{re_string}$").as_str()).or_else(|_| { + Err(anyhow!( + "Error creating regex for register {}", + register.name + )) + })?; + let ty_name = info_name.to_string(); // keep suffix for regex matching + let mut prev_match = None; + if ercs_type_info.len() > 0 { + // Check this type regex against previous names + for prev in &mut ercs_type_info { + let (prev_name, _prev_regex, prev_erc, prev_repr) = prev; + if let RegisterCluster::Register(_) = prev_erc { + // if matched force this cluster and the previous one to expand + if re.is_match(&prev_name) { + debug!( + "Array {} matched to register {}, expanding array", + register.name, prev_name + ); + (**prev_repr, prev_match) = + redo_prev_repr(*prev_repr, &prev_match, &ty_name); + } + } + } + } + *repr = if prev_match.is_some() { + Repr::Expand { + // don't duplicate info if another array was matched + include_info: !prev_match.unwrap(), + } + } else { + // Check if the array itself requires expansion + let register_size = register.properties.size.ok_or_else(|| { + anyhow!("Register {} has no `size` field", register.name) + })?; + let sequential_addresses = (array_info.dim == 1) + || (register_size == array_info.dim_increment * BITS_PER_BYTE); + let convert_list = match config.keep_list { + true => match &array_info.dim_name { + Some(dim_name) => dim_name.contains("[%s]"), + None => info.name.contains("[%s]"), + }, + false => true, + }; + let r = if sequential_addresses && convert_list { + Repr::Array { include_info: true } + } else { + Repr::Expand { include_info: true } + }; + r + }; + ercs_type_info.push((ty_name, Some(re), erc, repr)); + } + }; + } + + RegisterCluster::Cluster(cluster) => { + match cluster { + Cluster::Single(_) => { + let mut ty_name = cluster.name.to_string(); + let mut prev_match = false; + if ercs_type_info.len() > 0 { + // Check this type against previous regexs + for prev in &ercs_type_info { + let (prev_name, prev_regex, prev_erc, _prev_repr) = prev; + if let RegisterCluster::Cluster(_) = prev_erc { + if let Some(prev_re) = prev_regex { + // if matched adopt the previous type name + if prev_re.is_match(&ty_name) { + debug!( + "Cluster {} matched to array {}, expanding array", + cluster.name, prev_name + ); + ty_name = prev_name.to_string(); + prev_match = true; + break; + } + } + } + } + } + // don't render info if we matched a previous erc + *repr = Repr::Single { + include_info: !prev_match, + common: ty_name.clone(), + }; + ercs_type_info.push((ty_name, None, erc, repr)); + } + + Cluster::Array(info, array_info) => { + // Only match integer indeces when searching for disjoint arrays + let re_string = util::replace_suffix(&cluster.name, "([0-9]+|%s)"); + let re = Regex::new(format!("^{re_string}$").as_str()).or_else(|_| { + Err(anyhow!( + "creating regex for register {} failed", + cluster.name + )) + })?; + let ty_name = cluster.name.to_string(); // keep suffix for regex matching + let mut prev_match = None; + if ercs_type_info.len() > 0 { + // Check this type regex against previous names + for prev in &mut ercs_type_info { + let (prev_name, _prev_regex, prev_erc, prev_repr) = prev; + if let RegisterCluster::Cluster(_) = prev_erc { + // if matched force this cluster and the previous one to expand + if re.is_match(&prev_name) { + debug!( + "array {} matched to cluster {}, expanding array", + cluster.name, prev_name + ); + (**prev_repr, prev_match) = + redo_prev_repr(*prev_repr, &prev_match, &ty_name); + } + } + } + } + *repr = if prev_match.is_some() { + Repr::Expand { + // don't duplicate info if another array was matched + include_info: !prev_match.unwrap(), + } + } else { + // Check if the array itself requires expansion + let cluster_size = cluster_info_size_in_bits(cluster, repr, config) + .with_context(|| { + format!("Can't calculate cluster {} size", cluster.name) + })?; + let increment_bits = array_info.dim_increment * BITS_PER_BYTE; + let sequential_addresses = + (array_info.dim == 1) || (cluster_size == increment_bits); + let convert_list = match config.keep_list { + true => match &array_info.dim_name { + Some(dim_name) => dim_name.contains("[%s]"), + None => info.name.contains("[%s]"), + }, + false => true, + }; + if sequential_addresses && convert_list { + Repr::Array { include_info: true } + } else { + Repr::Expand { include_info: true } + } + }; + ercs_type_info.push((ty_name, Some(re), erc, repr)) + } + }; + } + }; + } + Ok(reprs) +} + +/// Called when a previous `Repr`esentation needs to be updated because it matches the regex of a +/// disjoint array. Returns a tuple of the updated `Repr` and match status. +fn redo_prev_repr( + prev_repr: &Repr, + prev_match: &Option, + ty_name: &String, +) -> (Repr, Option) { + match prev_repr { + Repr::Array { .. } => ( + // include info only if there wasn't a previous array match + Repr::Expand { + include_info: if let Some(b) = prev_match { !b } else { true }, + }, + Some(true), // found a match and it is an array + ), + Repr::Expand { .. } => ( + Repr::Expand { + include_info: if let Some(b) = prev_match { !b } else { true }, + }, + Some(true), // found a match and it is an array + ), + Repr::Single { .. } => ( + Repr::Single { + include_info: false, + common: ty_name.clone(), + }, + if prev_match.is_none() { + Some(false) // found a match and it isn't an array + } else { + *prev_match // don't overwrite a previous match when this is a single + }, + ), + } +} + /// Calculate the size of a Cluster. If it is an array, then the dimensions /// tell us the size of the array. Otherwise, inspect the contents using /// [cluster_info_size_in_bits]. -fn cluster_size_in_bits(cluster: &Cluster, config: &Config) -> Result { +fn cluster_size_in_bits(cluster: &Cluster, repr: &Repr, config: &Config) -> Result { match cluster { - Cluster::Single(info) => cluster_info_size_in_bits(info, config), + Cluster::Single(info) => cluster_info_size_in_bits(info, repr, config), // If the contained array cluster has a mismatch between the // dimIncrement and the size of the array items, then the array // will get expanded in expand_cluster below. The overall size @@ -678,7 +956,7 @@ fn cluster_size_in_bits(cluster: &Cluster, config: &Config) -> Result { return Ok(0); // Special case! } let last_offset = (dim.dim - 1) * dim.dim_increment * BITS_PER_BYTE; - let last_size = cluster_info_size_in_bits(info, config); + let last_size = cluster_info_size_in_bits(info, repr, config); Ok(last_offset + last_size?) } } @@ -686,13 +964,13 @@ fn cluster_size_in_bits(cluster: &Cluster, config: &Config) -> Result { /// Recursively calculate the size of a ClusterInfo. A cluster's size is the /// maximum end position of its recursive children. -fn cluster_info_size_in_bits(info: &ClusterInfo, config: &Config) -> Result { +fn cluster_info_size_in_bits(info: &ClusterInfo, repr: &Repr, config: &Config) -> Result { let mut size = 0; for c in &info.children { let end = match c { RegisterCluster::Register(reg) => { - let reg_size: u32 = expand_register(reg, config)? + let reg_size: u32 = expand_register(reg, repr, config)? .iter() .map(|rbf| rbf.size) .sum(); @@ -700,7 +978,7 @@ fn cluster_info_size_in_bits(info: &ClusterInfo, config: &Config) -> Result (reg.address_offset * BITS_PER_BYTE) + reg_size } RegisterCluster::Cluster(clust) => { - (clust.address_offset * BITS_PER_BYTE) + cluster_size_in_bits(clust, config)? + (clust.address_offset * BITS_PER_BYTE) + cluster_size_in_bits(clust, repr, config)? } }; @@ -710,10 +988,16 @@ fn cluster_info_size_in_bits(info: &ClusterInfo, config: &Config) -> Result } /// Render a given cluster (and any children) into `RegisterBlockField`s -fn expand_cluster(cluster: &Cluster, config: &Config) -> Result> { +/// `Option`al bool parameter set to `Some(false)` will force an array to be expanded +/// while `None` or `Some(true)` will follow the conversion check. +fn expand_cluster( + cluster: &Cluster, + repr: &Repr, + config: &Config, +) -> Result> { let mut cluster_expanded = vec![]; - let cluster_size = cluster_info_size_in_bits(cluster, config) + let cluster_size = cluster_info_size_in_bits(cluster, repr, config) .with_context(|| format!("Can't calculate cluster {} size", cluster.name))?; let description = cluster .description @@ -722,7 +1006,19 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result util::replace_suffix(&common, ""), + _ => { + bail!( + "Cluster {} is a single, but was given a {}", + cluster.name, + repr + ); + } + } } else { util::replace_suffix(&cluster.name, "") }; @@ -750,7 +1046,6 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result Result match &array_info.dim_name { - Some(dim_name) => dim_name.contains("[%s]"), - None => info.name.contains("[%s]"), - }, - false => true, - }; - - let array_convertible = sequential_addresses && convert_list; + match repr { + Repr::Array { .. } => { + let accessors = if sequential_indexes_from0 { + Vec::new() + } else { + let span = Span::call_site(); + 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 = + util::replace_suffix(&info.name, &idx).to_snake_case_ident(span); + let comment = make_comment( + cluster_size, + info.address_offset + (i as u32) * cluster_size / 8, + &description, + ); + let i = unsuffixed(i as _); + accessors.push(ArrayAccessor { + doc: comment, + name: idx_name, + ty: ty.clone(), + basename: nb_name_cs.clone(), + i, + }); + } + accessors + }; + let array_ty = new_syn_array(ty, array_info.dim); + cluster_expanded.push(RegisterBlockField { + syn_field: new_syn_field( + ty_name.to_snake_case_ident(Span::call_site()), + array_ty, + ), + description, + offset: info.address_offset, + size: cluster_size * array_info.dim, + accessors, + }); + } - if array_convertible { - let accessors = if sequential_indexes_from0 { - Vec::new() - } else { - let span = Span::call_site(); - 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 = - util::replace_suffix(&info.name, &idx).to_snake_case_ident(span); - let comment = make_comment( - cluster_size, - info.address_offset + (i as u32) * cluster_size / 8, - &description, - ); - let i = unsuffixed(i as _); - accessors.push(ArrayAccessor { - doc: comment, - name: idx_name, - ty: ty.clone(), - basename: nb_name_cs.clone(), - i, + Repr::Expand { .. } => { + if sequential_indexes_from0 && config.const_generic { + // Include a ZST ArrayProxy giving indexed access to the + // elements. + let ap_path = array_proxy_type(ty, array_info); + let syn_field = + new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ap_path); + cluster_expanded.push(RegisterBlockField { + syn_field, + description: info.description.as_ref().unwrap_or(&info.name).into(), + offset: info.address_offset, + size: 0, + accessors: Vec::new(), }); + } else { + for (field_num, idx) in array_info.indexes().enumerate() { + let nb_name = util::replace_suffix(&info.name, &idx); + let syn_field = new_syn_field( + nb_name.to_snake_case_ident(Span::call_site()), + ty.clone(), + ); + cluster_expanded.push(RegisterBlockField { + syn_field, + description: description.clone(), + offset: info.address_offset + + field_num as u32 * array_info.dim_increment, + size: cluster_size, + accessors: Vec::new(), + }); + } } - accessors - }; - let array_ty = new_syn_array(ty, array_info.dim); - cluster_expanded.push(RegisterBlockField { - syn_field: new_syn_field( - ty_name.to_snake_case_ident(Span::call_site()), - array_ty, - ), - description, - offset: info.address_offset, - size: cluster_size * array_info.dim, - accessors, - }); - } else if sequential_indexes_from0 && config.const_generic { - // Include a ZST ArrayProxy giving indexed access to the - // elements. - let ap_path = array_proxy_type(ty, array_info); - let syn_field = - new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ap_path); - cluster_expanded.push(RegisterBlockField { - syn_field, - description: info.description.as_ref().unwrap_or(&info.name).into(), - offset: info.address_offset, - size: 0, - accessors: Vec::new(), - }); - } else { - for (field_num, idx) in array_info.indexes().enumerate() { - let nb_name = util::replace_suffix(&info.name, &idx); - let syn_field = - new_syn_field(nb_name.to_snake_case_ident(Span::call_site()), ty.clone()); - - cluster_expanded.push(RegisterBlockField { - syn_field, - description: description.clone(), - offset: info.address_offset + field_num as u32 * array_info.dim_increment, - size: cluster_size, - accessors: Vec::new(), - }); + } + _ => { + bail!( + "cluster {} is an array, but was given a {}", + cluster.name, + repr + ); } } } @@ -840,8 +1140,14 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result Result> { +/// numeral indexes, or not containing all elements from 0 to size) they will be expanded. +/// `Option`al bool parameter set to `Some(false)` will force an array to be expanded +/// while `None` or `Some(true)` will follow the conversion check. +fn expand_register( + register: &Register, + repr: &Repr, + config: &Config, +) -> Result> { let mut register_expanded = vec![]; let register_size = register @@ -852,7 +1158,19 @@ fn expand_register(register: &Register, config: &Config) -> Result util::replace_suffix(&common, ""), + _ => { + bail!( + "register {} is a single, but was given a {}", + register.name, + repr + ); + } + } } else { util::replace_suffix(&info_name, "") }; @@ -860,7 +1178,7 @@ fn expand_register(register: &Register, config: &Config) -> Result { - let syn_field = new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ty); + let syn_field = new_syn_field(info_name.to_snake_case_ident(Span::call_site()), ty); register_expanded.push(RegisterBlockField { syn_field, description, @@ -870,75 +1188,75 @@ fn expand_register(register: &Register, config: &Config) -> Result { - let sequential_addresses = (array_info.dim == 1) - || (register_size == array_info.dim_increment * BITS_PER_BYTE); - - let convert_list = match config.keep_list { - true => match &array_info.dim_name { - Some(dim_name) => dim_name.contains("[%s]"), - None => info.name.contains("[%s]"), - }, - false => true, - }; - - let array_convertible = sequential_addresses && convert_list; - - if array_convertible { - // if dimIndex exists, test if it is a sequence of numbers from 0 to dim - let sequential_indexes_from0 = array_info - .indexes_as_range() - .filter(|r| *r.start() == 0) - .is_some(); - - let accessors = if sequential_indexes_from0 { - Vec::new() - } else { - let span = Span::call_site(); - 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 = - util::replace_suffix(&info_name, &idx).to_snake_case_ident(span); - let comment = make_comment( - register_size, - info.address_offset + (i as u32) * register_size / 8, - &description, - ); - let i = unsuffixed(i as _); - accessors.push(ArrayAccessor { - doc: comment, - name: idx_name, - ty: ty.clone(), - basename: nb_name_cs.clone(), - i, - }); - } - accessors - }; - let array_ty = new_syn_array(ty, array_info.dim); - let syn_field = - new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), array_ty); - register_expanded.push(RegisterBlockField { - syn_field, - description, - offset: info.address_offset, - size: register_size * array_info.dim, - accessors, - }); - } else { - for (field_num, idx) in array_info.indexes().enumerate() { - let nb_name = util::replace_suffix(&info_name, &idx); + match repr { + Repr::Array { .. } => { + // if dimIndex exists, test if it is a sequence of numbers from 0 to dim + let sequential_indexes_from0 = array_info + .indexes_as_range() + .filter(|r| *r.start() == 0) + .is_some(); + + let accessors = if sequential_indexes_from0 { + Vec::new() + } else { + let span = Span::call_site(); + 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 = + util::replace_suffix(&info_name, &idx).to_snake_case_ident(span); + let comment = make_comment( + register_size, + info.address_offset + (i as u32) * register_size / 8, + &description, + ); + let i = unsuffixed(i as _); + accessors.push(ArrayAccessor { + doc: comment, + name: idx_name, + ty: ty.clone(), + basename: nb_name_cs.clone(), + i, + }); + } + accessors + }; + let array_ty = new_syn_array(ty, array_info.dim); let syn_field = - new_syn_field(nb_name.to_snake_case_ident(Span::call_site()), ty.clone()); - + new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), array_ty); register_expanded.push(RegisterBlockField { syn_field, - description: description.clone(), - offset: info.address_offset + field_num as u32 * array_info.dim_increment, - size: register_size, - accessors: Vec::new(), + description, + offset: info.address_offset, + size: register_size * array_info.dim, + accessors, }); } + Repr::Expand { .. } => { + for (field_num, idx) in array_info.indexes().enumerate() { + let nb_name = util::replace_suffix(&info_name, &idx); + let syn_field = new_syn_field( + nb_name.to_snake_case_ident(Span::call_site()), + ty.clone(), + ); + + register_expanded.push(RegisterBlockField { + syn_field, + description: description.clone(), + offset: info.address_offset + + field_num as u32 * array_info.dim_increment, + size: register_size, + accessors: Vec::new(), + }); + } + } + _ => { + bail!( + "register {} is an array, but was given a {}", + register.name, + repr + ); + } } } } @@ -948,13 +1266,15 @@ fn expand_register(register: &Register, config: &Config) -> Result Result { let mut mod_items = TokenStream::new(); + let zipped = ercs.iter_mut().zip(reprs.iter()); - for erc in ercs { + for (erc, repr) in zipped { match erc { // Generate the sub-cluster blocks. RegisterCluster::Cluster(c) => { @@ -969,6 +1289,27 @@ fn render_ercs( // Generate definition for each of the registers. RegisterCluster::Register(reg) => { + // continue if the type is labeled for `NoInfo` + match repr { + Repr::Array { include_info } => { + if !include_info { + continue; + } + } + Repr::Expand { include_info } => { + if !include_info { + continue; + } + } + Repr::Single { + include_info, + common: _, + } => { + if !include_info { + continue; + } + } + } trace!("Register: {}", reg.name); let mut rpath = None; let dpath = reg.derived_from.take(); @@ -980,9 +1321,7 @@ fn render_ercs( let rendered_reg = register::render(reg, path, rpath, index, config).with_context(|| { let descrip = reg.description.as_deref().unwrap_or("No description"); - format!( - "Error rendering register\nName: {reg_name}\nDescription: {descrip}" - ) + format!("Erro rendering register\nName: {reg_name}\nDescription: {descrip}") })?; mod_items.extend(rendered_reg) } @@ -1033,7 +1372,8 @@ fn cluster_block( }) } else { let cpath = path.new_cluster(&c.name); - let mod_items = render_ercs(&mut c.children, &cpath, index, config)?; + let mod_reprs = check_erc_reprs(&c.children, config)?; + let mod_items = render_ercs(&mut c.children, &mod_reprs, &cpath, index, config)?; // Generate the register block. let cluster_size = match c { @@ -1042,8 +1382,13 @@ fn cluster_block( } _ => None, }; - let reg_block = - register_or_cluster_block(&c.children, Some(&mod_name), cluster_size, config)?; + let reg_block = register_or_cluster_block( + &c.children, + &mod_reprs, + Some(&mod_name), + cluster_size, + config, + )?; let mod_items = quote! { #reg_block From 3a49b3259d6819491d3d8353042642cd8a06bfc8 Mon Sep 17 00:00:00 2001 From: n8tlarsen Date: Mon, 10 Oct 2022 18:37:53 -0500 Subject: [PATCH 2/3] Use derives to handle disjoint arrays --- src/generate/peripheral.rs | 845 +++++++++++++++++-------------------- 1 file changed, 380 insertions(+), 465 deletions(-) diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs index f29f5701..a5c9dfaa 100644 --- a/src/generate/peripheral.rs +++ b/src/generate/peripheral.rs @@ -2,7 +2,9 @@ use regex::Regex; use std::borrow::Cow; use std::cmp::Ordering; use std::fmt; -use svd_parser::expand::{derive_cluster, derive_peripheral, derive_register, BlockPath, Index}; +use svd_parser::expand::{ + derive_cluster, derive_peripheral, derive_register, BlockPath, Index, RegisterPath, +}; use crate::svd::{array::names, Cluster, ClusterInfo, Peripheral, Register, RegisterCluster}; use log::{debug, trace, warn}; @@ -18,31 +20,6 @@ use anyhow::{anyhow, bail, Context, Result}; use crate::generate::register; -/// An enum defining how to represent registers in the rendered outputs, binding a bool -/// which describes whether or not renderers should include type information, -/// which allows for disjoint arrays to share the same type. -#[derive(Debug)] -enum Repr { - Array { include_info: bool }, - Expand { include_info: bool }, - // String to store the common type when the erc is part of a disjoint array - Single { include_info: bool, common: String }, -} - -impl Default for Repr { - fn default() -> Self { - Repr::Expand { - include_info: false, - } - } -} - -impl fmt::Display for Repr { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{self:?}") - } -} - pub fn render(p_original: &Peripheral, index: &Index, config: &Config) -> Result { let mut out = TokenStream::new(); @@ -218,20 +195,35 @@ pub fn render(p_original: &Peripheral, index: &Index, config: &Config) -> Result return Ok(TokenStream::new()); } - debug!("Checking array representation information"); - let reprs = check_erc_reprs(&ercs, config)?; + debug!("Checking derivation information"); + let derive_infos = check_erc_derive_infos(&mut ercs, &path, index, config)?; + let zipped = ercs.iter_mut().zip(derive_infos.iter()); + for (mut erc, derive_info) in zipped { + match &mut erc { + &mut RegisterCluster::Register(register) => match derive_info { + DeriveInfo::Implicit(rpath) => { + debug!( + "register {} implicitly derives from {}", + register.name, rpath.name + ); + } + _ => {} + }, + _ => {} + } + } debug!("Pushing cluster & register information into output"); // Push all cluster & register related information into the peripheral module - let mod_items = render_ercs(&mut ercs, &reprs, &path, index, config)?; + let mod_items = render_ercs(&mut ercs, &derive_infos, &path, index, config)?; // Push any register or cluster blocks into the output debug!( "Pushing {} register or cluster blocks into output", ercs.len() ); - let reg_block = register_or_cluster_block(&ercs, &reprs, None, None, config)?; + let reg_block = register_or_cluster_block(&ercs, &derive_infos, None, None, config)?; let open = Punct::new('{', Spacing::Alone); let close = Punct::new('}', Spacing::Alone); @@ -252,6 +244,28 @@ pub fn render(p_original: &Peripheral, index: &Index, config: &Config) -> Result Ok(out) } +/// An enum describing the derivation status of an erc, which allows for disjoint arrays to be +/// implicitly derived from a common type. +#[derive(Debug, PartialEq)] +enum DeriveInfo { + Root, + Explicit(RegisterPath), + Implicit(RegisterPath), + Cluster, // don't do anything different for clusters +} + +impl Default for DeriveInfo { + fn default() -> Self { + DeriveInfo::Root + } +} + +impl fmt::Display for DeriveInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{self:?}") + } +} + #[derive(Clone, Debug)] pub struct ArrayAccessor { pub doc: String, @@ -514,7 +528,7 @@ fn make_comment(size: u32, offset: u32, description: &str) -> String { fn register_or_cluster_block( ercs: &[RegisterCluster], - reprs: &[Repr], + derive_infos: &[DeriveInfo], name: Option<&str>, size: Option, config: &Config, @@ -522,10 +536,10 @@ fn register_or_cluster_block( let mut rbfs = TokenStream::new(); let mut accessors = TokenStream::new(); - let ercs_expanded = expand(ercs, reprs, config) + let ercs_expanded = expand(ercs, derive_infos, config) .with_context(|| "Could not expand register or cluster block")?; - // Locate conflicting regions; we'll need to use unions to represent them. + // Locate conflicting regions; we'll need to use unions to derive_infoesent them. let mut regions = FieldRegions::default(); for reg_block_field in &ercs_expanded { @@ -664,33 +678,33 @@ fn register_or_cluster_block( /// `RegisterBlockField`s containing `Field`s. fn expand( ercs: &[RegisterCluster], - reprs: &[Repr], + derive_infos: &[DeriveInfo], config: &Config, ) -> Result> { let mut ercs_expanded = vec![]; debug!("Expanding registers or clusters into Register Block Fields"); - let zipped = ercs.iter().zip(reprs.iter()); + let zipped = ercs.iter().zip(derive_infos.iter()); - for (erc, repr) in zipped { - match erc { + for (erc, derive_info) in zipped { + match &erc { RegisterCluster::Register(register) => { let reg_name = ®ister.name; - let expanded_reg = expand_register(register, repr, config).with_context(|| { - let descrip = register.description.as_deref().unwrap_or("No description"); - format!("Error expanding register\nName: {reg_name}\nDescription: {descrip}") - })?; + let expanded_reg = + expand_register(register, derive_info, config).with_context(|| { + let descrip = register.description.as_deref().unwrap_or("No description"); + format!( + "Error expanding register\nName: {reg_name}\nDescription: {descrip}" + ) + })?; trace!("Register: {reg_name}"); ercs_expanded.extend(expanded_reg); } RegisterCluster::Cluster(cluster) => { let cluster_name = &cluster.name; - let expanded_cluster = - expand_cluster(cluster, repr, config).with_context(|| { - let descrip = cluster.description.as_deref().unwrap_or("No description"); - format!( - "Error expanding cluster\nName: {cluster_name}\nDescription: {descrip}" - ) - })?; + let expanded_cluster = expand_cluster(cluster, config).with_context(|| { + let descrip = cluster.description.as_deref().unwrap_or("No description"); + format!("Error expanding cluster\nName: {cluster_name}\nDescription: {descrip}") + })?; trace!("Cluster: {cluster_name}"); ercs_expanded.extend(expanded_cluster); } @@ -702,55 +716,55 @@ fn expand( Ok(ercs_expanded) } -/// Gathers type information and decides how ercs (arrays in particular) should be -/// represented in the output. Returns a vector of `Repr` which should -/// be `zip`ped with ercs when rendering. -fn check_erc_reprs(ercs: &[RegisterCluster], config: &Config) -> Result> { - let mut ercs_type_info: Vec<(String, Option, &RegisterCluster, &mut Repr)> = vec![]; - let mut reprs: Vec = vec![]; - // fill the array so we can slice it (without implementing Clone) +/// Searches types using regex to find disjoint arrays which should implicitly derive from +/// another register. Returns a vector of `DeriveInfo` which should be `zip`ped with ercs when rendering. +fn check_erc_derive_infos( + ercs: &mut [RegisterCluster], + path: &BlockPath, + index: &Index, + config: &Config, +) -> Result> { + let mut ercs_type_info: Vec<(String, Option, &RegisterCluster, &mut DeriveInfo)> = + vec![]; + let mut derive_infos: Vec = vec![]; + // fill the array so we can slice it (without implementing clone) for _i in 0..ercs.len() { - reprs.push(Repr::default()); + derive_infos.push(DeriveInfo::default()); } - let reprs_slice = &mut reprs[0..ercs.len()]; - let zipped = ercs.iter().zip(reprs_slice.iter_mut()); - for (erc, repr) in zipped { - match erc { - RegisterCluster::Register(register) => { - let info_name = register.fullname(config.ignore_groups); + let derive_infos_slice = &mut derive_infos[0..ercs.len()]; + let zipped = ercs.iter_mut().zip(derive_infos_slice.iter_mut()); + for (mut erc, derive_info) in zipped { + match &mut erc { + &mut RegisterCluster::Register(register) => { + let info_name = register.fullname(config.ignore_groups).to_string(); + let explicit_rpath = match &mut register.derived_from.clone() { + Some(dpath) => Some(find_root(&dpath, path, index)?), + None => None, + }; match register { Register::Single(_) => { - let mut ty_name = info_name.to_string(); - let mut prev_match = false; - if ercs_type_info.len() > 0 { - // Check this type against previous regexs - for prev in &ercs_type_info { - let (prev_name, prev_regex, prev_erc, _prev_repr) = prev; - if let RegisterCluster::Register(_) = prev_erc { - if let Some(prev_re) = prev_regex { - // if matched adopt the previous type name - if prev_re.is_match(&ty_name) { - ty_name = prev_name.to_string(); - prev_match = true; - debug!( - "Register {} matched to array {}, expanding array", - register.name, prev_name - ); - break; + let ty_name = info_name.to_string(); + *derive_info = match explicit_rpath { + None => { + match regex_against_prev(&ty_name, &ercs_type_info) { + Some(prev_name) => { + // make sure the matched register isn't already deriving from this register + let root = find_root(&prev_name, path, index)?; + if ty_name == root.name { + DeriveInfo::Root + } else { + DeriveInfo::Implicit(root) } } + None => DeriveInfo::Root, } } - } - // don't render info if we matched a previous erc - *repr = Repr::Single { - include_info: !prev_match, - common: ty_name.clone(), + Some(rpath) => DeriveInfo::Explicit(rpath), }; - ercs_type_info.push((ty_name, None, erc, repr)); + ercs_type_info.push((ty_name, None, erc, derive_info)); } - Register::Array(info, array_info) => { + Register::Array(..) => { // Only match integer indeces when searching for disjoint arrays let re_string = util::replace_suffix(&info_name, "([0-9]+|%s)"); let re = Regex::new(format!("^{re_string}$").as_str()).or_else(|_| { @@ -760,193 +774,129 @@ fn check_erc_reprs(ercs: &[RegisterCluster], config: &Config) -> Result 0 { - // Check this type regex against previous names - for prev in &mut ercs_type_info { - let (prev_name, _prev_regex, prev_erc, prev_repr) = prev; - if let RegisterCluster::Register(_) = prev_erc { - // if matched force this cluster and the previous one to expand - if re.is_match(&prev_name) { - debug!( - "Array {} matched to register {}, expanding array", - register.name, prev_name - ); - (**prev_repr, prev_match) = - redo_prev_repr(*prev_repr, &prev_match, &ty_name); + *derive_info = match explicit_rpath { + None => { + match regex_against_prev(&ty_name, &ercs_type_info) { + Some(prev_name) => { + let root = find_root(&prev_name, path, index)?; + DeriveInfo::Implicit(root) } - } - } - } - *repr = if prev_match.is_some() { - Repr::Expand { - // don't duplicate info if another array was matched - include_info: !prev_match.unwrap(), - } - } else { - // Check if the array itself requires expansion - let register_size = register.properties.size.ok_or_else(|| { - anyhow!("Register {} has no `size` field", register.name) - })?; - let sequential_addresses = (array_info.dim == 1) - || (register_size == array_info.dim_increment * BITS_PER_BYTE); - let convert_list = match config.keep_list { - true => match &array_info.dim_name { - Some(dim_name) => dim_name.contains("[%s]"), - None => info.name.contains("[%s]"), - }, - false => true, - }; - let r = if sequential_addresses && convert_list { - Repr::Array { include_info: true } - } else { - Repr::Expand { include_info: true } - }; - r - }; - ercs_type_info.push((ty_name, Some(re), erc, repr)); - } - }; - } - - RegisterCluster::Cluster(cluster) => { - match cluster { - Cluster::Single(_) => { - let mut ty_name = cluster.name.to_string(); - let mut prev_match = false; - if ercs_type_info.len() > 0 { - // Check this type against previous regexs - for prev in &ercs_type_info { - let (prev_name, prev_regex, prev_erc, _prev_repr) = prev; - if let RegisterCluster::Cluster(_) = prev_erc { - if let Some(prev_re) = prev_regex { - // if matched adopt the previous type name - if prev_re.is_match(&ty_name) { - debug!( - "Cluster {} matched to array {}, expanding array", - cluster.name, prev_name - ); - ty_name = prev_name.to_string(); - prev_match = true; - break; + None => { + let mut my_derive_info = DeriveInfo::Root; + // Check this type regex against previous names + for prev in &mut ercs_type_info { + let ( + prev_name, + _prev_regex, + prev_erc, + prev_derive_info, + ) = prev; + if let RegisterCluster::Register(prev_reg) = prev_erc { + if let Register::Array(..) = prev_reg { + // Arrays had a chance to match above + continue; + } + if re.is_match(&prev_name) { + let loop_derive_info = match prev_derive_info { + DeriveInfo::Root => { + let implicit_rpath = + find_root(&ty_name, path, index)?; + **prev_derive_info = + DeriveInfo::Implicit(implicit_rpath); + DeriveInfo::Root + } + DeriveInfo::Explicit(rpath) => { + let implicit_rpath = find_root( + &rpath.name, + path, + index, + )?; + DeriveInfo::Implicit(implicit_rpath) + } + DeriveInfo::Implicit(rpath) => { + DeriveInfo::Implicit(rpath.clone()) + } + DeriveInfo::Cluster => { + return Err(anyhow!( + "register {} derive_infoesented as cluster", + register.name + )) + } + }; + if let DeriveInfo::Root = my_derive_info { + if my_derive_info != loop_derive_info { + my_derive_info = loop_derive_info; + } + } + } + } } + my_derive_info } } } - } - // don't render info if we matched a previous erc - *repr = Repr::Single { - include_info: !prev_match, - common: ty_name.clone(), - }; - ercs_type_info.push((ty_name, None, erc, repr)); - } - - Cluster::Array(info, array_info) => { - // Only match integer indeces when searching for disjoint arrays - let re_string = util::replace_suffix(&cluster.name, "([0-9]+|%s)"); - let re = Regex::new(format!("^{re_string}$").as_str()).or_else(|_| { - Err(anyhow!( - "creating regex for register {} failed", - cluster.name - )) - })?; - let ty_name = cluster.name.to_string(); // keep suffix for regex matching - let mut prev_match = None; - if ercs_type_info.len() > 0 { - // Check this type regex against previous names - for prev in &mut ercs_type_info { - let (prev_name, _prev_regex, prev_erc, prev_repr) = prev; - if let RegisterCluster::Cluster(_) = prev_erc { - // if matched force this cluster and the previous one to expand - if re.is_match(&prev_name) { - debug!( - "array {} matched to cluster {}, expanding array", - cluster.name, prev_name - ); - (**prev_repr, prev_match) = - redo_prev_repr(*prev_repr, &prev_match, &ty_name); - } - } - } - } - *repr = if prev_match.is_some() { - Repr::Expand { - // don't duplicate info if another array was matched - include_info: !prev_match.unwrap(), - } - } else { - // Check if the array itself requires expansion - let cluster_size = cluster_info_size_in_bits(cluster, repr, config) - .with_context(|| { - format!("Can't calculate cluster {} size", cluster.name) - })?; - let increment_bits = array_info.dim_increment * BITS_PER_BYTE; - let sequential_addresses = - (array_info.dim == 1) || (cluster_size == increment_bits); - let convert_list = match config.keep_list { - true => match &array_info.dim_name { - Some(dim_name) => dim_name.contains("[%s]"), - None => info.name.contains("[%s]"), - }, - false => true, - }; - if sequential_addresses && convert_list { - Repr::Array { include_info: true } - } else { - Repr::Expand { include_info: true } - } + Some(rpath) => DeriveInfo::Explicit(rpath), }; - ercs_type_info.push((ty_name, Some(re), erc, repr)) + ercs_type_info.push((ty_name, Some(re), erc, derive_info)); } }; } + &mut RegisterCluster::Cluster(cluster) => { + *derive_info = DeriveInfo::Cluster; + ercs_type_info.push((cluster.name.to_string(), None, erc, derive_info)); + } }; } - Ok(reprs) + Ok(derive_infos) } -/// Called when a previous `Repr`esentation needs to be updated because it matches the regex of a -/// disjoint array. Returns a tuple of the updated `Repr` and match status. -fn redo_prev_repr( - prev_repr: &Repr, - prev_match: &Option, - ty_name: &String, -) -> (Repr, Option) { - match prev_repr { - Repr::Array { .. } => ( - // include info only if there wasn't a previous array match - Repr::Expand { - include_info: if let Some(b) = prev_match { !b } else { true }, - }, - Some(true), // found a match and it is an array - ), - Repr::Expand { .. } => ( - Repr::Expand { - include_info: if let Some(b) = prev_match { !b } else { true }, - }, - Some(true), // found a match and it is an array - ), - Repr::Single { .. } => ( - Repr::Single { - include_info: false, - common: ty_name.clone(), - }, - if prev_match.is_none() { - Some(false) // found a match and it isn't an array - } else { - *prev_match // don't overwrite a previous match when this is a single - }, - ), +fn find_root(dpath: &str, path: &BlockPath, index: &Index) -> Result { + let (dblock, dname) = RegisterPath::parse_str(dpath); + let rdpath; + let reg_path; + let d = (if let Some(dblock) = dblock { + reg_path = dblock.new_register(dname); + rdpath = dblock; + index.registers.get(®_path) + } else { + reg_path = path.new_register(dname); + rdpath = path.clone(); + index.registers.get(®_path) + }) + .ok_or_else(|| anyhow!("register {} not found", dpath))?; + match d.derived_from.as_ref() { + Some(dp) => find_root(dp, &rdpath, index), + None => Ok(reg_path), } } +fn regex_against_prev( + ty_name: &str, + ercs_type_info: &Vec<(String, Option, &RegisterCluster, &mut DeriveInfo)>, +) -> Option { + let mut prev_match = None; + // Check this type name against previous regexs + for prev in ercs_type_info { + let (prev_name, prev_regex, prev_erc, _prev_derive_info) = prev; + if let RegisterCluster::Register(_) = prev_erc { + if let Some(prev_re) = prev_regex { + // if matched adopt the previous type name + if prev_re.is_match(&ty_name) { + prev_match = Some(prev_name.to_string()); + break; + } + } + } + } + prev_match +} + /// Calculate the size of a Cluster. If it is an array, then the dimensions /// tell us the size of the array. Otherwise, inspect the contents using /// [cluster_info_size_in_bits]. -fn cluster_size_in_bits(cluster: &Cluster, repr: &Repr, config: &Config) -> Result { +fn cluster_size_in_bits(cluster: &Cluster, config: &Config) -> Result { match cluster { - Cluster::Single(info) => cluster_info_size_in_bits(info, repr, config), + Cluster::Single(info) => cluster_info_size_in_bits(info, config), // If the contained array cluster has a mismatch between the // dimIncrement and the size of the array items, then the array // will get expanded in expand_cluster below. The overall size @@ -956,7 +906,7 @@ fn cluster_size_in_bits(cluster: &Cluster, repr: &Repr, config: &Config) -> Resu return Ok(0); // Special case! } let last_offset = (dim.dim - 1) * dim.dim_increment * BITS_PER_BYTE; - let last_size = cluster_info_size_in_bits(info, repr, config); + let last_size = cluster_info_size_in_bits(info, config); Ok(last_offset + last_size?) } } @@ -964,13 +914,13 @@ fn cluster_size_in_bits(cluster: &Cluster, repr: &Repr, config: &Config) -> Resu /// Recursively calculate the size of a ClusterInfo. A cluster's size is the /// maximum end position of its recursive children. -fn cluster_info_size_in_bits(info: &ClusterInfo, repr: &Repr, config: &Config) -> Result { +fn cluster_info_size_in_bits(info: &ClusterInfo, config: &Config) -> Result { let mut size = 0; for c in &info.children { let end = match c { RegisterCluster::Register(reg) => { - let reg_size: u32 = expand_register(reg, repr, config)? + let reg_size: u32 = expand_register(reg, &DeriveInfo::Root, config)? .iter() .map(|rbf| rbf.size) .sum(); @@ -978,7 +928,7 @@ fn cluster_info_size_in_bits(info: &ClusterInfo, repr: &Repr, config: &Config) - (reg.address_offset * BITS_PER_BYTE) + reg_size } RegisterCluster::Cluster(clust) => { - (clust.address_offset * BITS_PER_BYTE) + cluster_size_in_bits(clust, repr, config)? + (clust.address_offset * BITS_PER_BYTE) + cluster_size_in_bits(clust, config)? } }; @@ -988,16 +938,10 @@ fn cluster_info_size_in_bits(info: &ClusterInfo, repr: &Repr, config: &Config) - } /// Render a given cluster (and any children) into `RegisterBlockField`s -/// `Option`al bool parameter set to `Some(false)` will force an array to be expanded -/// while `None` or `Some(true)` will follow the conversion check. -fn expand_cluster( - cluster: &Cluster, - repr: &Repr, - config: &Config, -) -> Result> { +fn expand_cluster(cluster: &Cluster, config: &Config) -> Result> { let mut cluster_expanded = vec![]; - let cluster_size = cluster_info_size_in_bits(cluster, repr, config) + let cluster_size = cluster_info_size_in_bits(cluster, config) .with_context(|| format!("Can't calculate cluster {} size", cluster.name))?; let description = cluster .description @@ -1006,19 +950,7 @@ fn expand_cluster( .to_string(); let ty_name = if cluster.is_single() { - match repr { - Repr::Single { - include_info: _, - common, - } => util::replace_suffix(&common, ""), - _ => { - bail!( - "Cluster {} is a single, but was given a {}", - cluster.name, - repr - ); - } - } + cluster.name.to_string() } else { util::replace_suffix(&cluster.name, "") }; @@ -1046,6 +978,7 @@ fn expand_cluster( } 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 @@ -1053,84 +986,79 @@ fn expand_cluster( .filter(|r| *r.start() == 0) .is_some(); - match repr { - Repr::Array { .. } => { - let accessors = if sequential_indexes_from0 { - Vec::new() - } else { - let span = Span::call_site(); - 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 = - util::replace_suffix(&info.name, &idx).to_snake_case_ident(span); - let comment = make_comment( - cluster_size, - info.address_offset + (i as u32) * cluster_size / 8, - &description, - ); - let i = unsuffixed(i as _); - accessors.push(ArrayAccessor { - doc: comment, - name: idx_name, - ty: ty.clone(), - basename: nb_name_cs.clone(), - i, - }); - } - accessors - }; - let array_ty = new_syn_array(ty, array_info.dim); - cluster_expanded.push(RegisterBlockField { - syn_field: new_syn_field( - ty_name.to_snake_case_ident(Span::call_site()), - array_ty, - ), - description, - offset: info.address_offset, - size: cluster_size * array_info.dim, - accessors, - }); - } + let convert_list = match config.keep_list { + true => match &array_info.dim_name { + Some(dim_name) => dim_name.contains("[%s]"), + None => info.name.contains("[%s]"), + }, + false => true, + }; - Repr::Expand { .. } => { - if sequential_indexes_from0 && config.const_generic { - // Include a ZST ArrayProxy giving indexed access to the - // elements. - let ap_path = array_proxy_type(ty, array_info); - let syn_field = - new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ap_path); - cluster_expanded.push(RegisterBlockField { - syn_field, - description: info.description.as_ref().unwrap_or(&info.name).into(), - offset: info.address_offset, - size: 0, - accessors: Vec::new(), + let array_convertible = sequential_addresses && convert_list; + + if array_convertible { + let accessors = if sequential_indexes_from0 { + Vec::new() + } else { + let span = Span::call_site(); + 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 = + util::replace_suffix(&info.name, &idx).to_snake_case_ident(span); + let comment = make_comment( + cluster_size, + info.address_offset + (i as u32) * cluster_size / 8, + &description, + ); + let i = unsuffixed(i as _); + accessors.push(ArrayAccessor { + doc: comment, + name: idx_name, + ty: ty.clone(), + basename: nb_name_cs.clone(), + i, }); - } else { - for (field_num, idx) in array_info.indexes().enumerate() { - let nb_name = util::replace_suffix(&info.name, &idx); - let syn_field = new_syn_field( - nb_name.to_snake_case_ident(Span::call_site()), - ty.clone(), - ); - cluster_expanded.push(RegisterBlockField { - syn_field, - description: description.clone(), - offset: info.address_offset - + field_num as u32 * array_info.dim_increment, - size: cluster_size, - accessors: Vec::new(), - }); - } } - } - _ => { - bail!( - "cluster {} is an array, but was given a {}", - cluster.name, - repr - ); + accessors + }; + let array_ty = new_syn_array(ty, array_info.dim); + cluster_expanded.push(RegisterBlockField { + syn_field: new_syn_field( + ty_name.to_snake_case_ident(Span::call_site()), + array_ty, + ), + description, + offset: info.address_offset, + size: cluster_size * array_info.dim, + accessors, + }); + } else if sequential_indexes_from0 && config.const_generic { + // Include a ZST ArrayProxy giving indexed access to the + // elements. + let ap_path = array_proxy_type(ty, array_info); + let syn_field = + new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ap_path); + cluster_expanded.push(RegisterBlockField { + syn_field, + description: info.description.as_ref().unwrap_or(&info.name).into(), + offset: info.address_offset, + size: 0, + accessors: Vec::new(), + }); + } else { + for (field_num, idx) in array_info.indexes().enumerate() { + let nb_name = util::replace_suffix(&info.name, &idx); + let syn_field = + new_syn_field(nb_name.to_snake_case_ident(Span::call_site()), ty.clone()); + + cluster_expanded.push(RegisterBlockField { + syn_field, + description: description.clone(), + offset: info.address_offset + field_num as u32 * array_info.dim_increment, + size: cluster_size, + accessors: Vec::new(), + }); } } } @@ -1141,11 +1069,10 @@ fn expand_cluster( /// If svd register arrays can't be converted to rust arrays (non sequential addresses, non /// numeral indexes, or not containing all elements from 0 to size) they will be expanded. -/// `Option`al bool parameter set to `Some(false)` will force an array to be expanded -/// while `None` or `Some(true)` will follow the conversion check. +/// A `DeriveInfo::Implicit(_)` will also cause an array to be expanded. fn expand_register( register: &Register, - repr: &Repr, + derive_info: &DeriveInfo, config: &Config, ) -> Result> { let mut register_expanded = vec![]; @@ -1158,19 +1085,7 @@ fn expand_register( let info_name = register.fullname(config.ignore_groups); let ty_name = if register.is_single() { - match repr { - Repr::Single { - include_info: _, - common, - } => util::replace_suffix(&common, ""), - _ => { - bail!( - "register {} is a single, but was given a {}", - register.name, - repr - ); - } - } + info_name.to_string() } else { util::replace_suffix(&info_name, "") }; @@ -1178,7 +1093,7 @@ fn expand_register( match register { Register::Single(info) => { - let syn_field = new_syn_field(info_name.to_snake_case_ident(Span::call_site()), ty); + let syn_field = new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ty); register_expanded.push(RegisterBlockField { syn_field, description, @@ -1188,75 +1103,80 @@ fn expand_register( }) } Register::Array(info, array_info) => { - match repr { - Repr::Array { .. } => { - // if dimIndex exists, test if it is a sequence of numbers from 0 to dim - let sequential_indexes_from0 = array_info - .indexes_as_range() - .filter(|r| *r.start() == 0) - .is_some(); - - let accessors = if sequential_indexes_from0 { - Vec::new() - } else { - let span = Span::call_site(); - 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 = - util::replace_suffix(&info_name, &idx).to_snake_case_ident(span); - let comment = make_comment( - register_size, - info.address_offset + (i as u32) * register_size / 8, - &description, - ); - let i = unsuffixed(i as _); - accessors.push(ArrayAccessor { - doc: comment, - name: idx_name, - ty: ty.clone(), - basename: nb_name_cs.clone(), - i, - }); - } - accessors - }; - let array_ty = new_syn_array(ty, array_info.dim); + let sequential_addresses = (array_info.dim == 1) + || (register_size == array_info.dim_increment * BITS_PER_BYTE); + + let convert_list = match config.keep_list { + true => match &array_info.dim_name { + Some(dim_name) => dim_name.contains("[%s]"), + None => info.name.contains("[%s]"), + }, + false => true, + }; + + // force expansion if we're implicitly deriving so we don't get name collisions + let array_convertible = if let DeriveInfo::Implicit(_) = derive_info { + false + } else { + sequential_addresses && convert_list + }; + + if array_convertible { + // if dimIndex exists, test if it is a sequence of numbers from 0 to dim + let sequential_indexes_from0 = array_info + .indexes_as_range() + .filter(|r| *r.start() == 0) + .is_some(); + + let accessors = if sequential_indexes_from0 { + Vec::new() + } else { + let span = Span::call_site(); + 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 = + util::replace_suffix(&info_name, &idx).to_snake_case_ident(span); + let comment = make_comment( + register_size, + info.address_offset + (i as u32) * register_size / 8, + &description, + ); + let i = unsuffixed(i as _); + accessors.push(ArrayAccessor { + doc: comment, + name: idx_name, + ty: ty.clone(), + basename: nb_name_cs.clone(), + i, + }); + } + accessors + }; + let array_ty = new_syn_array(ty, array_info.dim); + let syn_field = + new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), array_ty); + register_expanded.push(RegisterBlockField { + syn_field, + description, + offset: info.address_offset, + size: register_size * array_info.dim, + accessors, + }); + } else { + for (field_num, idx) in array_info.indexes().enumerate() { + let nb_name = util::replace_suffix(&info_name, &idx); let syn_field = - new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), array_ty); + new_syn_field(nb_name.to_snake_case_ident(Span::call_site()), ty.clone()); + register_expanded.push(RegisterBlockField { syn_field, - description, - offset: info.address_offset, - size: register_size * array_info.dim, - accessors, + description: description.clone(), + offset: info.address_offset + field_num as u32 * array_info.dim_increment, + size: register_size, + accessors: Vec::new(), }); } - Repr::Expand { .. } => { - for (field_num, idx) in array_info.indexes().enumerate() { - let nb_name = util::replace_suffix(&info_name, &idx); - let syn_field = new_syn_field( - nb_name.to_snake_case_ident(Span::call_site()), - ty.clone(), - ); - - register_expanded.push(RegisterBlockField { - syn_field, - description: description.clone(), - offset: info.address_offset - + field_num as u32 * array_info.dim_increment, - size: register_size, - accessors: Vec::new(), - }); - } - } - _ => { - bail!( - "register {} is an array, but was given a {}", - register.name, - repr - ); - } } } } @@ -1266,15 +1186,15 @@ fn expand_register( fn render_ercs( ercs: &mut [RegisterCluster], - reprs: &[Repr], + derive_infos: &[DeriveInfo], path: &BlockPath, index: &Index, config: &Config, ) -> Result { let mut mod_items = TokenStream::new(); - let zipped = ercs.iter_mut().zip(reprs.iter()); - for (erc, repr) in zipped { + let zipped = ercs.iter_mut().zip(derive_infos.iter()); + for (erc, derive_info) in zipped { match erc { // Generate the sub-cluster blocks. RegisterCluster::Cluster(c) => { @@ -1289,40 +1209,35 @@ fn render_ercs( // Generate definition for each of the registers. RegisterCluster::Register(reg) => { - // continue if the type is labeled for `NoInfo` - match repr { - Repr::Array { include_info } => { - if !include_info { - continue; + trace!("Register: {}, DeriveInfo: {}", reg.name, derive_info); + let mut rpath = None; + let before_name = reg.name.to_string(); + if let DeriveInfo::Implicit(rp) = derive_info { + let mut idx_name = None; + let info_name = reg.fullname(config.ignore_groups).to_string(); + if let Register::Array(_, array_info) = reg { + for (_, i) in array_info.indexes().enumerate() { + idx_name = Some(util::replace_suffix(&info_name, &i).to_string()); } } - Repr::Expand { include_info } => { - if !include_info { - continue; - } + if let Some(name) = idx_name { + reg.name = name; } - Repr::Single { - include_info, - common: _, - } => { - if !include_info { - continue; - } + rpath = Some(rp.clone()); + } else { + let dpath = reg.derived_from.take(); + if let Some(dpath) = dpath { + rpath = derive_register(reg, &dpath, path, index)?; } } - trace!("Register: {}", reg.name); - let mut rpath = None; - let dpath = reg.derived_from.take(); - if let Some(dpath) = dpath { - rpath = derive_register(reg, &dpath, path, index)?; - } - let reg_name = ®.name; - let rendered_reg = register::render(reg, path, rpath, index, config).with_context(|| { let descrip = reg.description.as_deref().unwrap_or("No description"); - format!("Erro rendering register\nName: {reg_name}\nDescription: {descrip}") + format!( + "Error rendering register\nName: {before_name}\nDescription: {descrip}" + ) })?; + reg.name = before_name; mod_items.extend(rendered_reg) } } @@ -1372,8 +1287,8 @@ fn cluster_block( }) } else { let cpath = path.new_cluster(&c.name); - let mod_reprs = check_erc_reprs(&c.children, config)?; - let mod_items = render_ercs(&mut c.children, &mod_reprs, &cpath, index, config)?; + let mod_derive_infos = check_erc_derive_infos(&mut c.children, &cpath, index, config)?; + let mod_items = render_ercs(&mut c.children, &mod_derive_infos, &cpath, index, config)?; // Generate the register block. let cluster_size = match c { @@ -1384,7 +1299,7 @@ fn cluster_block( }; let reg_block = register_or_cluster_block( &c.children, - &mod_reprs, + &mod_derive_infos, Some(&mod_name), cluster_size, config, From 874f7df2b634e7fe85388b6416c1370e84802a3f Mon Sep 17 00:00:00 2001 From: n8tlarsen Date: Sat, 5 Nov 2022 14:10:05 -0500 Subject: [PATCH 3/3] Resolve name conflicts when deriving --- CHANGELOG.md | 2 +- src/generate/peripheral.rs | 57 +++++++++++++++++++++----------------- src/generate/register.rs | 22 ++++++++++++--- 3 files changed, 50 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4992308a..ad384a24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Test patched STM32 - simplify ci strategy - Fix generated code for MSP430 atomics +- Add handling for disjoint arrays ## [v0.27.1] - 2022-10-25 @@ -40,7 +41,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Bring documentation on how to generate MSP430 PACs up to date (in line with [msp430_svd](https://github.com/pftbest/msp430_svd)). - Prefix submodule path with self:: when reexporting submodules to avoid ambiguity in crate path. -- Add handling for disjoint arrays ## [v0.25.1] - 2022-08-22 diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs index a5c9dfaa..2f0cc8ef 100644 --- a/src/generate/peripheral.rs +++ b/src/generate/peripheral.rs @@ -539,7 +539,7 @@ fn register_or_cluster_block( let ercs_expanded = expand(ercs, derive_infos, config) .with_context(|| "Could not expand register or cluster block")?; - // Locate conflicting regions; we'll need to use unions to derive_infoesent them. + // Locate conflicting regions; we'll need to use unions to represent them. let mut regions = FieldRegions::default(); for reg_block_field in &ercs_expanded { @@ -1084,15 +1084,15 @@ fn expand_register( let description = register.description.clone().unwrap_or_default(); let info_name = register.fullname(config.ignore_groups); - let ty_name = if register.is_single() { + let mut ty_name = if register.is_single() { info_name.to_string() } else { util::replace_suffix(&info_name, "") }; - let ty = name_to_ty(&ty_name); match register { Register::Single(info) => { + let ty = name_to_ty(&ty_name); let syn_field = new_syn_field(ty_name.to_snake_case_ident(Span::call_site()), ty); register_expanded.push(RegisterBlockField { syn_field, @@ -1114,20 +1114,36 @@ fn expand_register( false => true, }; - // force expansion if we're implicitly deriving so we don't get name collisions - let array_convertible = if let DeriveInfo::Implicit(_) = derive_info { - false + // if dimIndex exists, test if it is a sequence of numbers from 0 to dim + let sequential_indexes_from0 = array_info + .indexes_as_range() + .filter(|r| *r.start() == 0) + .is_some(); + + // force expansion and rename if we're deriving an array that doesnt start at 0 so we don't get name collisions + let index: Cow = if let Some(dim_index) = &array_info.dim_index { + dim_index.first().unwrap().into() } else { - sequential_addresses && convert_list + if sequential_indexes_from0 { + "0".into() + } else { + "".into() + } + }; + let array_convertible = match derive_info { + DeriveInfo::Implicit(_) => { + ty_name = util::replace_suffix(&info_name, &index); + sequential_addresses && convert_list && sequential_indexes_from0 + } + DeriveInfo::Explicit(_) => { + ty_name = util::replace_suffix(&info_name, &index); + sequential_addresses && convert_list && sequential_indexes_from0 + } + _ => sequential_addresses && convert_list, }; + let ty = name_to_ty(&ty_name); if array_convertible { - // if dimIndex exists, test if it is a sequence of numbers from 0 to dim - let sequential_indexes_from0 = array_info - .indexes_as_range() - .filter(|r| *r.start() == 0) - .is_some(); - let accessors = if sequential_indexes_from0 { Vec::new() } else { @@ -1211,18 +1227,7 @@ fn render_ercs( RegisterCluster::Register(reg) => { trace!("Register: {}, DeriveInfo: {}", reg.name, derive_info); let mut rpath = None; - let before_name = reg.name.to_string(); if let DeriveInfo::Implicit(rp) = derive_info { - let mut idx_name = None; - let info_name = reg.fullname(config.ignore_groups).to_string(); - if let Register::Array(_, array_info) = reg { - for (_, i) in array_info.indexes().enumerate() { - idx_name = Some(util::replace_suffix(&info_name, &i).to_string()); - } - } - if let Some(name) = idx_name { - reg.name = name; - } rpath = Some(rp.clone()); } else { let dpath = reg.derived_from.take(); @@ -1230,14 +1235,14 @@ fn render_ercs( rpath = derive_register(reg, &dpath, path, index)?; } } + let reg_name = ®.name; let rendered_reg = register::render(reg, path, rpath, index, config).with_context(|| { let descrip = reg.description.as_deref().unwrap_or("No description"); format!( - "Error rendering register\nName: {before_name}\nDescription: {descrip}" + "Error rendering register\nName: {reg_name}\nDescription: {descrip}" ) })?; - reg.name = before_name; mod_items.extend(rendered_reg) } } diff --git a/src/generate/register.rs b/src/generate/register.rs index a2c16420..60aed796 100644 --- a/src/generate/register.rs +++ b/src/generate/register.rs @@ -1,17 +1,21 @@ use crate::svd::{ - Access, BitRange, EnumeratedValues, Field, ModifiedWriteValues, ReadAction, Register, - RegisterProperties, Usage, WriteConstraint, + Access, BitRange, EnumeratedValues, Field, MaybeArray, ModifiedWriteValues, ReadAction, + Register, RegisterProperties, Usage, WriteConstraint, }; use core::u64; use log::warn; use proc_macro2::{Ident, Punct, Spacing, Span, TokenStream}; use quote::{quote, ToTokens}; +use std::borrow::Cow; use std::collections::HashSet; use svd_parser::expand::{ derive_enumerated_values, derive_field, BlockPath, EnumPath, FieldPath, Index, RegisterPath, }; -use crate::util::{self, ident_to_path, path_segment, type_path, Config, ToSanitizedCase, U32Ext}; +use crate::util::{ + self, ident_to_path, path_segment, replace_suffix, type_path, Config, FullName, + ToSanitizedCase, U32Ext, +}; use anyhow::{anyhow, Result}; use syn::punctuated::Punctuated; @@ -22,7 +26,17 @@ pub fn render( index: &Index, config: &Config, ) -> Result { - let name = util::name_of(register, config.ignore_groups); + let mut name = util::name_of(register, config.ignore_groups); + // Rename if this is a derived array + if dpath.is_some() { + if let MaybeArray::Array(info, array_info) = register { + if let Some(dim_index) = &array_info.dim_index { + let index: Cow = dim_index.first().unwrap().into(); + name = + replace_suffix(&info.fullname(config.ignore_groups), &index.to_string()).into() + } + } + } let span = Span::call_site(); let name_constant_case = name.to_constant_case_ident(span); let name_snake_case = name.to_snake_case_ident(span);