From 144b4829f83cc210157d2b45c104b0540ec3bf75 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 4 Jul 2019 07:54:50 +1000 Subject: [PATCH 1/2] refactor Value internals to avoid needing to buffer that means doing two things: 1. allowing us to create a value with any lifetime from some owned primitive, like u8, bool etc 2. allowing us to create a value from some type that _can_ produce a concrete value we can use, but doesn't guarantee it will live as long as the value itself --- src/kv/mod.rs | 2 +- src/kv/source.rs | 30 ++++++++ src/kv/value/impls.rs | 161 ++++++++++++++++----------------------- src/kv/value/internal.rs | 40 +++++++--- src/kv/value/mod.rs | 79 ++++++++++++++++--- src/lib.rs | 16 +--- 6 files changed, 198 insertions(+), 130 deletions(-) diff --git a/src/kv/mod.rs b/src/kv/mod.rs index f10441254..5fe6ab833 100644 --- a/src/kv/mod.rs +++ b/src/kv/mod.rs @@ -3,7 +3,7 @@ mod error; mod source; mod key; -mod value; +pub mod value; pub use self::error::Error; pub use self::source::{Source, Visitor}; diff --git a/src/kv/source.rs b/src/kv/source.rs index a14e93546..0538c14f4 100644 --- a/src/kv/source.rs +++ b/src/kv/source.rs @@ -1,5 +1,6 @@ //! Sources for key-value pairs. +use std::fmt; use kv::{Error, Key, ToKey, Value, ToValue}; /// A source of key-value pairs. @@ -168,6 +169,35 @@ where } } +impl<'a, 'b: 'a, 'kvs> Visitor<'kvs> for fmt::DebugMap<'a, 'b> { + fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), Error> { + self.entry(&key, &value); + Ok(()) + } +} + +impl<'a, 'b: 'a, 'kvs> Visitor<'kvs> for fmt::DebugList<'a, 'b> { + fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), Error> { + self.entry(&(key, value)); + Ok(()) + } +} + +impl<'a, 'b: 'a, 'kvs> Visitor<'kvs> for fmt::DebugSet<'a, 'b> { + fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), Error> { + self.entry(&(key, value)); + Ok(()) + } +} + +impl<'a, 'b: 'a, 'kvs> Visitor<'kvs> for fmt::DebugTuple<'a, 'b> { + fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), Error> { + self.field(&key); + self.field(&value); + Ok(()) + } +} + #[cfg(feature = "std")] mod std_support { use super::*; diff --git a/src/kv/value/impls.rs b/src/kv/value/impls.rs index 651097d27..7e42b3468 100644 --- a/src/kv/value/impls.rs +++ b/src/kv/value/impls.rs @@ -1,196 +1,190 @@ use std::fmt; -use super::{Error, ToValue, Value, Visit, Visitor}; +use super::{ToValue, Value, Primitive}; impl ToValue for usize { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for usize { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.u64(*self as u64) +impl<'v> From for Value<'v> { + fn from(value: usize) -> Self { + Value::from_primitive(Primitive::Unsigned(value as u64)) } } impl ToValue for isize { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for isize { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.i64(*self as i64) +impl<'v> From for Value<'v> { + fn from(value: isize) -> Self { + Value::from_primitive(Primitive::Signed(value as i64)) } } impl ToValue for u8 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for u8 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.u64(*self as u64) +impl<'v> From for Value<'v> { + fn from(value: u8) -> Self { + Value::from_primitive(Primitive::Unsigned(value as u64)) } } impl ToValue for u16 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for u16 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.u64(*self as u64) +impl<'v> From for Value<'v> { + fn from(value: u16) -> Self { + Value::from_primitive(Primitive::Unsigned(value as u64)) } } impl ToValue for u32 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for u32 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.u64(*self as u64) +impl<'v> From for Value<'v> { + fn from(value: u32) -> Self { + Value::from_primitive(Primitive::Unsigned(value as u64)) } } impl ToValue for u64 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for u64 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.u64(*self) +impl<'v> From for Value<'v> { + fn from(value: u64) -> Self { + Value::from_primitive(Primitive::Unsigned(value)) } } impl ToValue for i8 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for i8 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.i64(*self as i64) +impl<'v> From for Value<'v> { + fn from(value: i8) -> Self { + Value::from_primitive(Primitive::Signed(value as i64)) } } impl ToValue for i16 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for i16 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.i64(*self as i64) +impl<'v> From for Value<'v> { + fn from(value: i16) -> Self { + Value::from_primitive(Primitive::Signed(value as i64)) } } impl ToValue for i32 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for i32 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.i64(*self as i64) +impl<'v> From for Value<'v> { + fn from(value: i32) -> Self { + Value::from_primitive(Primitive::Signed(value as i64)) } } impl ToValue for i64 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for i64 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.i64(*self) +impl<'v> From for Value<'v> { + fn from(value: i64) -> Self { + Value::from_primitive(Primitive::Signed(value)) } } impl ToValue for f32 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for f32 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.f64(*self as f64) +impl<'v> From for Value<'v> { + fn from(value: f32) -> Self { + Value::from_primitive(Primitive::Float(value as f64)) } } impl ToValue for f64 { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for f64 { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.f64(*self) +impl<'v> From for Value<'v> { + fn from(value: f64) -> Self { + Value::from_primitive(Primitive::Float(value)) } } impl ToValue for bool { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for bool { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.bool(*self) +impl<'v> From for Value<'v> { + fn from(value: bool) -> Self { + Value::from_primitive(Primitive::Bool(value)) } } impl ToValue for char { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl Visit for char { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.char(*self) +impl<'v> From for Value<'v> { + fn from(value: char) -> Self { + Value::from_primitive(Primitive::Char(value)) } } impl<'v> ToValue for &'v str { fn to_value(&self) -> Value { - Value::from_internal(self) + Value::from(*self) } } -impl<'v> Visit for &'v str { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.str(*self) +impl<'v> From<&'v str> for Value<'v> { + fn from(value: &'v str) -> Self { + Value::from_primitive(Primitive::Str(value)) } } impl ToValue for () { fn to_value(&self) -> Value { - Value::from_internal(self) - } -} - -impl Visit for () { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.none() + Value::from_primitive(Primitive::None) } } @@ -199,18 +193,9 @@ where T: ToValue, { fn to_value(&self) -> Value { - Value::from_internal(self) - } -} - -impl Visit for Option -where - T: ToValue, -{ - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { match *self { - Some(ref value) => value.to_value().visit(visitor), - None => visitor.none(), + Some(ref value) => value.to_value(), + None => Value::from_primitive(Primitive::None), } } } @@ -238,25 +223,13 @@ mod std_support { impl ToValue for String { fn to_value(&self) -> Value { - Value::from_internal(self) - } - } - - impl Visit for String { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.str(&*self) + Value::from_primitive(Primitive::Str(&*self)) } } - impl<'a> ToValue for Cow<'a, str> { + impl<'v> ToValue for Cow<'v, str> { fn to_value(&self) -> Value { - Value::from_internal(self) - } - } - - impl<'a> Visit for Cow<'a, str> { - fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { - visitor.str(&*self) + Value::from_primitive(Primitive::Str(&*self)) } } } diff --git a/src/kv/value/internal.rs b/src/kv/value/internal.rs index 2570bffbb..1a4d4697a 100644 --- a/src/kv/value/internal.rs +++ b/src/kv/value/internal.rs @@ -1,6 +1,6 @@ use std::fmt; -use super::Error; +use super::{Fill, Slot, Error}; // `Visit` and `Visitor` is an internal API for visiting the structure of a value. // It's not intended to be public (at this stage). @@ -12,9 +12,10 @@ use super::Error; /// A container for a structured value for a specific kind of visitor. #[derive(Clone, Copy)] pub(super) enum Inner<'v> { - /// An internal `Visit`. It'll be an internal structure-preserving - /// type from the standard library that's implemented in this crate. - Internal(&'v Visit), + /// A simple primitive value that can be copied without allocating. + Primitive(Primitive<'v>), + /// A value that can be filled. + Fill(&'v Fill), /// A debuggable value. Debug(&'v fmt::Debug), /// A displayable value. @@ -24,18 +25,22 @@ pub(super) enum Inner<'v> { impl<'v> Inner<'v> { pub(super) fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { match *self { - Inner::Internal(ref value) => value.visit(visitor), - Inner::Debug(ref value) => visitor.debug(value), - Inner::Display(ref value) => visitor.display(value), + Inner::Primitive(value) => match value { + Primitive::Signed(value) => visitor.i64(value), + Primitive::Unsigned(value) => visitor.u64(value), + Primitive::Float(value) => visitor.f64(value), + Primitive::Bool(value) => visitor.bool(value), + Primitive::Char(value) => visitor.char(value), + Primitive::Str(value) => visitor.str(value), + Primitive::None => visitor.none(), + }, + Inner::Fill(value) => value.fill(Slot::new(visitor)), + Inner::Debug(value) => visitor.debug(value), + Inner::Display(value) => visitor.display(value), } } } -/// An internal structure-preserving value. -pub(super) trait Visit { - fn visit(&self, backend: &mut Visitor) -> Result<(), Error>; -} - /// The internal serialization contract. pub(super) trait Visitor { fn debug(&mut self, v: &fmt::Debug) -> Result<(), Error>; @@ -52,6 +57,17 @@ pub(super) trait Visitor { fn none(&mut self) -> Result<(), Error>; } +#[derive(Clone, Copy)] +pub(super) enum Primitive<'v> { + Signed(i64), + Unsigned(u64), + Float(f64), + Bool(bool), + Char(char), + Str(&'v str), + None, +} + /// A visitor for `std::fmt`. pub(super) struct FmtVisitor<'a, 'b: 'a>(pub(super) &'a mut fmt::Formatter<'b>); diff --git a/src/kv/value/mod.rs b/src/kv/value/mod.rs index ddde43314..431858dc4 100644 --- a/src/kv/value/mod.rs +++ b/src/kv/value/mod.rs @@ -8,9 +8,9 @@ mod impls; #[cfg(test)] pub(in kv) mod test; -use kv::Error; +pub use kv::Error; -use self::internal::{Inner, Visit, Visitor}; +use self::internal::{Inner, Visitor, Primitive}; /// A type that can be converted into a [`Value`](struct.Value.html). pub trait ToValue { @@ -35,6 +35,60 @@ impl<'v> ToValue for Value<'v> { } } +/// A type that requires extra work to convert into a [`Value`](struct.Value.html). +/// +/// This trait is a more advanced initialization API than [`ToValue`](trait.ToValue.html). +/// It's intended for erased values coming from other logging frameworks that may need +/// to perform extra work to determine the concrete type to use. +pub trait Fill { + /// Fill a value. + fn fill(&self, slot: Slot) -> Result<(), Error>; +} + +impl<'a, T> Fill for &'a T +where + T: Fill + ?Sized, +{ + fn fill(&self, slot: Slot) -> Result<(), Error> { + (**self).fill(slot) + } +} + +/// A value slot to fill using the [`Fill`](trait.Fill.html) trait. +pub struct Slot<'a> { + filled: bool, + visitor: &'a mut Visitor, +} + +impl<'a> fmt::Debug for Slot<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("Slot").finish() + } +} + +impl<'a> Slot<'a> { + fn new(visitor: &'a mut Visitor) -> Self { + Slot { + visitor, + filled: false, + } + } + + /// Fill the slot with a value. + /// + /// The given value doesn't need to satisfy any particular lifetime constraints. + /// + /// # Panics + /// + /// Calling `fill` more than once will panic. + pub fn fill(&mut self, value: Value) -> Result<(), Error> { + assert!(!self.filled, "the slot has already been filled"); + self.filled = true; + + value.visit(self.visitor) + } +} + /// A value in a structured key-value pair. pub struct Value<'v> { inner: Inner<'v>, @@ -42,12 +96,9 @@ pub struct Value<'v> { impl<'v> Value<'v> { /// Get a value from an internal `Visit`. - fn from_internal(value: &'v T) -> Self - where - T: Visit, - { + fn from_primitive(value: Primitive<'v>) -> Self { Value { - inner: Inner::Internal(value), + inner: Inner::Primitive(value), } } @@ -61,8 +112,8 @@ impl<'v> Value<'v> { } } - /// Get a value from a displayable type. - pub fn from_display(value: &'v T) -> Self + /// Get a value from a displayable type. + pub fn from_display(value: &'v T) -> Self where T: fmt::Display, { @@ -71,6 +122,16 @@ impl<'v> Value<'v> { } } + /// Get a value from a fillable slot. + pub fn from_fill(value: &'v T) -> Self + where + T: Fill, + { + Value { + inner: Inner::Fill(value), + } + } + fn visit(&self, visitor: &mut Visitor) -> Result<(), Error> { self.inner.visit(visitor) } diff --git a/src/lib.rs b/src/lib.rs index e1e6fdf4a..ee70aac92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -736,21 +736,9 @@ struct KeyValues<'a>(&'a kv::Source); #[cfg(feature = "kv_unstable")] impl<'a> fmt::Debug for KeyValues<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use self::kv::{Key, Value, Visitor, Error}; - - struct FmtVisitor<'a, 'b: 'a>(fmt::DebugMap<'a, 'b>); - - impl<'a, 'b: 'a, 'kvs> Visitor<'kvs> for FmtVisitor<'a, 'b> { - fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), Error> { - self.0.entry(&key, &value); - - Ok(()) - } - } - - let mut visitor = FmtVisitor(f.debug_map()); + let mut visitor = f.debug_map(); self.0.visit(&mut visitor)?; - visitor.0.finish() + visitor.finish() } } From b4aec46c7589708fe7e0641c7be9545867cd0c76 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 4 Jul 2019 09:12:59 +1000 Subject: [PATCH 2/2] add test for Fill --- src/kv/value/internal.rs | 2 +- src/kv/value/mod.rs | 41 ++++++++++++++++++++++++++++++++++++++-- src/kv/value/test.rs | 6 ++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/kv/value/internal.rs b/src/kv/value/internal.rs index 1a4d4697a..2c4772322 100644 --- a/src/kv/value/internal.rs +++ b/src/kv/value/internal.rs @@ -34,7 +34,7 @@ impl<'v> Inner<'v> { Primitive::Str(value) => visitor.str(value), Primitive::None => visitor.none(), }, - Inner::Fill(value) => value.fill(Slot::new(visitor)), + Inner::Fill(value) => value.fill(&mut Slot::new(visitor)), Inner::Debug(value) => visitor.debug(value), Inner::Display(value) => visitor.display(value), } diff --git a/src/kv/value/mod.rs b/src/kv/value/mod.rs index 431858dc4..1053589e5 100644 --- a/src/kv/value/mod.rs +++ b/src/kv/value/mod.rs @@ -42,14 +42,14 @@ impl<'v> ToValue for Value<'v> { /// to perform extra work to determine the concrete type to use. pub trait Fill { /// Fill a value. - fn fill(&self, slot: Slot) -> Result<(), Error>; + fn fill(&self, slot: &mut Slot) -> Result<(), Error>; } impl<'a, T> Fill for &'a T where T: Fill + ?Sized, { - fn fill(&self, slot: Slot) -> Result<(), Error> { + fn fill(&self, slot: &mut Slot) -> Result<(), Error> { (**self).fill(slot) } } @@ -152,3 +152,40 @@ impl<'v> fmt::Display for Value<'v> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fill_value() { + struct TestFill; + + impl Fill for TestFill { + fn fill(&self, slot: &mut Slot) -> Result<(), Error> { + let dbg: &fmt::Debug = &1; + + slot.fill(Value::from_debug(&dbg)) + } + } + + assert_eq!("1", Value::from_fill(&TestFill).to_str_buf()); + } + + #[test] + #[should_panic] + fn fill_multiple_times_panics() { + struct BadFill; + + impl Fill for BadFill { + fn fill(&self, slot: &mut Slot) -> Result<(), Error> { + slot.fill(42.into())?; + slot.fill(6789.into())?; + + Ok(()) + } + } + + let _ = Value::from_fill(&BadFill).to_str_buf(); + } +} diff --git a/src/kv/value/test.rs b/src/kv/value/test.rs index 5f3296f6a..0d5239bcc 100644 --- a/src/kv/value/test.rs +++ b/src/kv/value/test.rs @@ -44,6 +44,12 @@ impl<'a> PartialEq<&'a str> for StrBuf { } } +impl<'a> PartialEq for &'a str { + fn eq(&self, other: &StrBuf) -> bool { + *self == other.as_ref() + } +} + impl AsRef for StrBuf { fn as_ref(&self) -> &str { str::from_utf8(&self.buf[0..self.len]).unwrap()