From c88f7c5b64a00a7ca0428c992708c42b01510334 Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@wezfurlong.org>
Date: Thu, 3 Jan 2019 23:24:18 +0000
Subject: [PATCH 1/5] remove dep on untagged unions
This removes any observable side effect of the --nightly switch by
removing the use of unions and the untagged_unions feature gate.
Unions are replaced with accessor functions that return the appropriate
register block reference.
Here's a playground link that shows that the pointer calculation looks
reasonable:
https://play.integer32.com/?version=stable&mode=debug&edition=2018&gist=cd56444abc03e6781e526bbddc7082bc
This commit is a breaking change.
This is based on this WIP PR branch:
https://github.com/rust-embedded/svd2rust/pull/256
It implements the easiest standalone portion of this issue:
https://github.com/rust-embedded/svd2rust/issues/218
and also makes accessing the unions "safe" too, which is requested here:
https://github.com/rust-embedded/svd2rust/issues/230
---
src/generate/device.rs | 6 --
src/generate/peripheral.rs | 119 ++++++++++++-------------------------
2 files changed, 37 insertions(+), 88 deletions(-)
diff --git a/src/generate/device.rs b/src/generate/device.rs
index bcb0be12..35a5bfe2 100644
--- a/src/generate/device.rs
+++ b/src/generate/device.rs
@@ -48,12 +48,6 @@ pub fn render(
#![no_std]
});
- if nightly {
- out.push(quote! {
- #![feature(untagged_unions)]
- });
- }
-
match target {
Target::CortexM => {
out.push(quote! {
diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
index 1867cbeb..47657d6f 100644
--- a/src/generate/peripheral.rs
+++ b/src/generate/peripheral.rs
@@ -358,85 +358,10 @@ fn register_or_cluster_block(
ercs: &[RegisterCluster],
defs: &Defaults,
name: Option<&str>,
- nightly: bool,
-) -> Result<Tokens> {
- if nightly {
- register_or_cluster_block_nightly(ercs, defs, name)
- } else {
- register_or_cluster_block_stable(ercs, defs, name)
- }
-}
-
-fn register_or_cluster_block_stable(
- ercs: &[RegisterCluster],
- defs: &Defaults,
- name: Option<&str>,
-) -> Result<Tokens> {
- let mut fields = Tokens::new();
- // enumeration of reserved fields
- let mut i = 0;
- // offset from the base address, in bytes
- let mut offset = 0;
-
- let ercs_expanded = expand(ercs, defs, name)?;
-
- for reg_block_field in ercs_expanded {
- let pad = if let Some(pad) = reg_block_field.offset.checked_sub(offset) {
- pad
- } else {
- warn!(
- "{:?} overlaps with another register block at offset {}. \
- Ignoring.",
- reg_block_field.field.ident, reg_block_field.offset
- );
- continue;
- };
-
- if pad != 0 {
- let name = Ident::new(format!("_reserved{}", i));
- let pad = pad as usize;
- fields.append(quote! {
- #name : [u8; #pad],
- });
- i += 1;
- }
-
- let comment = &format!(
- "0x{:02x} - {}",
- reg_block_field.offset,
- util::escape_brackets(util::respace(®_block_field.description).as_ref()),
- )[..];
-
- fields.append(quote! {
- #[doc = #comment]
- });
-
- reg_block_field.field.to_tokens(&mut fields);
- Ident::new(",").to_tokens(&mut fields);
-
- offset = reg_block_field.offset + reg_block_field.size / BITS_PER_BYTE;
- }
-
- let name = Ident::new(match name {
- Some(name) => name.to_sanitized_upper_case(),
- None => "RegisterBlock".into(),
- });
-
- Ok(quote! {
- /// Register block
- #[repr(C)]
- pub struct #name {
- #fields
- }
- })
-}
-
-fn register_or_cluster_block_nightly(
- ercs: &[RegisterCluster],
- defs: &Defaults,
- name: Option<&str>,
+ _nightly: bool,
) -> Result<Tokens> {
let mut fields = Tokens::new();
+ let mut accessors = Tokens::new();
let mut helper_types = Tokens::new();
let ercs_expanded = expand(ercs, defs, name)?;
@@ -484,15 +409,39 @@ fn register_or_cluster_block_nightly(
util::escape_brackets(util::respace(®_block_field.description).as_ref()),
)[..];
+ if block_is_union {
+ let name = ®_block_field.field.ident;
+ let mut_name = Ident::new(format!("{}_mut", name.as_ref().unwrap()));
+ let ty = ®_block_field.field.ty;
+ let offset = reg_block_field.offset;
+ accessors.append(quote! {
+ pub fn #name(&self) -> &#ty {
+ unsafe {
+ &*(((self as *const Self) as *const u8).add(#offset) as *const #ty)
+ }
+ }
+ pub fn #mut_name(&self) -> &mut #ty {
+ unsafe {
+ &mut *(((self as *const Self) as *mut u8).add(#offset) as *mut #ty)
+ }
+ }
+ });
+
+ } else {
+
region_fields.append(quote! {
#[doc = #comment]
});
reg_block_field.field.to_tokens(&mut region_fields);
Ident::new(",").to_tokens(&mut region_fields);
+ }
}
- if region.fields.len() > 1 && !block_is_union {
+ if block_is_union {
+ continue
+ }
+ if region.fields.len() > 1 {
let (type_name, name) = match region.ident.clone() {
Some(prefix) => (
Ident::new(format!("{}_UNION", prefix.to_sanitized_upper_case())),
@@ -531,19 +480,25 @@ fn register_or_cluster_block_nightly(
None => "RegisterBlock".into(),
});
- let block_type = if block_is_union {
- Ident::new("union")
+ let accessors = if block_is_union {
+ quote! {
+ impl #name {
+ #accessors
+ }
+ }
} else {
- Ident::new("struct")
+ quote! {}
};
Ok(quote! {
/// Register block
#[repr(C)]
- pub #block_type #name {
+ pub struct #name {
#fields
}
+ #accessors
+
#helper_types
})
}
From 225f51a19fb0c41ae09ea14744f5704509cb49d5 Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@wezfurlong.org>
Date: Thu, 3 Jan 2019 23:54:38 +0000
Subject: [PATCH 2/5] add missing doc comments, use usize for the offsets
---
src/generate/peripheral.rs | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
index 47657d6f..649c1ca6 100644
--- a/src/generate/peripheral.rs
+++ b/src/generate/peripheral.rs
@@ -413,13 +413,16 @@ fn register_or_cluster_block(
let name = ®_block_field.field.ident;
let mut_name = Ident::new(format!("{}_mut", name.as_ref().unwrap()));
let ty = ®_block_field.field.ty;
- let offset = reg_block_field.offset;
+ let offset = reg_block_field.offset as usize;
accessors.append(quote! {
+ #[doc = #comment]
pub fn #name(&self) -> &#ty {
unsafe {
&*(((self as *const Self) as *const u8).add(#offset) as *const #ty)
}
}
+
+ #[doc = #comment]
pub fn #mut_name(&self) -> &mut #ty {
unsafe {
&mut *(((self as *const Self) as *mut u8).add(#offset) as *mut #ty)
From 445a238d2c1a4840d7c105be8294b938fad1016f Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@wezfurlong.org>
Date: Fri, 4 Jan 2019 00:34:43 +0000
Subject: [PATCH 3/5] simplify union removal, and remove all traces of unions
---
src/generate/peripheral.rs | 81 +++++++-------------------------------
1 file changed, 14 insertions(+), 67 deletions(-)
diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
index 649c1ca6..e5fe7930 100644
--- a/src/generate/peripheral.rs
+++ b/src/generate/peripheral.rs
@@ -213,24 +213,9 @@ impl Region {
self.shortest_ident()
}
}
- /// Return a description of this region
- fn description(&self) -> String {
- let mut result = String::new();
- for f in &self.fields {
- // In the Atmel SVDs the union variants all tend to
- // have the same description. Rather than emitting
- // the same text three times over, only join in the
- // text from the other variants if it is different.
- // This isn't a foolproof way of emitting the most
- // reasonable short description, but it's good enough.
- if f.description != result {
- if !result.is_empty() {
- result.push(' ');
- }
- result.push_str(&f.description);
- }
- }
- result
+
+ fn is_union(&self) -> bool {
+ self.fields.len() > 1
}
}
@@ -318,10 +303,6 @@ impl FieldRegions {
Ok(())
}
- pub fn is_union(&self) -> bool {
- self.regions.len() == 1 && self.regions[0].fields.len() > 1
- }
-
/// Resolves type name conflicts
pub fn resolve_idents(&mut self) -> Result<()> {
let idents: Vec<_> = {
@@ -362,7 +343,7 @@ fn register_or_cluster_block(
) -> Result<Tokens> {
let mut fields = Tokens::new();
let mut accessors = Tokens::new();
- let mut helper_types = Tokens::new();
+ let mut have_accessors = false;
let ercs_expanded = expand(ercs, defs, name)?;
@@ -373,7 +354,6 @@ fn register_or_cluster_block(
regions.add(reg_block_field)?;
}
- let block_is_union = regions.is_union();
// We need to compute the idents of each register/union block first to make sure no conflicts exists.
regions.resolve_idents()?;
// The end of the region from the prior iteration of the loop
@@ -393,6 +373,7 @@ fn register_or_cluster_block(
last_end = region.end;
let mut region_fields = Tokens::new();
+ let is_region_a_union = region.is_union();
for reg_block_field in ®ion.fields {
if reg_block_field.offset != region.offset {
@@ -409,11 +390,12 @@ fn register_or_cluster_block(
util::escape_brackets(util::respace(®_block_field.description).as_ref()),
)[..];
- if block_is_union {
+ if is_region_a_union {
let name = ®_block_field.field.ident;
let mut_name = Ident::new(format!("{}_mut", name.as_ref().unwrap()));
let ty = ®_block_field.field.ty;
let offset = reg_block_field.offset as usize;
+ have_accessors = true;
accessors.append(quote! {
#[doc = #comment]
pub fn #name(&self) -> &#ty {
@@ -431,49 +413,16 @@ fn register_or_cluster_block(
});
} else {
+ region_fields.append(quote! {
+ #[doc = #comment]
+ });
- region_fields.append(quote! {
- #[doc = #comment]
- });
-
- reg_block_field.field.to_tokens(&mut region_fields);
- Ident::new(",").to_tokens(&mut region_fields);
+ reg_block_field.field.to_tokens(&mut region_fields);
+ Ident::new(",").to_tokens(&mut region_fields);
}
}
- if block_is_union {
- continue
- }
- if region.fields.len() > 1 {
- let (type_name, name) = match region.ident.clone() {
- Some(prefix) => (
- Ident::new(format!("{}_UNION", prefix.to_sanitized_upper_case())),
- Ident::new(prefix),
- ),
- // If we can't find a name, fall back to the region index as a
- // unique-within-this-block identifier counter.
- None => {
- let ident = Ident::new(format!("U{}", i));
- (ident.clone(), ident)
- }
- };
-
- let description = region.description();
-
- helper_types.append(quote! {
- #[doc = #description]
- #[repr(C)]
- pub union #type_name {
- #region_fields
- }
- });
-
- fields.append(quote! {
- #[doc = #description]
- pub #name: #type_name
- });
- Ident::new(",").to_tokens(&mut fields);
- } else {
+ if !is_region_a_union {
fields.append(®ion_fields);
}
}
@@ -483,7 +432,7 @@ fn register_or_cluster_block(
None => "RegisterBlock".into(),
});
- let accessors = if block_is_union {
+ let accessors = if have_accessors {
quote! {
impl #name {
#accessors
@@ -501,8 +450,6 @@ fn register_or_cluster_block(
}
#accessors
-
- #helper_types
})
}
From 2d578ba8908a1b17513116ea60668195fe3fbfca Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@wezfurlong.org>
Date: Fri, 4 Jan 2019 10:08:40 +0000
Subject: [PATCH 4/5] fixup padding of fields emitted after a union!
---
src/generate/peripheral.rs | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
index e5fe7930..4b2f9617 100644
--- a/src/generate/peripheral.rs
+++ b/src/generate/peripheral.rs
@@ -356,7 +356,7 @@ fn register_or_cluster_block(
// We need to compute the idents of each register/union block first to make sure no conflicts exists.
regions.resolve_idents()?;
- // The end of the region from the prior iteration of the loop
+ // The end of the region for which we previously emitted a field into `fields`
let mut last_end = 0;
for (i, region) in regions.regions.iter().enumerate() {
@@ -370,20 +370,10 @@ fn register_or_cluster_block(
});
}
- last_end = region.end;
-
let mut region_fields = Tokens::new();
let is_region_a_union = region.is_union();
for reg_block_field in ®ion.fields {
- if reg_block_field.offset != region.offset {
- // TODO: need to emit padding for this case.
- // Happens for freescale_mkl43z4
- warn!(
- "field {:?} has different offset {} than its union container {}",
- reg_block_field.field.ident, reg_block_field.offset, region.offset
- );
- }
let comment = &format!(
"0x{:02x} - {}",
reg_block_field.offset,
@@ -411,7 +401,6 @@ fn register_or_cluster_block(
}
}
});
-
} else {
region_fields.append(quote! {
#[doc = #comment]
@@ -424,7 +413,26 @@ fn register_or_cluster_block(
if !is_region_a_union {
fields.append(®ion_fields);
+ } else {
+ // Emit padding for the items that we're not emitting
+ // as fields so that subsequent fields have the correct
+ // alignment in the struct. We could omit this and just
+ // not updated `last_end`, so that the padding check in
+ // the outer loop kicks in, but it is nice to be able to
+ // see that the padding is attributed to a union when
+ // visually inspecting the alignment in the struct.
+ //
+ // Include the computed ident for the union in the padding
+ // name, along with the region number, falling back to
+ // the offset and end in case we couldn't figure out a
+ // nice identifier.
+ let name = Ident::new(format!("_reserved_{}_{}", i, region.compute_ident().unwrap_or_else(|| format!("{}_{}", region.offset, region.end))));
+ let pad = (region.end - region.offset) as usize;
+ fields.append(quote! {
+ #name: [u8; #pad],
+ })
}
+ last_end = region.end;
}
let name = Ident::new(match name {
From 36a661ebcf7d953ae06fcda25ae27fa29db84751 Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@wezfurlong.org>
Date: Fri, 4 Jan 2019 10:10:50 +0000
Subject: [PATCH 5/5] inline the union accessors
---
src/generate/peripheral.rs | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/generate/peripheral.rs b/src/generate/peripheral.rs
index 4b2f9617..27b97393 100644
--- a/src/generate/peripheral.rs
+++ b/src/generate/peripheral.rs
@@ -388,6 +388,7 @@ fn register_or_cluster_block(
have_accessors = true;
accessors.append(quote! {
#[doc = #comment]
+ #[inline(always)]
pub fn #name(&self) -> &#ty {
unsafe {
&*(((self as *const Self) as *const u8).add(#offset) as *const #ty)
@@ -395,6 +396,7 @@ fn register_or_cluster_block(
}
#[doc = #comment]
+ #[inline(always)]
pub fn #mut_name(&self) -> &mut #ty {
unsafe {
&mut *(((self as *const Self) as *mut u8).add(#offset) as *mut #ty)