From ee05032759cf2fd6c04b6501b8c35b8e2e3942a7 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Thu, 18 Apr 2019 16:00:36 +1000 Subject: [PATCH] refactor Value internals to use trait objects --- src/kv/value/any.rs | 97 ----------------------- src/kv/value/backend.rs | 62 --------------- src/kv/value/impls.rs | 165 +++++++++++++++++++++++++++++++-------- src/kv/value/internal.rs | 86 ++++++++++++++++++++ src/kv/value/mod.rs | 29 ++++--- 5 files changed, 238 insertions(+), 201 deletions(-) delete mode 100644 src/kv/value/any.rs delete mode 100644 src/kv/value/backend.rs create mode 100644 src/kv/value/internal.rs diff --git a/src/kv/value/any.rs b/src/kv/value/any.rs deleted file mode 100644 index 04baf4844..000000000 --- a/src/kv/value/any.rs +++ /dev/null @@ -1,97 +0,0 @@ -use std::{fmt, mem}; -use std::marker::PhantomData; - -use super::{ToValue, KeyValueError}; -use super::backend::Backend; - -/// A function for converting some type `T` into a [`Value`](struct.Value.html). -pub type FromAnyFn = fn(FromAny, &T) -> Result<(), KeyValueError>; - -/// A helper for converting any type into a [`Value`](struct.Value.html). -pub struct FromAny<'a>(&'a mut Backend); - -impl<'a> FromAny<'a> { - pub(super) fn value(self, v: T) -> Result<(), KeyValueError> - where - T: ToValue, - { - v.to_value().inner.visit(self.0) - } - - /// Convert a formattable type into a value. - pub fn debug(self, v: T) -> Result<(), KeyValueError> - where - T: fmt::Debug - { - self.0.fmt(format_args!("{:?}", v)) - } - - /// Convert a `u64` into a value. - pub fn u64(self, v: u64) -> Result<(), KeyValueError> { - self.0.u64(v) - } - - /// Convert a `i64` into a value. - pub fn i64(self, v: i64) -> Result<(), KeyValueError> { - self.0.i64(v) - } - - /// Convert a `f64` into a value. - pub fn f64(self, v: f64) -> Result<(), KeyValueError> { - self.0.f64(v) - } - - /// Convert a `bool` into a value. - pub fn bool(self, v: bool) -> Result<(), KeyValueError> { - self.0.bool(v) - } - - /// Convert a `char` into a value. - pub fn char(self, v: char) -> Result<(), KeyValueError> { - self.0.char(v) - } - - /// Convert an empty type into a value. - pub fn none(self) -> Result<(), KeyValueError> { - self.0.none() - } - - /// Convert a string into a value. - pub fn str(self, v: &str) -> Result<(), KeyValueError> { - self.0.str(v) - } -} - -// `Any<'a>` is very similar to `std::fmt::Arguments<'a>` -// It takes a &T and fn pointer and stores them in an erased structure. -// It's a bit like an ad-hoc trait object that can accept any arbitrary -// value without those values needing to implement any traits. - -#[derive(Clone, Copy)] -pub(super) struct Any<'a> { - data: &'a Void, - from: FromAnyFn, -} - -// FIXME: This would be more correct as an extern type -// Replace once the `extern_types` feature is stable -// and available -struct Void { - _priv: (), - _oibit_remover: PhantomData<*mut Fn()>, -} - -impl<'a> Any<'a> { - pub(super) fn new(data: &'a T, from: FromAnyFn) -> Self { - unsafe { - Any { - data: mem::transmute::<&'a T, &'a Void>(data), - from: mem::transmute::, FromAnyFn>(from), - } - } - } - - pub(super) fn visit(&self, backend: &mut Backend) -> Result<(), KeyValueError> { - (self.from)(FromAny(backend), self.data) - } -} diff --git a/src/kv/value/backend.rs b/src/kv/value/backend.rs deleted file mode 100644 index a378581dd..000000000 --- a/src/kv/value/backend.rs +++ /dev/null @@ -1,62 +0,0 @@ -use std::fmt; - -use super::KeyValueError; - -// `Backend` is an internal visitor for the structure of a value. -// Right now we only have an implementation for `std::fmt`, but -// this trait makes it possible to add more structured backends like -// `serde` that can retain some of that original structure. -// -// `Backend` isn't expected to be public, so that `log` can hide its -// internal serialization contract. This keeps its public API small -// and gives us room to move while the API is still evolving. -// For a public API, see the `FromAny` type. - -pub(super) trait Backend { - fn fmt(&mut self, v: fmt::Arguments) -> Result<(), KeyValueError>; - fn u64(&mut self, v: u64) -> Result<(), KeyValueError>; - fn i64(&mut self, v: i64) -> Result<(), KeyValueError>; - fn f64(&mut self, v: f64) -> Result<(), KeyValueError>; - fn bool(&mut self, v: bool) -> Result<(), KeyValueError>; - fn char(&mut self, v: char) -> Result<(), KeyValueError>; - fn str(&mut self, v: &str) -> Result<(), KeyValueError>; - fn none(&mut self) -> Result<(), KeyValueError>; -} - -pub(super) struct FmtBackend<'a, 'b: 'a>(pub(super) &'a mut fmt::Formatter<'b>); - -impl<'a, 'b: 'a> Backend for FmtBackend<'a, 'b> { - fn fmt(&mut self, v: fmt::Arguments) -> Result<(), KeyValueError> { - fmt::Debug::fmt(&v, self.0)?; - - Ok(()) - } - - fn u64(&mut self, v: u64) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", v)) - } - - fn i64(&mut self, v: i64) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", v)) - } - - fn f64(&mut self, v: f64) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", v)) - } - - fn bool(&mut self, v: bool) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", v)) - } - - fn char(&mut self, v: char) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", v)) - } - - fn str(&mut self, v: &str) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", v)) - } - - fn none(&mut self) -> Result<(), KeyValueError> { - self.fmt(format_args!("{:?}", Option::None::<()>)) - } -} diff --git a/src/kv/value/impls.rs b/src/kv/value/impls.rs index ece0f6e72..a78498541 100644 --- a/src/kv/value/impls.rs +++ b/src/kv/value/impls.rs @@ -1,88 +1,172 @@ use std::fmt; -use super::{ToValue, Value}; +use super::{KeyValueError, ToValue, Value, Visit, Visitor}; impl ToValue for u8 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.u64(*value as u64)) + Value::from_internal(self) + } +} + +impl Visit for u8 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.u64(*self as u64) } } impl ToValue for u16 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.u64(*value as u64)) + Value::from_internal(self) + } +} + +impl Visit for u16 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.u64(*self as u64) } } impl ToValue for u32 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.u64(*value as u64)) + Value::from_internal(self) + } +} + +impl Visit for u32 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.u64(*self as u64) } } impl ToValue for u64 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.u64(*value)) + Value::from_internal(self) + } +} + +impl Visit for u64 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.u64(*self) } } impl ToValue for i8 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.i64(*value as i64)) + Value::from_internal(self) + } +} + +impl Visit for i8 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.i64(*self as i64) } } impl ToValue for i16 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.i64(*value as i64)) + Value::from_internal(self) + } +} + +impl Visit for i16 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.i64(*self as i64) } } impl ToValue for i32 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.i64(*value as i64)) + Value::from_internal(self) + } +} + +impl Visit for i32 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.i64(*self as i64) } } impl ToValue for i64 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.i64(*value)) + Value::from_internal(self) + } +} + +impl Visit for i64 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.i64(*self) } } impl ToValue for f32 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.f64(*value as f64)) + Value::from_internal(self) + } +} + +impl Visit for f32 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.f64(*self as f64) } } impl ToValue for f64 { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.f64(*value)) + Value::from_internal(self) + } +} + +impl Visit for f64 { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.f64(*self) } } impl ToValue for bool { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.bool(*value)) + Value::from_internal(self) + } +} + +impl Visit for bool { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.bool(*self) } } impl ToValue for char { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.char(*value)) + Value::from_internal(self) + } +} + +impl Visit for char { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.char(*self) } } impl<'v> ToValue for &'v str { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.str(value)) + Value::from_internal(self) + } +} + +impl<'v> Visit for &'v str { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.str(*self) } } impl ToValue for () { fn to_value(&self) -> Value { - Value::from_any(self, |from, _| from.none()) + Value::from_internal(self) + } +} + +impl Visit for () { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.none() } } @@ -91,18 +175,25 @@ where T: ToValue, { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| { - match *value { - Some(ref value) => from.value(value), - None => from.none(), - } - }) + Value::from_internal(self) + } +} + +impl Visit for Option +where + T: ToValue, +{ + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + match *self { + Some(ref value) => value.to_value().visit(visitor), + None => visitor.none(), + } } } impl<'v> ToValue for fmt::Arguments<'v> { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.debug(value)) + Value::from_debug(self) } } @@ -123,13 +214,25 @@ mod std_support { impl ToValue for String { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.str(&*value)) + Value::from_internal(self) + } + } + + impl Visit for String { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.str(&*self) } } impl<'a> ToValue for Cow<'a, str> { fn to_value(&self) -> Value { - Value::from_any(self, |from, value| from.str(&*value)) + Value::from_internal(self) + } + } + + impl<'a> Visit for Cow<'a, str> { + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + visitor.str(&*self) } } } @@ -138,7 +241,7 @@ mod std_support { mod tests { use super::*; use kv::value::KeyValueError; - use kv::value::backend::Backend; + use kv::value::internal::Visitor; use std::fmt::Write; use std::str::{self, Utf8Error}; @@ -216,15 +319,15 @@ mod tests { None, } - struct TestBackend(F); + struct TestVisitor(F); - impl Backend for TestBackend + impl Visitor for TestVisitor where F: Fn(Token), { - fn fmt(&mut self, v: fmt::Arguments) -> Result<(), KeyValueError> { + fn debug(&mut self, v: &fmt::Debug) -> Result<(), KeyValueError> { let mut buf = Buffer::new(); - write!(&mut buf, "{}", v)?; + write!(&mut buf, "{:?}", v)?; let s = buf.as_str().map_err(|_| KeyValueError::msg("invalid UTF8"))?; (self.0)(Token::Str(s)); @@ -269,8 +372,8 @@ mod tests { // Check that a value retains the right structure fn check(value: Value, expected: Token) { - let mut backend = TestBackend(|token: Token| assert_eq!(expected, token)); - value.inner.visit(&mut backend).unwrap(); + let mut visitor = TestVisitor(|token: Token| assert_eq!(expected, token)); + value.visit(&mut visitor).unwrap(); } check(42u64.to_value(), Token::U64(42)); diff --git a/src/kv/value/internal.rs b/src/kv/value/internal.rs new file mode 100644 index 000000000..4b1d91b73 --- /dev/null +++ b/src/kv/value/internal.rs @@ -0,0 +1,86 @@ +use std::fmt; + +use super::KeyValueError; + +// `Visit` and `Visitor` is an internal API for visiting the structure of a value. +// It's not intended to be public (at this stage). +// +// Right now we only have an implementation for `std::fmt`, but +// this trait makes it possible to add more structured backends like +// `serde` that can retain that original structure. + +/// 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 formattable value. + Debug(&'v fmt::Debug), +} + +impl<'v> Inner<'v> { + pub(super) fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + match *self { + Inner::Internal(ref value) => value.visit(visitor), + Inner::Debug(ref value) => visitor.debug(value), + } + } +} + +/// An internal structure-preserving value. +pub(super) trait Visit { + fn visit(&self, backend: &mut Visitor) -> Result<(), KeyValueError>; +} + +/// The internal serialization contract. +pub(super) trait Visitor { + fn debug(&mut self, v: &fmt::Debug) -> Result<(), KeyValueError>; + + fn u64(&mut self, v: u64) -> Result<(), KeyValueError>; + fn i64(&mut self, v: i64) -> Result<(), KeyValueError>; + fn f64(&mut self, v: f64) -> Result<(), KeyValueError>; + fn bool(&mut self, v: bool) -> Result<(), KeyValueError>; + fn char(&mut self, v: char) -> Result<(), KeyValueError>; + fn str(&mut self, v: &str) -> Result<(), KeyValueError>; + fn none(&mut self) -> Result<(), KeyValueError>; +} + +/// A visitor for `std::fmt`. +pub(super) struct FmtVisitor<'a, 'b: 'a>(pub(super) &'a mut fmt::Formatter<'b>); + +impl<'a, 'b: 'a> Visitor for FmtVisitor<'a, 'b> { + fn debug(&mut self, v: &fmt::Debug) -> Result<(), KeyValueError> { + v.fmt(self.0)?; + + Ok(()) + } + + fn u64(&mut self, v: u64) -> Result<(), KeyValueError> { + self.debug(&format_args!("{:?}", v)) + } + + fn i64(&mut self, v: i64) -> Result<(), KeyValueError> { + self.debug(&format_args!("{:?}", v)) + } + + fn f64(&mut self, v: f64) -> Result<(), KeyValueError> { + self.debug(&format_args!("{:?}", v)) + } + + fn bool(&mut self, v: bool) -> Result<(), KeyValueError> { + self.debug(&format_args!("{:?}", v)) + } + + fn char(&mut self, v: char) -> Result<(), KeyValueError> { + self.debug(&format_args!("{:?}", v)) + } + + fn str(&mut self, v: &str) -> Result<(), KeyValueError> { + self.debug(&format_args!("{:?}", v)) + } + + fn none(&mut self) -> Result<(), KeyValueError> { + self.debug(&format_args!("None")) + } +} diff --git a/src/kv/value/mod.rs b/src/kv/value/mod.rs index 0450d3152..727e89ef5 100644 --- a/src/kv/value/mod.rs +++ b/src/kv/value/mod.rs @@ -2,13 +2,12 @@ use std::fmt; -mod any; -mod backend; +mod internal; mod impls; use kv::KeyValueError; -use self::any::{Any, FromAnyFn}; +use self::internal::{Inner, Visit, Visitor}; /// A type that can be converted into a [`Value`](struct.Value.html). pub trait ToValue { @@ -35,15 +34,17 @@ impl<'v> ToValue for Value<'v> { /// A value in a structured key-value pair. pub struct Value<'v> { - inner: Any<'v>, + inner: Inner<'v>, } impl<'v> Value<'v> { - // FIXME: The `from_any` API is intended to be public. - // Its implications need discussion first though. - fn from_any(v: &'v T, from: FromAnyFn) -> Self { + /// Get a value from an internal `Visit`. + fn from_internal(value: &'v T) -> Self + where + T: Visit, + { Value { - inner: Any::new(v, from) + inner: Inner::Internal(value), } } @@ -52,13 +53,19 @@ impl<'v> Value<'v> { where T: fmt::Debug, { - Self::from_any(value, |from, value| from.debug(value)) + Value { + inner: Inner::Debug(value), + } + } + + fn visit(&self, visitor: &mut Visitor) -> Result<(), KeyValueError> { + self.inner.visit(visitor) } } impl<'v> fmt::Debug for Value<'v> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.inner.visit(&mut self::backend::FmtBackend(f))?; + self.visit(&mut self::internal::FmtVisitor(f))?; Ok(()) } @@ -66,7 +73,7 @@ impl<'v> fmt::Debug for Value<'v> { impl<'v> fmt::Display for Value<'v> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.inner.visit(&mut self::backend::FmtBackend(f))?; + self.visit(&mut self::internal::FmtVisitor(f))?; Ok(()) }