Merge pull request #871 from rust-embedded/clippy182

clippy
rust-embedded · Oct 19, 2024 · 29bcfd8 · 29bcfd8
2 parents f4a317e + 8aadb82
commit 29bcfd8
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+- Add warning about indexing register arrays
+- Skip generating `.add(0)` and `1 *` in accessors
 - Bump MSRV of generated code to 1.76
 - move `must_use` from methods to generic type
 

diff --git a/src/generate/interrupt.rs b/src/generate/interrupt.rs
@@ -62,10 +62,10 @@ pub fn render(
             interrupt
                 .0
                 .description
-                .as_ref()
-                .map(|s| util::respace(s))
-                .as_ref()
-                .map(|s| util::escape_special_chars(s))
+                .as_deref()
+                .map(util::respace)
+                .as_deref()
+                .map(util::escape_special_chars)
                 .unwrap_or_else(|| interrupt.0.name.clone())
         );
 

diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
@@ -305,7 +305,7 @@ impl Region {
         let mut idents: Vec<_> = self
             .rbfs
             .iter()
-            .filter_map(|f| f.syn_field.ident.as_ref().map(|ident| ident.to_string()))
+            .filter_map(|f| f.syn_field.ident.as_ref().map(ToString::to_string))
             .collect();
         if idents.is_empty() {
             return None;
@@ -343,7 +343,7 @@ impl Region {
         let idents: Vec<_> = self
             .rbfs
             .iter()
-            .filter_map(|f| f.syn_field.ident.as_ref().map(|ident| ident.to_string()))
+            .filter_map(|f| f.syn_field.ident.as_ref().map(ToString::to_string))
             .collect();
 
         if idents.is_empty() {
@@ -726,7 +726,7 @@ fn check_erc_derive_infos(
                 };
                 match register {
                     Register::Single(_) => {
-                        let ty_name = info_name.to_string();
+                        let ty_name = info_name.clone();
                         *derive_info = match explicit_rpath {
                             None => {
                                 match compare_this_against_prev(
@@ -758,7 +758,7 @@ fn check_erc_derive_infos(
                         let re = Regex::new(format!("^{re_string}$").as_str()).map_err(|_| {
                             anyhow!("Error creating regex for register {}", register.name)
                         })?;
-                        let ty_name = info_name.to_string(); // keep suffix for regex matching
+                        let ty_name = info_name.clone(); // keep suffix for regex matching
                         *derive_info = match explicit_rpath {
                             None => {
                                 match compare_this_against_prev(
@@ -787,7 +787,7 @@ fn check_erc_derive_infos(
             }
             RegisterCluster::Cluster(cluster) => {
                 *derive_info = DeriveInfo::Cluster;
-                ercs_type_info.push((cluster.name.to_string(), None, erc, derive_info));
+                ercs_type_info.push((cluster.name.clone(), None, erc, derive_info));
             }
         };
     }
@@ -998,7 +998,7 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                 doc,
                 name,
                 ty,
-                offset: unsuffixed(info.address_offset),
+                offset: info.address_offset,
             })
             .raw_if(false);
             cluster_expanded.push(RegisterBlockField {
@@ -1052,14 +1052,19 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                     &description,
                 );
                 let mut accessors = Vec::with_capacity((array_info.dim + 1) as _);
+                let first_name = svd::array::names(info, array_info).next().unwrap();
+                let note = (array_info.indexes().next().unwrap() != "0").then(||
+                    format!("<div class=\"warning\">`n` is the index of {0} in the array. `n == 0` corresponds to `{first_name}` {0}.</div>", "cluster")
+                );
                 accessors.push(
                     Accessor::Array(ArrayAccessor {
                         doc,
                         name: accessor_name.clone(),
                         ty: ty.clone(),
-                        offset: unsuffixed(info.address_offset),
-                        dim: unsuffixed(array_info.dim),
-                        increment: unsuffixed(array_info.dim_increment),
+                        offset: info.address_offset,
+                        dim: array_info.dim,
+                        increment: array_info.dim_increment,
+                        note,
                     })
                     .raw_if(!array_convertible),
                 );
@@ -1071,7 +1076,6 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                             ci.address_offset,
                             ci.description.as_deref().unwrap_or(&ci.name),
                         );
-                        let i = unsuffixed(i as u64);
                         accessors.push(
                             Accessor::ArrayElem(ArrayElemAccessor {
                                 doc,
@@ -1114,7 +1118,7 @@ fn expand_cluster(cluster: &Cluster, config: &Config) -> Result<Vec<RegisterBloc
                         doc,
                         name,
                         ty: ty.clone(),
-                        offset: unsuffixed(info.address_offset),
+                        offset: info.address_offset,
                     })
                     .raw_if(false);
                     cluster_expanded.push(RegisterBlockField {
@@ -1166,7 +1170,7 @@ fn expand_register(
                 doc,
                 name,
                 ty,
-                offset: unsuffixed(info.address_offset),
+                offset: info.address_offset,
             })
             .raw_if(false);
             register_expanded.push(RegisterBlockField {
@@ -1235,14 +1239,19 @@ fn expand_register(
                     &description,
                 );
                 let mut accessors = Vec::with_capacity((array_info.dim + 1) as _);
+                let first_name = svd::array::names(info, array_info).next().unwrap();
+                let note = (array_info.indexes().next().unwrap() != "0").then(||
+                    format!("<div class=\"warning\">`n` is the index of {0} in the array. `n == 0` corresponds to `{first_name}` {0}.</div>", "register")
+                );
                 accessors.push(
                     Accessor::Array(ArrayAccessor {
                         doc,
                         name: accessor_name.clone(),
                         ty: ty.clone(),
-                        offset: unsuffixed(info.address_offset),
-                        dim: unsuffixed(array_info.dim),
-                        increment: unsuffixed(array_info.dim_increment),
+                        offset: info.address_offset,
+                        dim: array_info.dim,
+                        increment: array_info.dim_increment,
+                        note,
                     })
                     .raw_if(!array_convertible),
                 );
@@ -1259,7 +1268,6 @@ fn expand_register(
                             ri.address_offset,
                             ri.description.as_deref().unwrap_or(&ri.name),
                         );
-                        let i = unsuffixed(i as u64);
                         accessors.push(
                             Accessor::ArrayElem(ArrayElemAccessor {
                                 doc,
@@ -1302,7 +1310,7 @@ fn expand_register(
                         doc,
                         name,
                         ty: ty.clone(),
-                        offset: unsuffixed(info.address_offset),
+                        offset: info.address_offset,
                     })
                     .raw_if(false);
                     register_expanded.push(RegisterBlockField {

diff --git a/src/generate/peripheral/accessor.rs b/src/generate/peripheral/accessor.rs
@@ -1,6 +1,8 @@
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::{quote, ToTokens};
 
+use crate::util::unsuffixed;
+
 #[derive(Clone, Debug)]
 pub enum Accessor {
     Reg(RegAccessor),
@@ -51,18 +53,24 @@ impl ToTokens for AccessType {
                 ty,
                 offset,
             })) => {
+                let offset = (*offset != 0).then(|| unsuffixed(*offset)).map(|o| quote!(.add(#o)));
                 quote! {
                     #[doc = #doc]
                     #[inline(always)]
                     pub const fn #name(&self) -> &#ty {
-                        unsafe { &*core::ptr::from_ref(self).cast::<u8>().add(#offset).cast() }
+                        unsafe { &*core::ptr::from_ref(self).cast::<u8>() #offset .cast() }
                     }
                 }
             }
-            Self::Ref(Accessor::Array(ArrayAccessor { doc, name, ty, .. })) => {
+            Self::Ref(Accessor::Array(ArrayAccessor { doc, name, ty, note, .. })) => {
                 let name_iter = Ident::new(&format!("{name}_iter"), Span::call_site());
+                let note = note.as_ref().map(|note| quote! {
+                    #[doc = ""]
+                    #[doc = #note]
+                });
                 quote! {
                     #[doc = #doc]
+                    #note
                     #[inline(always)]
                     pub const fn #name(&self, n: usize) -> &#ty {
                         &self.#name[n]
@@ -82,11 +90,20 @@ impl ToTokens for AccessType {
                 offset,
                 dim,
                 increment,
+                note,
             })) => {
                 let name_iter = Ident::new(&format!("{name}_iter"), Span::call_site());
-                let cast = quote! { unsafe { &*core::ptr::from_ref(self).cast::<u8>().add(#offset).add(#increment * n).cast() } };
+                let offset = (*offset != 0).then(|| unsuffixed(*offset)).map(|o| quote!(.add(#o)));
+                let dim = unsuffixed(*dim);
+                let increment = (*increment != 1).then(|| unsuffixed(*increment)).map(|i| quote!(#i *));
+                let note = note.as_ref().map(|note| quote! {
+                    #[doc = ""]
+                    #[doc = #note]
+                });
+                let cast = quote! { unsafe { &*core::ptr::from_ref(self).cast::<u8>() #offset .add(#increment n).cast() } };
                 quote! {
                     #[doc = #doc]
+                    #note
                     #[inline(always)]
                     pub const fn #name(&self, n: usize) -> &#ty {
                         #[allow(clippy::no_effect)]
@@ -109,6 +126,7 @@ impl ToTokens for AccessType {
                     basename,
                     i,
                 } = elem;
+                let i = unsuffixed(*i as u64);
                 quote! {
                     #[doc = #doc]
                     #[inline(always)]
@@ -127,17 +145,18 @@ pub struct RegAccessor {
     pub doc: String,
     pub name: Ident,
     pub ty: syn::Type,
-    pub offset: syn::LitInt,
+    pub offset: u32,
 }
 
 #[derive(Clone, Debug)]
 pub struct ArrayAccessor {
     pub doc: String,
     pub name: Ident,
     pub ty: syn::Type,
-    pub offset: syn::LitInt,
-    pub dim: syn::LitInt,
-    pub increment: syn::LitInt,
+    pub offset: u32,
+    pub dim: u32,
+    pub increment: u32,
+    pub note: Option<String>,
 }
 
 #[derive(Clone, Debug)]
@@ -146,5 +165,5 @@ pub struct ArrayElemAccessor {
     pub name: Ident,
     pub ty: syn::Type,
     pub basename: Ident,
-    pub i: syn::LitInt,
+    pub i: usize,
 }
diff --git a/src/generate/register.rs b/src/generate/register.rs
@@ -3,7 +3,6 @@ use crate::svd::{
     ModifiedWriteValues, ReadAction, Register, RegisterProperties, Usage, WriteConstraint,
     WriteConstraintRange,
 };
-use core::u64;
 use log::warn;
 use proc_macro2::{Delimiter, Group, Ident, Span, TokenStream};
 use quote::quote;
@@ -117,7 +116,7 @@ pub fn render(
                 register.properties.reset_value.is_some(),
                 &mod_ty,
                 false,
-                &register,
+                register,
                 &rpath,
                 config,
             )?,
@@ -162,6 +161,7 @@ fn read_action_docs(can_read: bool, read_action: Option<ReadAction>) -> String {
     doc
 }
 
+#[allow(clippy::too_many_arguments)]
 fn api_docs(
     can_read: bool,
     can_write: bool,
@@ -222,7 +222,7 @@ fn api_docs(
             let idx = format!("[{idx}]");
             rpath.name.replace("[%s]", &idx).replace("%s", &idx)
         } else {
-            rpath.name.to_string()
+            rpath.name.clone()
         };
         // TODO: support html_urls for registers in cluster
         if rpath.block.path.is_empty() {

diff --git a/src/lib.rs b/src/lib.rs
@@ -37,7 +37,7 @@
 //!
 //! - `build.rs`, build script that places `device.x` somewhere the linker can find.
 //! - `device.x`, linker script that weakly aliases all the interrupt handlers to the default
-//! exception handler (`DefaultHandler`).
+//!   exception handler (`DefaultHandler`).
 //! - `lib.rs`, the generated code.
 //!
 //! All these files must be included in the same device crate. The `lib.rs` file contains several
@@ -95,7 +95,7 @@
 //!
 //! - `build.rs`, build script that places `device.x` somewhere the linker can find.
 //! - `device.x`, linker script that weakly aliases all the interrupt handlers to the default
-//! exception handler (`DefaultHandler`).
+//!   exception handler (`DefaultHandler`).
 //! - `lib.rs`, the generated code.
 //!
 //! All these files must be included in the same device crate. The `lib.rs` file contains several