Skip to content

Commit 0a1d8f6

Browse files
committed
Auto merge of #84 - whitequark:single-bit-bool-fields, r=japaric
Generate `bool` accessors for single-bit fields This makes using such fields significantly more ergonomic, as previously they would have been `u8`s, leading to unnecessary conversion and silent truncation in downstream code. The `bool` writers are never unsafe because it is meaningless to have a writable field that only admits one (or worse, zero) valid values.
2 parents 2c34e26 + 3405452 commit 0a1d8f6

File tree

3 files changed

+93
-25
lines changed

3 files changed

+93
-25
lines changed

src/generate.rs

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,12 @@ fn unsafety(write_constraint: Option<&WriteConstraint>, width: u32) -> Option<Id
402402
// any value that can fit in the field
403403
None
404404
}
405+
None if width == 1 => {
406+
// the field is one bit wide, so we assume it's legal to write
407+
// either value into it or it wouldn't exist; despite that
408+
// if a writeConstraint exists then respect it
409+
None
410+
}
405411
_ => Some(Ident::new("unsafe"))
406412
}
407413
}
@@ -635,6 +641,7 @@ pub fn fields(
635641
pc_r: Ident,
636642
pc_w: Ident,
637643
sc: Ident,
644+
bits: Ident,
638645
ty: Ident,
639646
width: u32,
640647
write_constraint: Option<&'a WriteConstraint>,
@@ -649,6 +656,11 @@ pub fn fields(
649656
let pc_w = Ident::new(&*format!("{}W", pc));
650657
let _pc_w = Ident::new(&*format!("_{}W", pc));
651658
let _sc = Ident::new(&*format!("_{}", sc));
659+
let bits = if width == 1 {
660+
Ident::new("bit")
661+
} else {
662+
Ident::new("bits")
663+
};
652664
let mut description = if width == 1 {
653665
format!("Bit {}", offset)
654666
} else {
@@ -665,11 +677,12 @@ pub fn fields(
665677
description: description,
666678
pc_r: pc_r,
667679
pc_w: pc_w,
680+
bits: bits,
668681
width: width,
669682
access: f.access,
670683
evs: &f.enumerated_values,
671684
sc: Ident::new(&*sc),
672-
mask: util::unsuffixed((1 << width) - 1),
685+
mask: util::unsuffixed_or_bool((1 << width) - 1, width),
673686
name: &f.name,
674687
offset: util::unsuffixed(u64(f.bit_range.offset)),
675688
ty: width.to_ty()?,
@@ -688,14 +701,20 @@ pub fn fields(
688701
continue;
689702
}
690703

704+
let bits = &f.bits;
691705
let mask = &f.mask;
692706
let offset = &f.offset;
693707
let fty = &f.ty;
694-
let bits = quote! {
708+
let cast = if f.width == 1 {
709+
quote! { != 0 }
710+
} else {
711+
quote! { as #fty }
712+
};
713+
let value = quote! {
695714
const MASK: #fty = #mask;
696715
const OFFSET: u8 = #offset;
697716

698-
((self.bits >> OFFSET) & MASK as #rty) as #fty
717+
((self.bits >> OFFSET) & MASK as #rty) #cast
699718
};
700719

701720
if let Some((evs, base)) =
@@ -775,7 +794,7 @@ pub fn fields(
775794
#[doc = #description]
776795
#[inline(always)]
777796
pub fn #sc(&self) -> #pc_r {
778-
#pc_r::_from({ #bits })
797+
#pc_r::_from({ #value })
779798
}
780799
},
781800
);
@@ -821,7 +840,7 @@ pub fn fields(
821840
.iter()
822841
.map(
823842
|v| {
824-
let value = util::unsuffixed(v.value);
843+
let value = util::unsuffixed_or_bool(v.value, f.width);
825844
let pc = &v.pc;
826845

827846
quote! {
@@ -841,7 +860,7 @@ pub fn fields(
841860
quote! {
842861
/// Value of the field as raw bits
843862
#[inline(always)]
844-
pub fn bits(&self) -> #fty {
863+
pub fn #bits(&self) -> #fty {
845864
match *self {
846865
#(#arms),*
847866
}
@@ -853,7 +872,7 @@ pub fn fields(
853872
.iter()
854873
.map(
855874
|v| {
856-
let i = util::unsuffixed(v.value);
875+
let i = util::unsuffixed_or_bool(v.value, f.width);
857876
let pc = &v.pc;
858877

859878
quote! {
@@ -869,7 +888,7 @@ pub fn fields(
869888
i => #pc_r::_Reserved(i)
870889
},
871890
);
872-
} else {
891+
} else if 1 << f.width.to_ty_width()? != variants.len() {
873892
arms.push(
874893
quote! {
875894
_ => unreachable!()
@@ -882,8 +901,8 @@ pub fn fields(
882901
#[allow(missing_docs)]
883902
#[doc(hidden)]
884903
#[inline(always)]
885-
pub fn _from(bits: #fty) -> #pc_r {
886-
match bits {
904+
pub fn _from(value: #fty) -> #pc_r {
905+
match value {
887906
#(#arms),*,
888907
}
889908
}
@@ -932,7 +951,7 @@ pub fn fields(
932951
#[doc = #description]
933952
#[inline(always)]
934953
pub fn #sc(&self) -> #pc_r {
935-
let bits = { # bits };
954+
let bits = { #value };
936955
#pc_r { bits }
937956
}
938957
},
@@ -948,7 +967,7 @@ pub fn fields(
948967
impl #pc_r {
949968
/// Value of the field as raw bits
950969
#[inline(always)]
951-
pub fn bits(&self) -> #fty {
970+
pub fn #bits(&self) -> #fty {
952971
self.bits
953972
}
954973
}
@@ -968,9 +987,11 @@ pub fn fields(
968987
let mut proxy_items = vec![];
969988

970989
let mut unsafety = unsafety(f.write_constraint, f.width);
990+
let bits = &f.bits;
971991
let fty = &f.ty;
972992
let offset = &f.offset;
973993
let mask = &f.mask;
994+
let width = f.width;
974995

975996
if let Some((evs, base)) =
976997
util::lookup(
@@ -1078,7 +1099,7 @@ pub fn fields(
10781099
.map(
10791100
|v| {
10801101
let pc = &v.pc;
1081-
let value = util::unsuffixed(v.value);
1102+
let value = util::unsuffixed_or_bool(v.value, f.width);
10821103

10831104
quote! {
10841105
#pc_w::#pc => #value
@@ -1109,7 +1130,7 @@ pub fn fields(
11091130
#[inline(always)]
11101131
pub fn variant(self, variant: #pc_w) -> &'a mut W {
11111132
#unsafety {
1112-
self.bits(variant._bits())
1133+
self.#bits(variant._bits())
11131134
}
11141135
}
11151136
},
@@ -1142,18 +1163,32 @@ pub fn fields(
11421163
);
11431164
}
11441165
}
1166+
} else if width == 1 {
1167+
proxy_items.push(
1168+
quote! {
1169+
/// Sets the field bit
1170+
pub fn set(self) -> &'a mut W {
1171+
self.bit(true)
1172+
}
1173+
1174+
/// Clears the field bit
1175+
pub fn clear(self) -> &'a mut W {
1176+
self.bit(false)
1177+
}
1178+
}
1179+
);
11451180
}
11461181

11471182
proxy_items.push(
11481183
quote! {
11491184
/// Writes raw bits to the field
11501185
#[inline(always)]
1151-
pub #unsafety fn bits(self, bits: #fty) -> &'a mut W {
1186+
pub #unsafety fn #bits(self, value: #fty) -> &'a mut W {
11521187
const MASK: #fty = #mask;
11531188
const OFFSET: u8 = #offset;
11541189

11551190
self.w.bits &= !((MASK as #rty) << OFFSET);
1156-
self.w.bits |= ((bits & MASK) as #rty) << OFFSET;
1191+
self.w.bits |= ((value & MASK) as #rty) << OFFSET;
11571192
self.w
11581193
}
11591194
},

src/lib.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@
183183
//!
184184
//! ``` rust
185185
//! // is the SADD0 bit of the CR2 register set?
186-
//! if i2c1.c2r.read().sadd0().bits() == 1 {
186+
//! if i2c1.c2r.read().sadd0().bit() {
187187
//! // yes
188188
//! } else {
189189
//! // no
@@ -225,7 +225,7 @@
225225
//! // Starting from the reset value, `0x0000_0000`, change the bitfields SADD0
226226
//! // and SADD1 to `1` and `0b0011110` respectively and write that to the
227227
//! // register CR2.
228-
//! i2c1.cr2.write(|w| unsafe { w.sadd0().bits(1).sadd1().bits(0b0011110) });
228+
//! i2c1.cr2.write(|w| unsafe { w.sadd0().bit(true).sadd1().bits(0b0011110) });
229229
//! // NOTE ^ unsafe because you could be writing a reserved bit pattern into
230230
//! // the register. In this case, the SVD doesn't provide enough information to
231231
//! // check whether that's the case.
@@ -246,10 +246,10 @@
246246
//!
247247
//! ``` rust
248248
//! // Set the START bit to 1 while KEEPING the state of the other bits intact
249-
//! i2c1.cr2.modify(|_, w| unsafe { w.start().bits(1) });
249+
//! i2c1.cr2.modify(|_, w| unsafe { w.start().bit(true) });
250250
//!
251251
//! // TOGGLE the STOP bit, all the other bits will remain untouched
252-
//! i2c1.cr2.modify(|r, w| w.stop().bits(r.stop().bits() ^ 1));
252+
//! i2c1.cr2.modify(|r, w| w.stop().bit(!r.stop().bit()));
253253
//! ```
254254
//!
255255
//! # enumeratedValues
@@ -314,12 +314,12 @@
314314
//! gpioa.dir.write(|w| w.pin0().output());
315315
//! ```
316316
//!
317-
//! The `bits` method is still available but will become safe if it's impossible
318-
//! to write a reserved bit pattern into the register
317+
//! The `bits` (or `bit`) method is still available but will become safe if it's
318+
//! impossible to write a reserved bit pattern into the register:
319319
//!
320320
//! ```
321321
//! // safe because there are only two options: `0` or `1`
322-
//! gpioa.dir.write(|w| w.pin0().bits(1));
322+
//! gpioa.dir.write(|w| w.pin0().bit(true));
323323
//! ```
324324
//!
325325
//! # Interrupt API

src/util.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,18 @@ pub fn unsuffixed(n: u64) -> Lit {
235235
Lit::Int(n, IntTy::Unsuffixed)
236236
}
237237

238+
pub fn unsuffixed_or_bool(n: u64, width: u32) -> Lit {
239+
if width == 1 {
240+
if n == 0 {
241+
Lit::Bool(false)
242+
} else {
243+
Lit::Bool(true)
244+
}
245+
} else {
246+
unsuffixed(n)
247+
}
248+
}
249+
238250
#[derive(Clone, Debug)]
239251
pub struct Base<'a> {
240252
pub register: Option<&'a str>,
@@ -427,24 +439,45 @@ fn lookup_in_register<'r>
427439

428440
pub trait U32Ext {
429441
fn to_ty(&self) -> Result<Ident>;
442+
fn to_ty_width(&self) -> Result<u32>;
430443
}
431444

432445
impl U32Ext for u32 {
433446
fn to_ty(&self) -> Result<Ident> {
434447
Ok(
435448
match *self {
436-
1...8 => Ident::new("u8"),
449+
1 => Ident::new("bool"),
450+
2...8 => Ident::new("u8"),
437451
9...16 => Ident::new("u16"),
438452
17...32 => Ident::new("u32"),
439453
_ => {
440454
Err(
441455
format!(
442-
"can't convert {} bits into a Rust integer type",
456+
"can't convert {} bits into a Rust integral type",
443457
*self
444458
),
445459
)?
446460
}
447461
},
448462
)
449463
}
464+
465+
fn to_ty_width(&self) -> Result<u32> {
466+
Ok(
467+
match *self {
468+
1 => 1,
469+
2...8 => 8,
470+
9...16 => 16,
471+
17...32 => 32,
472+
_ => {
473+
Err(
474+
format!(
475+
"can't convert {} bits into a Rust integral type width",
476+
*self
477+
),
478+
)?
479+
}
480+
}
481+
)
482+
}
450483
}

0 commit comments

Comments
 (0)