diff --git a/CHANGELOG.md b/CHANGELOG.md index 1133619f..927f4f02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleases](https://github.com/quartiq/miniconf/compare/v0.17.2...HEAD) - DATE + +### Changed + +* `KeyLookup`, `Walk`, and `Metadata` layout and behavior + ## [0.17.2](https://github.com/quartiq/miniconf/compare/v0.17.1...v0.17.2) - 2024-11-19 ### Added diff --git a/miniconf/examples/scpi.rs b/miniconf/examples/scpi.rs index 589ebd51..f6315957 100644 --- a/miniconf/examples/scpi.rs +++ b/miniconf/examples/scpi.rs @@ -21,37 +21,40 @@ struct ScpiKey(T); impl + ?Sized> Key for ScpiKey { fn find(&self, lookup: &KeyLookup) -> Result { let s = self.0.as_ref(); - if let Some(names) = lookup.names { - let mut truncated = None; - let mut ambiguous = false; - for (i, name) in names.iter().enumerate() { - if name.len() < s.len() - || !name - .chars() - .zip(s.chars()) - .all(|(n, s)| n.to_ascii_lowercase() == s.to_ascii_lowercase()) - { - continue; + match lookup { + KeyLookup::Named(names) => { + let mut truncated = None; + let mut ambiguous = false; + for (i, name) in names.iter().enumerate() { + if name.len() < s.len() + || !name + .chars() + .zip(s.chars()) + .all(|(n, s)| n.to_ascii_lowercase() == s.to_ascii_lowercase()) + { + continue; + } + if name.len() == s.len() { + // Exact match: return immediately + return Ok(i); + } + if truncated.is_some() { + // Multiple truncated matches: ambiguous unless there is an additional exact match + ambiguous = true; + } else { + // First truncated match: fine if there is only one. + truncated = Some(i); + } } - if name.len() == s.len() { - // Exact match: return immediately - return Ok(i); - } - if truncated.is_some() { - // Multiple truncated matches: ambiguous unless there is an additional exact match - ambiguous = true; + if ambiguous { + None } else { - // First truncated match: fine if there is only one. - truncated = Some(i); + truncated } } - if ambiguous { - None - } else { - truncated + KeyLookup::Numbered(len) | KeyLookup::Homogeneous(len) => { + s.parse().ok().filter(|i| *i < len.get()) } - } else { - s.parse().ok() } .ok_or(Traversal::NotFound(1)) } diff --git a/miniconf/src/impls.rs b/miniconf/src/impls.rs index f669b157..d372a26e 100644 --- a/miniconf/src/impls.rs +++ b/miniconf/src/impls.rs @@ -15,10 +15,7 @@ macro_rules! impl_tuple { #[allow(unreachable_code, unused_mut, unused)] impl<$($t: TreeKey),+> TreeKey for ($($t,)+) { fn traverse_all() -> Result { - let k = KeyLookup::homogeneous($n); - let mut walk = W::internal(); - $(walk = walk.merge(&$t::traverse_all()?, Some($i), &k)?;)+ - Ok(walk) + W::internal(&[$(&$t::traverse_all()?, )+], &KeyLookup::numbered($n)) } fn traverse_by_key(mut keys: K, mut func: F) -> Result> @@ -26,9 +23,9 @@ macro_rules! impl_tuple { K: Keys, F: FnMut(usize, Option<&'static str>, NonZero) -> Result<(), E>, { - let k = KeyLookup::homogeneous($n); + let k = KeyLookup::numbered($n); let index = keys.next(&k)?; - func(index, None, k.len).map_err(|err| Error::Inner(1, err))?; + func(index, None, k.len()).map_err(|err| Error::Inner(1, err))?; Error::increment_result(match index { $($i => $t::traverse_by_key(keys, func),)+ _ => unreachable!() @@ -43,7 +40,7 @@ macro_rules! impl_tuple { K: Keys, S: Serializer, { - let index = keys.next(&KeyLookup::homogeneous($n))?; + let index = keys.next(&KeyLookup::numbered($n))?; Error::increment_result(match index { $($i => self.$i.serialize_by_key(keys, ser),)+ _ => unreachable!() @@ -58,7 +55,7 @@ macro_rules! impl_tuple { K: Keys, D: Deserializer<'de>, { - let index = keys.next(&KeyLookup::homogeneous($n))?; + let index = keys.next(&KeyLookup::numbered($n))?; Error::increment_result(match index { $($i => self.$i.deserialize_by_key(keys, de),)+ _ => unreachable!() @@ -72,24 +69,22 @@ macro_rules! impl_tuple { where K: Keys, { - let index = keys.next(&KeyLookup::homogeneous($n))?; - let ret: Result<_, _> = match index { + let index = keys.next(&KeyLookup::numbered($n))?; + match index { $($i => self.$i.ref_any_by_key(keys),)+ _ => unreachable!() - }; - ret.map_err(Traversal::increment) + }.map_err(Traversal::increment) } fn mut_any_by_key(&mut self, mut keys: K) -> Result<&mut dyn Any, Traversal> where K: Keys, { - let index = keys.next(&KeyLookup::homogeneous($n))?; - let ret: Result<_, _> = match index { + let index = keys.next(&KeyLookup::numbered($n))?; + match index { $($i => self.$i.mut_any_by_key(keys),)+ _ => unreachable!() - }; - ret.map_err(Traversal::increment) + }.map_err(Traversal::increment) } } } @@ -114,7 +109,7 @@ impl Assert { impl TreeKey for [T; N] { fn traverse_all() -> Result { let () = Assert::::GREATER; // internal nodes must have at least one leaf - W::internal().merge(&T::traverse_all()?, None, &KeyLookup::homogeneous(N)) + W::internal(&[&T::traverse_all()?], &KeyLookup::homogeneous(N)) } fn traverse_by_key(mut keys: K, mut func: F) -> Result> @@ -125,7 +120,7 @@ impl TreeKey for [T; N] { let () = Assert::::GREATER; // internal nodes must have at least one leaf let k = KeyLookup::homogeneous(N); let index = keys.next(&k)?; - func(index, None, k.len).map_err(|err| Error::Inner(1, err))?; + func(index, None, k.len()).map_err(|err| Error::Inner(1, err))?; Error::increment_result(T::traverse_by_key(keys, func)) } } @@ -242,17 +237,12 @@ impl TreeAny for Option { ///////////////////////////////////////////////////////////////////////////////////////// -const RESULT_LOOKUP: KeyLookup = KeyLookup { - len: NonZero::::MIN.saturating_add(1), // 2 - names: Some(&["Ok", "Err"]), -}; +const RESULT_LOOKUP: KeyLookup = KeyLookup::Named(&["Ok", "Err"]); impl TreeKey for Result { #[inline] fn traverse_all() -> Result { - W::internal() - .merge(&T::traverse_all()?, Some(0), &RESULT_LOOKUP)? - .merge(&E::traverse_all()?, Some(1), &RESULT_LOOKUP) + W::internal(&[&T::traverse_all()?, &E::traverse_all()?], &RESULT_LOOKUP) } #[inline] @@ -276,9 +266,9 @@ impl TreeSerialize for Result { K: Keys, S: Serializer, { - Error::increment_result(match (keys.next(&RESULT_LOOKUP)?, self) { - (0, Ok(value)) => value.serialize_by_key(keys, ser), - (1, Err(value)) => value.serialize_by_key(keys, ser), + Error::increment_result(match (self, keys.next(&RESULT_LOOKUP)?) { + (Ok(value), 0) => value.serialize_by_key(keys, ser), + (Err(value), 1) => value.serialize_by_key(keys, ser), _ => Err(Traversal::Absent(0).into()), }) } @@ -291,9 +281,9 @@ impl<'de, T: TreeDeserialize<'de>, E: TreeDeserialize<'de>> TreeDeserialize<'de> K: Keys, D: Deserializer<'de>, { - Error::increment_result(match (keys.next(&RESULT_LOOKUP)?, self) { - (0, Ok(value)) => value.deserialize_by_key(keys, de), - (1, Err(value)) => value.deserialize_by_key(keys, de), + Error::increment_result(match (self, keys.next(&RESULT_LOOKUP)?) { + (Ok(value), 0) => value.deserialize_by_key(keys, de), + (Err(value), 1) => value.deserialize_by_key(keys, de), _ => Err(Traversal::Absent(0).into()), }) } @@ -305,9 +295,9 @@ impl TreeAny for Result { where K: Keys, { - match (keys.next(&RESULT_LOOKUP)?, self) { - (0, Ok(value)) => value.ref_any_by_key(keys), - (1, Err(value)) => value.ref_any_by_key(keys), + match (self, keys.next(&RESULT_LOOKUP)?) { + (Ok(value), 0) => value.ref_any_by_key(keys), + (Err(value), 1) => value.ref_any_by_key(keys), _ => Err(Traversal::Absent(0)), } .map_err(Traversal::increment) @@ -318,9 +308,9 @@ impl TreeAny for Result { where K: Keys, { - match (keys.next(&RESULT_LOOKUP)?, self) { - (0, Ok(value)) => value.mut_any_by_key(keys), - (1, Err(value)) => value.mut_any_by_key(keys), + match (self, keys.next(&RESULT_LOOKUP)?) { + (Ok(value), 0) => value.mut_any_by_key(keys), + (Err(value), 1) => value.mut_any_by_key(keys), _ => Err(Traversal::Absent(0)), } .map_err(Traversal::increment) @@ -329,18 +319,12 @@ impl TreeAny for Result { ///////////////////////////////////////////////////////////////////////////////////////// -const BOUND_LOOKUP: KeyLookup = KeyLookup { - len: NonZero::::MIN.saturating_add(1), - names: Some(&["Included", "Excluded"]), -}; +const BOUND_LOOKUP: KeyLookup = KeyLookup::Named(&["Included", "Excluded"]); impl TreeKey for Bound { #[inline] fn traverse_all() -> Result { - let t = T::traverse_all()?; - W::internal() - .merge(&t, Some(0), &BOUND_LOOKUP)? - .merge(&t, Some(1), &BOUND_LOOKUP) + W::internal(&[&T::traverse_all()?; 2], &BOUND_LOOKUP) } #[inline] @@ -363,8 +347,8 @@ impl TreeSerialize for Bound { K: Keys, S: Serializer, { - Error::increment_result(match (keys.next(&BOUND_LOOKUP)?, self) { - (0, Self::Included(value)) | (1, Self::Excluded(value)) => { + Error::increment_result(match (self, keys.next(&BOUND_LOOKUP)?) { + (Self::Included(value), 0) | (Self::Excluded(value), 1) => { value.serialize_by_key(keys, ser) } _ => Err(Traversal::Absent(0).into()), @@ -379,8 +363,8 @@ impl<'de, T: TreeDeserialize<'de>> TreeDeserialize<'de> for Bound { K: Keys, D: Deserializer<'de>, { - Error::increment_result(match (keys.next(&BOUND_LOOKUP)?, self) { - (0, Self::Included(value)) | (1, Self::Excluded(value)) => { + Error::increment_result(match (self, keys.next(&BOUND_LOOKUP)?) { + (Self::Included(value), 0) | (Self::Excluded(value), 1) => { value.deserialize_by_key(keys, de) } _ => Err(Traversal::Absent(0).into()), @@ -394,8 +378,8 @@ impl TreeAny for Bound { where K: Keys, { - match (keys.next(&BOUND_LOOKUP)?, self) { - (0, Self::Included(value)) | (1, Self::Excluded(value)) => value.ref_any_by_key(keys), + match (self, keys.next(&BOUND_LOOKUP)?) { + (Self::Included(value), 0) | (Self::Excluded(value), 1) => value.ref_any_by_key(keys), _ => Err(Traversal::Absent(0)), } .map_err(Traversal::increment) @@ -406,8 +390,8 @@ impl TreeAny for Bound { where K: Keys, { - match (keys.next(&BOUND_LOOKUP)?, self) { - (0, Self::Included(value)) | (1, Self::Excluded(value)) => value.mut_any_by_key(keys), + match (self, keys.next(&BOUND_LOOKUP)?) { + (Self::Included(value), 0) | (Self::Excluded(value), 1) => value.mut_any_by_key(keys), _ => Err(Traversal::Absent(0)), } .map_err(Traversal::increment) @@ -416,18 +400,12 @@ impl TreeAny for Bound { ///////////////////////////////////////////////////////////////////////////////////////// -const RANGE_LOOKUP: KeyLookup = KeyLookup { - len: NonZero::::MIN.saturating_add(1), - names: Some(&["start", "end"]), -}; +const RANGE_LOOKUP: KeyLookup = KeyLookup::Named(&["start", "end"]); impl TreeKey for Range { #[inline] fn traverse_all() -> Result { - let t = T::traverse_all()?; - W::internal() - .merge(&t, Some(0), &RANGE_LOOKUP)? - .merge(&t, Some(1), &RANGE_LOOKUP) + W::internal(&[&T::traverse_all()?; 2], &RANGE_LOOKUP) } #[inline] @@ -506,10 +484,7 @@ impl TreeAny for Range { impl TreeKey for RangeInclusive { #[inline] fn traverse_all() -> Result { - let t = T::traverse_all()?; - W::internal() - .merge(&t, Some(0), &RANGE_LOOKUP)? - .merge(&t, Some(1), &RANGE_LOOKUP) + W::internal(&[&T::traverse_all()?; 2], &RANGE_LOOKUP) } #[inline] @@ -542,15 +517,12 @@ impl TreeSerialize for RangeInclusive { ///////////////////////////////////////////////////////////////////////////////////////// -const RANGE_FROM_LOOKUP: KeyLookup = KeyLookup { - len: NonZero::::MIN, - names: Some(&["start"]), -}; +const RANGE_FROM_LOOKUP: KeyLookup = KeyLookup::Named(&["start"]); impl TreeKey for RangeFrom { #[inline] fn traverse_all() -> Result { - W::internal().merge(&T::traverse_all()?, Some(0), &RANGE_FROM_LOOKUP) + W::internal(&[&T::traverse_all()?], &RANGE_FROM_LOOKUP) } #[inline] @@ -622,15 +594,12 @@ impl TreeAny for RangeFrom { ///////////////////////////////////////////////////////////////////////////////////////// -const RANGE_TO_LOOKUP: KeyLookup = KeyLookup { - len: NonZero::::MIN, - names: Some(&["end"]), -}; +const RANGE_TO_LOOKUP: KeyLookup = KeyLookup::Named(&["end"]); impl TreeKey for RangeTo { #[inline] fn traverse_all() -> Result { - W::internal().merge(&T::traverse_all()?, Some(0), &RANGE_TO_LOOKUP) + W::internal(&[&T::traverse_all()?], &RANGE_TO_LOOKUP) } #[inline] diff --git a/miniconf/src/iter.rs b/miniconf/src/iter.rs index f3509385..4d4093a1 100644 --- a/miniconf/src/iter.rs +++ b/miniconf/src/iter.rs @@ -48,7 +48,7 @@ impl core::iter::FusedIterator for ExactSize {} // unsafe impl core::iter::TrustedLen for ExactSize {} /// A Keys wrapper that can always finalize() -struct Consume(T); +pub(crate) struct Consume(pub(crate) T); impl Keys for Consume { #[inline] fn next(&mut self, lookup: &KeyLookup) -> Result { @@ -125,7 +125,7 @@ impl NodeIter { assert_eq!(self.depth, D + 1, "NodeIter partially consumed"); assert_eq!(self.root, 0, "NodeIter on sub-tree"); debug_assert_eq!(&self.state, &[0; D]); // ensured by depth = D + 1 marker and contract - let meta: Metadata = M::traverse_all().unwrap(); + let meta: Metadata = M::traverse_all().unwrap(); // Note(unwrap): infallible assert!( D >= meta.max_depth, "depth D = {D} must be at least {}", @@ -133,7 +133,7 @@ impl NodeIter { ); ExactSize { iter: self, - count: meta.count, + count: meta.count.get(), } } @@ -167,7 +167,7 @@ where // Not initial state: increment self.state[self.depth - 1] += 1; } - return match M::transcode(Consume(self.state.into_keys())) { + return match M::transcode(Consume(self.state.iter().into_keys())) { Err(Traversal::NotFound(depth)) => { // Reset index at current depth, then retry with incremented index at depth - 1 or terminate // Key lookup was performed and failed: depth is always >= 1 diff --git a/miniconf/src/key.rs b/miniconf/src/key.rs index f1a5d081..5a8aa06a 100644 --- a/miniconf/src/key.rs +++ b/miniconf/src/key.rs @@ -1,21 +1,20 @@ use core::{iter::Fuse, num::NonZero}; +use serde::Serialize; + use crate::Traversal; /// Data to look up field names and convert to indices /// /// This struct used together with [`crate::TreeKey`]. -pub struct KeyLookup { - /// The number of top-level nodes. - /// - /// This is used by `impl Keys for Packed`. - pub len: NonZero, - - /// Node names, if any. - /// - /// If nodes have names, this is a slice of them. - /// If it is `Some`, it's `.len()` is guaranteed to be `LEN`. - pub names: Option<&'static [&'static str]>, +#[derive(Clone, Debug, PartialEq, PartialOrd, Hash, Serialize)] +pub enum KeyLookup { + /// Named children + Named(&'static [&'static str]), + /// Numbered heterogeneous children + Numbered(NonZero), + /// Homogeneous numbered children + Homogeneous(NonZero), } impl KeyLookup { @@ -23,24 +22,42 @@ impl KeyLookup { #[inline] pub const fn homogeneous(len: usize) -> Self { match NonZero::new(len) { - Some(len) => Self { len, names: None }, - None => panic!("Internal nodes must have at least one leaf"), + Some(len) => Self::Homogeneous(len), + None => panic!("Must have at least one child"), + } + } + + /// Return a homogenenous unnamed KeyLookup + #[inline] + pub const fn numbered(len: usize) -> Self { + match NonZero::new(len) { + Some(len) => Self::Numbered(len), + None => panic!("Must have at least one child"), + } + } + + /// Return the number of elements in the lookup + #[inline] + pub const fn len(&self) -> NonZero { + match self { + Self::Named(names) => match NonZero::new(names.len()) { + Some(len) => len, + None => panic!("Must have at least one child"), + }, + Self::Numbered(len) | Self::Homogeneous(len) => *len, } } /// Perform a index-to-name lookup #[inline] pub fn lookup(&self, index: usize) -> Result, Traversal> { - match self.names { - Some(names) => { - debug_assert!(names.len() == self.len.get()); - match names.get(index) { - Some(name) => Ok(Some(name)), - None => Err(Traversal::NotFound(1)), - } - } - None => { - if index >= self.len.get() { + match self { + Self::Named(names) => match names.get(index) { + Some(name) => Ok(Some(name)), + None => Err(Traversal::NotFound(1)), + }, + Self::Numbered(len) | Self::Homogeneous(len) => { + if index >= len.get() { Err(Traversal::NotFound(1)) } else { Ok(None) @@ -48,16 +65,6 @@ impl KeyLookup { } } } - - /// Check that index is within range - #[inline] - pub fn clamp(&self, index: usize) -> Result { - if index >= self.len.get() { - Err(Traversal::NotFound(1)) - } else { - Ok(index) - } - } } /// Convert a `&str` key into a node index on a `KeyLookup` @@ -92,8 +99,11 @@ macro_rules! impl_key_integer { impl Key for $t { #[inline] fn find(&self, lookup: &KeyLookup) -> Result { - let index = (*self).try_into().or(Err(Traversal::NotFound(1)))?; - lookup.clamp(index) + (*self) + .try_into() + .ok() + .filter(|i| *i < lookup.len().get()) + .ok_or(Traversal::NotFound(1)) } } )+}; @@ -104,12 +114,13 @@ impl_key_integer!(usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128); impl Key for str { #[inline] fn find(&self, lookup: &KeyLookup) -> Result { - let index = match lookup.names { - Some(names) => names.iter().position(|n| *n == self), - None => self.parse().ok(), + match lookup { + KeyLookup::Named(names) => names.iter().position(|n| *n == self), + KeyLookup::Homogeneous(len) | KeyLookup::Numbered(len) => { + self.parse().ok().filter(|i| *i < len.get()) + } } - .ok_or(Traversal::NotFound(1))?; - lookup.clamp(index) + .ok_or(Traversal::NotFound(1)) } } diff --git a/miniconf/src/packed.rs b/miniconf/src/packed.rs index 7e0b127c..1b4de975 100644 --- a/miniconf/src/packed.rs +++ b/miniconf/src/packed.rs @@ -244,7 +244,7 @@ impl Packed { impl Keys for Packed { #[inline] fn next(&mut self, lookup: &KeyLookup) -> Result { - let bits = Self::bits_for(lookup.len.get() - 1); + let bits = Self::bits_for(lookup.len().get() - 1); let index = self.pop_msb(bits).ok_or(Traversal::TooShort(0))?; index.find(lookup) } diff --git a/miniconf/src/tree.rs b/miniconf/src/tree.rs index ef769947..edda1c8c 100644 --- a/miniconf/src/tree.rs +++ b/miniconf/src/tree.rs @@ -162,7 +162,7 @@ pub trait TreeKey { /// bar: [Leaf; 2], /// }; /// let m: Metadata = S::traverse_all().unwrap(); - /// assert_eq!((m.max_depth, m.max_length, m.count), (2, 4, 3)); + /// assert_eq!((m.max_depth, m.max_length, m.count.get()), (2, 4, 3)); /// ``` fn traverse_all() -> Result; diff --git a/miniconf/src/walk.rs b/miniconf/src/walk.rs index b30f79e4..c09d9681 100644 --- a/miniconf/src/walk.rs +++ b/miniconf/src/walk.rs @@ -1,10 +1,12 @@ +use core::num::NonZero; + use crate::{KeyLookup, Packed}; /// Metadata about a `TreeKey` namespace. /// /// Metadata includes paths that may be [`crate::Traversal::Absent`] at runtime. #[non_exhaustive] -#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct Metadata { /// The maximum length of a path in bytes. /// @@ -20,7 +22,7 @@ pub struct Metadata { pub max_depth: usize, /// The exact total number of keys. - pub count: usize, + pub count: NonZero, /// The maximum number of bits (see [`crate::Packed`]) pub max_bits: u32, @@ -42,75 +44,62 @@ pub trait Walk: Sized { /// Error type for `merge()` type Error; - /// Return the walk starting point for an an empty internal node - fn internal() -> Self; - /// Return the walk starting point for a single leaf node fn leaf() -> Self; /// Merge node metadata into self. /// /// # Args - /// * `walk`: The walk of the node to merge. - /// * `index`: Either the node index in case of a single node - /// or `None`, in case of `lookup.len` nodes of homogeneous type. + /// * `children`: The walk of the children to merge. /// * `lookup`: The namespace the node(s) are in. - fn merge( - self, - walk: &Self, - index: Option, - lookup: &KeyLookup, - ) -> Result; + fn internal(children: &[&Self], lookup: &KeyLookup) -> Result; } impl Walk for Metadata { type Error = core::convert::Infallible; - #[inline] - fn internal() -> Self { - Default::default() - } - #[inline] fn leaf() -> Self { Self { - count: 1, - ..Default::default() + count: NonZero::::MIN, + max_length: 0, + max_depth: 0, + max_bits: 0, } } #[inline] - fn merge( - mut self, - meta: &Self, - index: Option, - lookup: &KeyLookup, - ) -> Result { - let (ident_len, count) = match index { - None => ( - // homogeneous [meta; len] - match lookup.names { - Some(names) => names.iter().map(|n| n.len()).max().unwrap_or_default(), - None => lookup.len.ilog10() as usize + 1, - }, - lookup.len.get(), - ), - Some(index) => ( - // one meta at index - match lookup.names { - Some(names) => names[index].len(), - None => index.checked_ilog10().unwrap_or_default() as usize + 1, - }, - 1, - ), - }; - self.max_depth = self.max_depth.max(1 + meta.max_depth); - self.max_length = self.max_length.max(ident_len + meta.max_length); - debug_assert_ne!(meta.count, 0); - self.count += count * meta.count; - self.max_bits = self - .max_bits - .max(Packed::bits_for(lookup.len.get() - 1) + meta.max_bits); - Ok(self) + fn internal(children: &[&Self], lookup: &KeyLookup) -> Result { + let mut max_depth = 0; + let mut max_length = 0; + let mut count = 0; + let mut max_bits = 0; + // TODO: swap loop and match + for (index, child) in children.iter().enumerate() { + let (len, n) = match lookup { + KeyLookup::Named(names) => { + debug_assert_eq!(children.len(), names.len()); + (names[index].len(), 1) + } + KeyLookup::Numbered(len) => { + debug_assert_eq!(children.len(), len.get()); + (index.checked_ilog10().unwrap_or_default() as usize + 1, 1) + } + KeyLookup::Homogeneous(len) => { + debug_assert_eq!(children.len(), 1); + (len.ilog10() as usize + 1, len.get()) + } + }; + max_depth = max_depth.max(1 + child.max_depth); + max_length = max_length.max(len + child.max_length); + count += n * child.count.get(); + max_bits = max_bits.max(Packed::bits_for(lookup.len().get() - 1) + child.max_bits); + } + Ok(Self { + max_bits, + max_depth, + max_length, + count: NonZero::new(count).unwrap(), + }) } } diff --git a/miniconf/tests/arrays.rs b/miniconf/tests/arrays.rs index 66f45c0b..b0a67378 100644 --- a/miniconf/tests/arrays.rs +++ b/miniconf/tests/arrays.rs @@ -132,5 +132,5 @@ fn metadata() { let m: Metadata = Settings::traverse_all().unwrap(); assert_eq!(m.max_depth, 4); assert_eq!(m.max_length("/"), "/aam/0/0/c".len()); - assert_eq!(m.count, 11); + assert_eq!(m.count.get(), 11); } diff --git a/miniconf/tests/basic.rs b/miniconf/tests/basic.rs index c31a0a06..8c19896b 100644 --- a/miniconf/tests/basic.rs +++ b/miniconf/tests/basic.rs @@ -18,7 +18,7 @@ fn meta() { let meta = Settings::traverse_all::().unwrap(); assert_eq!(meta.max_depth, 2); assert_eq!(meta.max_length("/"), "/c/inner".len()); - assert_eq!(meta.count, 3); + assert_eq!(meta.count.get(), 3); } #[test] diff --git a/miniconf/tests/generics.rs b/miniconf/tests/generics.rs index 6ab11709..39f64c70 100644 --- a/miniconf/tests/generics.rs +++ b/miniconf/tests/generics.rs @@ -15,7 +15,7 @@ fn generic_type() { let metadata = Settings::::traverse_all::().unwrap(); assert_eq!(metadata.max_depth, 1); assert_eq!(metadata.max_length, "data".len()); - assert_eq!(metadata.count, 1); + assert_eq!(metadata.count.get(), 1); } #[test] @@ -34,7 +34,7 @@ fn generic_array() { let metadata = Settings::::traverse_all::().unwrap(); assert_eq!(metadata.max_depth, 2); assert_eq!(metadata.max_length("/"), "/data/0".len()); - assert_eq!(metadata.count, 2); + assert_eq!(metadata.count.get(), 2); } #[test] @@ -58,7 +58,7 @@ fn generic_struct() { let metadata = Settings::::traverse_all::().unwrap(); assert_eq!(metadata.max_depth, 1); assert_eq!(metadata.max_length("/"), "/inner".len()); - assert_eq!(metadata.count, 1); + assert_eq!(metadata.count.get(), 1); } #[test] diff --git a/miniconf/tests/packed.rs b/miniconf/tests/packed.rs index 640923f0..f2c85532 100644 --- a/miniconf/tests/packed.rs +++ b/miniconf/tests/packed.rs @@ -152,7 +152,7 @@ fn size() { let meta: Metadata = A31::traverse_all().unwrap(); assert_eq!(meta.max_bits, 31); assert_eq!(meta.max_depth, 31); - assert_eq!(meta.count, 1usize.pow(31)); + assert_eq!(meta.count.get(), 1usize.pow(31)); assert_eq!(meta.max_length, 31); // Another way to get to 32 bit is to take 15 length-3 (2 bit) levels and one length-1 (1 bit) level to fill it, needing (3**15 ~ 14 M) storage. @@ -167,6 +167,6 @@ fn size() { let meta: Metadata = A16::traverse_all().unwrap(); assert_eq!(meta.max_bits, 31); assert_eq!(meta.max_depth, 16); - assert_eq!(meta.count, 3usize.pow(15)); + assert_eq!(meta.count.get(), 3usize.pow(15)); assert_eq!(meta.max_length, 16); } diff --git a/miniconf/tests/skipped.rs b/miniconf/tests/skipped.rs index 8fa36657..c3872a8b 100644 --- a/miniconf/tests/skipped.rs +++ b/miniconf/tests/skipped.rs @@ -16,7 +16,7 @@ fn meta() { let meta = Settings::traverse_all::().unwrap(); assert_eq!(meta.max_depth, 1); assert_eq!(meta.max_length("/"), "/value".len()); - assert_eq!(meta.count, 1); + assert_eq!(meta.count.get(), 1); } #[test] diff --git a/miniconf/tests/structs.rs b/miniconf/tests/structs.rs index 40eae70c..971d458c 100644 --- a/miniconf/tests/structs.rs +++ b/miniconf/tests/structs.rs @@ -42,7 +42,7 @@ fn structs() { let metadata = Settings::traverse_all::().unwrap(); assert_eq!(metadata.max_depth, 2); assert_eq!(metadata.max_length("/"), "/d/a".len()); - assert_eq!(metadata.count, 4); + assert_eq!(metadata.count.get(), 4); assert_eq!(paths::(), ["/a", "/b", "/c", "/d/a"]); } diff --git a/miniconf_derive/src/field.rs b/miniconf_derive/src/field.rs index c53a5600..b7e2f443 100644 --- a/miniconf_derive/src/field.rs +++ b/miniconf_derive/src/field.rs @@ -62,7 +62,7 @@ impl TreeField { { None } else { - let bound: Option = match traite { + match traite { TreeTrait::Key => Some(parse_quote!(::miniconf::TreeKey)), TreeTrait::Serialize => self .deny @@ -76,8 +76,8 @@ impl TreeField { .then_some(parse_quote!(::miniconf::TreeDeserialize<'de>)), TreeTrait::Any => (self.deny.ref_any.is_none() || self.deny.mut_any.is_none()) .then_some(parse_quote!(::miniconf::TreeAny)), - }; - bound.map(|bound| { + } + .map(|bound: syn::TraitBound| { let ty = self.typ(); quote_spanned!(self.span()=> #ty: #bound,) }) @@ -106,7 +106,7 @@ impl TreeField { pub fn traverse_all(&self) -> TokenStream { let typ = self.typ(); - quote_spanned!(self.span()=> <#typ as ::miniconf::TreeKey>::traverse_all()?) + quote_spanned!(self.span()=> <#typ as ::miniconf::TreeKey>::traverse_all()) } fn getter(&self, i: Option) -> TokenStream { diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 58cf4c22..38856047 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -186,15 +186,6 @@ impl Tree { let where_clause = self.bound_generics(TreeTrait::Key, orig_where_clause); let fields = self.fields(); let fields_len = fields.len(); - let traverse_all_arms = fields.iter().enumerate().map(|(i, f)| { - let w = f.traverse_all(); - if self.flatten.is_present() { - quote!(walk = #w;) - } else { - quote!(walk = walk.merge(&#w, Some(#i), &Self::__MINICONF_LOOKUP)?;) - } - }); - let traverse_arms = fields.iter().enumerate().map(|(i, f)| f.traverse_by_key(i)); let names: Option> = match &self.data { Data::Struct(fields) if fields.style.is_struct() => Some( fields @@ -218,20 +209,30 @@ impl Tree { _ => None, }; let names = match names { - None => quote!(::core::option::Option::None), - Some(names) => quote!(::core::option::Option::Some(&[#(#names ,)*])), + None => quote! { + ::miniconf::KeyLookup::Numbered( + match ::core::num::NonZero::new(#fields_len) { + Some(n) => n, + None => unreachable!(), + }, + ) + }, + Some(names) => quote!(::miniconf::KeyLookup::Named(&[#(#names ,)*])), }; + let traverse_arms = fields.iter().enumerate().map(|(i, f)| f.traverse_by_key(i)); let index = self.index(); - let (traverse, increment) = if self.flatten.is_present() { - (None, None) + let (traverse, increment, traverse_all) = if self.flatten.is_present() { + (None, None, fields[0].traverse_all()) } else { + let w = fields.iter().map(|f| f.traverse_all()); ( Some(quote! { let name = Self::__MINICONF_LOOKUP.lookup(index)?; - func(index, name, Self::__MINICONF_LOOKUP.len) + func(index, name, Self::__MINICONF_LOOKUP.len()) .map_err(|err| ::miniconf::Error::Inner(1, err))?; }), Some(quote!(::miniconf::Error::increment_result)), + quote!(W::internal(&[#(&#w? ,)*], &Self::__MINICONF_LOOKUP)), ) }; @@ -239,22 +240,13 @@ impl Tree { // TODO: can these be hidden and disambiguated w.r.t. collision? #[automatically_derived] impl #impl_generics #ident #ty_generics #orig_where_clause { - const __MINICONF_LOOKUP: ::miniconf::KeyLookup = ::miniconf::KeyLookup { - len: match ::core::num::NonZero::new(#fields_len) { - Some(n) => n, - None => unreachable!(), - }, - names: #names, - }; + const __MINICONF_LOOKUP: ::miniconf::KeyLookup = #names; } #[automatically_derived] impl #impl_generics ::miniconf::TreeKey for #ident #ty_generics #where_clause { fn traverse_all() -> ::core::result::Result { - #[allow(unused_mut)] - let mut walk = W::internal(); - #(#traverse_all_arms)* - ::core::result::Result::Ok(walk) + #traverse_all } fn traverse_by_key(mut keys: K, mut func: F) -> ::core::result::Result> diff --git a/miniconf_mqtt/src/lib.rs b/miniconf_mqtt/src/lib.rs index f4e86e84..c3149262 100644 --- a/miniconf_mqtt/src/lib.rs +++ b/miniconf_mqtt/src/lib.rs @@ -251,7 +251,7 @@ where config: ConfigBuilder<'a, Broker>, ) -> Result { assert_eq!("/".len(), SEPARATOR.len_utf8()); - let meta: Metadata = Settings::traverse_all().unwrap(); + let meta: Metadata = Settings::traverse_all().unwrap(); // Note(unwrap): infallible assert!(meta.max_depth <= Y); assert!(prefix.len() + "/settings".len() + meta.max_length("/") <= MAX_TOPIC_LENGTH); @@ -527,21 +527,20 @@ where Traversal::TooShort(_depth), )) => { // Internal node: Dump or List - (state.state() == &sm::States::Single) - .then_some(()) - .ok_or("Pending multipart response") - .and_then(|()| Multipart::try_from(properties)) - .map_or_else( - |err| { - Self::respond(err, ResponseCode::Error, properties, client) - .ok(); - }, - |m| { - *pending = m.root(path).unwrap(); // Note(unwrap) checked that it's TooShort but valid leaf - state.process_event(sm::Events::Multipart).unwrap(); - // Responses come through iter_list/iter_dump - }, - ); + (state.state() != &sm::States::Single) + .then_some("Pending multipart response") + .or_else(|| { + Multipart::try_from(properties) + .map(|m| { + *pending = m.root(path).unwrap(); // Note(unwrap) checked that it's TooShort but valid leaf + state.process_event(sm::Events::Multipart).unwrap(); + // Responses come through iter_list/iter_dump + }) + .err() + }) + .map(|msg| { + Self::respond(msg, ResponseCode::Error, properties, client).ok() + }); } minimq::PubError::Serialization(err) => { Self::respond(err, ResponseCode::Error, properties, client).ok(); @@ -557,11 +556,16 @@ where State::Unchanged } else { // Set - json::set_by_key(settings, path, payload) - .map_err(|err| Self::respond(err, ResponseCode::Error, properties, client).ok()) - .map(|_depth| Self::respond("OK", ResponseCode::Ok, properties, client).ok()) - .is_ok() - .into() + match json::set_by_key(settings, path, payload) { + Err(err) => { + Self::respond(err, ResponseCode::Error, properties, client).ok(); + State::Unchanged + } + Ok(_depth) => { + Self::respond("OK", ResponseCode::Ok, properties, client).ok(); + State::Changed + } + } } }) .map(Option::unwrap_or_default)