From 009637717e5b824eb7c1e8d50558b49efd0c6ff0 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Tue, 30 Jan 2024 08:02:41 +1000 Subject: [PATCH 1/5] some more kv docs --- src/kv/mod.rs | 81 ++++++++++++++++++---------------- src/kv/source.rs | 14 +++--- src/kv/value.rs | 110 +++++++++++++++++++++++++++++++++++------------ src/lib.rs | 2 +- 4 files changed, 135 insertions(+), 72 deletions(-) diff --git a/src/kv/mod.rs b/src/kv/mod.rs index 650fc6da2..7bff51fb2 100644 --- a/src/kv/mod.rs +++ b/src/kv/mod.rs @@ -20,31 +20,33 @@ //! unstructured text first. //! //! In `log`, user-defined attributes are part of a [`Source`] on the log record. -//! Each attribute is a key-value; a pair of [`Key`] and [`Value`]. Keys are strings -//! and values are a datum of any type that can be formatted or serialized. Simple types +//! Each attribute is a key-value; a pair of [`Key`] and [`Value`]. Keys are strings +//! and values are a datum of any type that can be formatted or serialized. Simple types //! like strings, booleans, and numbers are supported, as well as arbitrarily complex //! structures involving nested objects and sequences. //! //! ## Adding key-values to log records //! -//! Key-values appear after the message format in the `log!` macros: +//! Key-values appear before the message format in the `log!` macros: //! //! ``` -//! .. +//! # use log::info; +//! info!(a = 1; "Something of interest"); //! ``` //! //! ## Working with key-values on log records //! //! Use the [`LogRecord::key_values`] method to access key-values. -//! +//! //! Individual values can be pulled from the source by their key: //! //! ``` //! # fn main() -> Result<(), log::kv::Error> { //! use log::kv::{Source, Key, Value}; //! # let record = log::Record::builder().key_values(&[("a", 1)]).build(); -//! -//! // info!("Something of interest"; a = 1); +//! +//! // info!(a = 1; "Something of interest"); +//! //! let a: Value = record.key_values().get(Key::from("a")).unwrap(); //! # Ok(()) //! # } @@ -56,28 +58,29 @@ //! # fn main() -> Result<(), log::kv::Error> { //! # let record = log::Record::builder().key_values(&[("a", 1), ("b", 2), ("c", 3)]).build(); //! use std::collections::BTreeMap; -//! +//! //! use log::kv::{self, Source, Key, Value, source::Visitor}; -//! +//! //! struct Collect<'kvs>(BTreeMap, Value<'kvs>>); -//! +//! //! impl<'kvs> Visitor<'kvs> for Collect<'kvs> { //! fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), kv::Error> { //! self.0.insert(key, value); -//! +//! //! Ok(()) //! } //! } -//! +//! //! let mut visitor = Collect(BTreeMap::new()); -//! -//! // info!("Something of interest"; a = 1, b = 2, c = 3); +//! +//! // info!(a = 1, b = 2, c = 3; "Something of interest"); +//! //! record.key_values().visit(&mut visitor)?; -//! +//! //! let collected = visitor.0; -//! +//! //! assert_eq!( -//! vec!["a", "b", "c"], +//! vec!["a", "b", "c"], //! collected //! .keys() //! .map(|k| k.as_str()) @@ -93,10 +96,11 @@ //! # fn main() -> Result<(), log::kv::Error> { //! use log::kv::{Source, Key}; //! # let record = log::Record::builder().key_values(&[("a", 1)]).build(); -//! -//! // info!("Something of interest"; a = 1); +//! +//! // info!(a = 1; "Something of interest"); +//! //! let a = record.key_values().get(Key::from("a")).unwrap(); -//! +//! //! assert_eq!(1, a.to_i64().unwrap()); //! # Ok(()) //! # } @@ -109,9 +113,9 @@ //! # fn main() -> Result<(), log::kv::Error> { //! use log::kv::{self, Source, Key, value::Visitor}; //! # let record = log::Record::builder().key_values(&[("a", 1)]).build(); -//! +//! //! struct IsNumeric(bool); -//! +//! //! impl<'kvs> Visitor<'kvs> for IsNumeric { //! fn visit_any(&mut self, _value: kv::Value) -> Result<(), kv::Error> { //! self.0 = false; @@ -143,23 +147,24 @@ //! Ok(()) //! } //! } -//! -//! // info!("Something of interest"; a = 1); +//! +//! // info!(a = 1; "Something of interest"); +//! //! let a = record.key_values().get(Key::from("a")).unwrap(); -//! +//! //! let mut visitor = IsNumeric(false); -//! +//! //! a.visit(&mut visitor)?; -//! +//! //! let is_numeric = visitor.0; -//! +//! //! assert!(is_numeric); //! # Ok(()) //! # } //! ``` //! //! To serialize a value to a format like JSON, you can also use either `serde` or `sval`: -//! +//! //! ``` //! # fn main() -> Result<(), Box> { //! # #[cfg(feature = "serde")] @@ -169,16 +174,17 @@ //! let data = Data { a: 1, b: true, c: "Some data" }; //! # let source = [("a", log::kv::Value::from_serde(&data))]; //! # let record = log::Record::builder().key_values(&source).build(); -//! -//! // info!("Something of interest"; a = data); +//! +//! // info!(a = data; "Something of interest"); +//! //! let a = record.key_values().get(Key::from("a")).unwrap(); -//! +//! //! assert_eq!("{\"a\":1,\"b\":true,\"c\":\"Some data\"}", serde_json::to_string(&a)?); //! # } //! # Ok(()) //! # } //! ``` -//! +//! //! The choice of serialization framework depends on the needs of the consumer. //! If you're in a no-std environment, you can use `sval`. In other cases, you can use `serde`. //! Log producers and log consumers don't need to agree on the serialization framework. @@ -187,17 +193,18 @@ //! //! Values can also always be formatted using the standard `Debug` and `Display` //! traits: -//! +//! //! ``` //! # use log::kv::Key; //! # #[derive(Debug)] struct Data { a: i32, b: bool, c: &'static str } //! let data = Data { a: 1, b: true, c: "Some data" }; //! # let source = [("a", log::kv::Value::from_debug(&data))]; //! # let record = log::Record::builder().key_values(&source).build(); -//! -//! // info!("Something of interest"; a = data); +//! +//! // info!(a = data; "Something of interest"); +//! //! let a = record.key_values().get(Key::from("a")).unwrap(); -//! +//! //! assert_eq!("Data { a: 1, b: true, c: \"Some data\" }", format!("{a:?}")); //! ``` diff --git a/src/kv/source.rs b/src/kv/source.rs index 69544162d..87d955417 100644 --- a/src/kv/source.rs +++ b/src/kv/source.rs @@ -1,5 +1,5 @@ //! Sources for key-values. -//! +//! //! This module defines the [`Source`] type and supporting APIs for //! working with collections of key-values. @@ -11,7 +11,7 @@ use std::fmt; /// The source may be a single pair, a set of pairs, or a filter over a set of pairs. /// Use the [`Visitor`](trait.Visitor.html) trait to inspect the structured data /// in a source. -/// +/// /// A source is like an iterator over its key-values, except with a push-based API /// instead of a pull-based one. /// @@ -22,19 +22,19 @@ use std::fmt; /// ``` /// # fn main() -> Result<(), log::kv::Error> { /// use log::kv::{self, Source, Key, Value, source::Visitor}; -/// +/// /// // A `Visitor` that prints all key-values /// // Visitors are fed the key-value pairs of each key-values /// struct Printer; -/// +/// /// impl<'kvs> Visitor<'kvs> for Printer { /// fn visit_pair(&mut self, key: Key<'kvs>, value: Value<'kvs>) -> Result<(), kv::Error> { /// println!("{key}: {value}"); -/// +/// /// Ok(()) /// } /// } -/// +/// /// // A source with 3 key-values /// // Common collection types implement the `Source` trait /// let source = &[ @@ -42,7 +42,7 @@ use std::fmt; /// ("b", 2), /// ("c", 3), /// ]; -/// +/// /// // Pass an instance of the `Visitor` to a `Source` to visit it /// source.visit(&mut Printer)?; /// # Ok(()) diff --git a/src/kv/value.rs b/src/kv/value.rs index 1abc01bde..c8cdf6dbb 100644 --- a/src/kv/value.rs +++ b/src/kv/value.rs @@ -1,5 +1,5 @@ //! Structured values. -//! +//! //! This module defines the [`Value`] type and supporting APIs for //! capturing and serializing them. @@ -45,7 +45,6 @@ impl<'v> ToValue for Value<'v> { /// ## Using the `Value::from_*` methods /// /// `Value` offers a few constructor methods that capture values of different kinds. -/// These methods don't require `T: 'static`, but can't support downcasting. /// /// ``` /// use log::kv::Value; @@ -79,25 +78,43 @@ impl<'v> ToValue for Value<'v> { /// assert_eq!(Some(42), value.to_i64()); /// ``` /// +/// # Data model +/// +/// Values can hold one of a number of types: +/// +/// - **Null:** The absence of any other meaningful value. Note that +/// `Some(Value::null())` is not the same as `None`. The former is +/// `null` while the latter is `undefined`. This is important to be +/// able to tell the difference between a key-value that was logged, +/// but its value was empty (`Some(Value::null())`) and a key-value +/// that was never logged at all (`None`). +/// - **Strings:** `str`, `char`. +/// - **Booleans:** `bool`. +/// - **Integers:** `u8`-`u128`, `i8`-`i128`, `NonZero*`. +/// - **Floating point numbers:** `f32`-`f64`. +/// - **Errors:** `dyn (Error + 'static)`. +/// - **`serde`:** Any type in `serde`'s data model. +/// - **`sval`:** Any type in `sval`'s data model. +/// /// # Serialization /// -/// `Value` provides a number of ways to be serialized. +/// Values provide a number of ways to be serialized. /// /// For basic types the [`Value::visit`] method can be used to extract the /// underlying typed value. However this is limited in the amount of types -/// supported (see the [`Visit`] trait methods). +/// supported (see the [`Visitor`] trait methods). /// /// For more complex types one of the following traits can be used: -/// * [`sval::Value`], requires the `kv_unstable_sval` feature. -/// * [`serde::Serialize`], requires the `kv_unstable_serde` feature. -/// -/// You don't need a [`Visit`] to serialize values. -/// +/// * `sval::Value`, requires the `kv_unstable_sval` feature. +/// * `serde::Serialize`, requires the `kv_unstable_serde` feature. +/// +/// You don't need a visitor to serialize values through `serde` or `sval`. +/// /// A value can always be serialized using any supported framework, regardless /// of how it was captured. If, for example, a value was captured using its -/// `Display` implementation, it will serialize as a string. If it was captured -/// as a struct using its `serde::Serialize`, it will also serialize as a struct -/// through `sval`, or be formatted using a `Debug`-compatible representation. +/// `Display` implementation, it will serialize through `serde` as a string. If it was +/// captured as a struct using `serde`, it will also serialize as a struct +/// through `sval`, or can be formatted using a `Debug`-compatible representation. pub struct Value<'v> { inner: inner::Inner<'v>, } @@ -175,6 +192,13 @@ impl<'v> Value<'v> { } } + /// Get a `null` value. + pub fn null() -> Self { + Value { + inner: inner::Inner::empty(), + } + } + /// Get a value from an internal primitive. fn from_inner(value: T) -> Self where @@ -458,15 +482,22 @@ mod std_support { } } -/// A visitor for a `Value`. +/// A visitor for a [`Value`]. /// -/// Also see [`Value`'s documentation on seralization]. +/// Also see [`Value`'s documentation on seralization]. Visitors are a simple alternative +/// to a more fully-featured serialization framework like `serde` or `sval`. A visitor +/// can differentiate primitive types through methods like [`Visitor::visit_bool`] and +/// [`Visitor::visit_str`], but more complex types like maps and sequences +/// will fallthrough to [`Visitor::visit_any`]. +/// +/// If you're trying to serialize a value to a format like JSON, you can use either `serde` +/// or `sval` directly with the value. You don't need a visitor. /// /// [`Value`'s documentation on seralization]: Value#serialization pub trait Visitor<'v> { /// Visit a `Value`. /// - /// This is the only required method on `Visit` and acts as a fallback for any + /// This is the only required method on `Visitor` and acts as a fallback for any /// more specific methods that aren't overridden. /// The `Value` may be formatted using its `fmt::Debug` or `fmt::Display` implementation, /// or serialized using its `sval::Value` or `serde::Serialize` implementation. @@ -622,39 +653,57 @@ pub(in crate::kv) mod inner { } fn visit_u64(&mut self, value: u64) -> Result<(), Error> { - self.0.visit_u64(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_u64(value) + .map_err(crate::kv::Error::into_value) } fn visit_i64(&mut self, value: i64) -> Result<(), Error> { - self.0.visit_i64(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_i64(value) + .map_err(crate::kv::Error::into_value) } fn visit_u128(&mut self, value: u128) -> Result<(), Error> { - self.0.visit_u128(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_u128(value) + .map_err(crate::kv::Error::into_value) } fn visit_i128(&mut self, value: i128) -> Result<(), Error> { - self.0.visit_i128(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_i128(value) + .map_err(crate::kv::Error::into_value) } fn visit_f64(&mut self, value: f64) -> Result<(), Error> { - self.0.visit_f64(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_f64(value) + .map_err(crate::kv::Error::into_value) } fn visit_bool(&mut self, value: bool) -> Result<(), Error> { - self.0.visit_bool(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_bool(value) + .map_err(crate::kv::Error::into_value) } fn visit_str(&mut self, value: &str) -> Result<(), Error> { - self.0.visit_str(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_str(value) + .map_err(crate::kv::Error::into_value) } fn visit_borrowed_str(&mut self, value: &'v str) -> Result<(), Error> { - self.0.visit_borrowed_str(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_borrowed_str(value) + .map_err(crate::kv::Error::into_value) } fn visit_char(&mut self, value: char) -> Result<(), Error> { - self.0.visit_char(value).map_err(crate::kv::Error::into_value) + self.0 + .visit_char(value) + .map_err(crate::kv::Error::into_value) } #[cfg(feature = "kv_unstable_std")] @@ -662,7 +711,9 @@ pub(in crate::kv) mod inner { &mut self, err: &(dyn std::error::Error + 'static), ) -> Result<(), Error> { - self.0.visit_error(err).map_err(crate::kv::Error::into_value) + self.0 + .visit_error(err) + .map_err(crate::kv::Error::into_value) } #[cfg(feature = "kv_unstable_std")] @@ -670,7 +721,9 @@ pub(in crate::kv) mod inner { &mut self, err: &'v (dyn std::error::Error + 'static), ) -> Result<(), Error> { - self.0.visit_borrowed_error(err).map_err(crate::kv::Error::into_value) + self.0 + .visit_borrowed_error(err) + .map_err(crate::kv::Error::into_value) } } @@ -826,6 +879,10 @@ pub(in crate::kv) mod inner { todo!() } + pub fn empty() -> Self { + todo!() + } + pub fn to_bool(&self) -> Option { todo!() } @@ -873,7 +930,6 @@ pub(in crate::kv) mod inner { F64(f64), I64(i64), U64(u64), - } #[derive(Debug)] diff --git a/src/lib.rs b/src/lib.rs index 6a8e57762..86f3f8ec2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -121,7 +121,7 @@ //! # #[cfg(not(feature = "kv_unstable_serde"))] //! # fn main() {} //! ``` -//! +//! //! See the [`kv`] module documentation for more details. //! //! # Available logging implementations From 54c34f7fbd330c3dffd10addd75480e527a0bae5 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Tue, 30 Jan 2024 20:35:35 +1000 Subject: [PATCH 2/5] start filling in no-dependency value --- Cargo.toml | 2 +- src/kv/value.rs | 244 ++++++++++++++++++++++++++++++------------------ 2 files changed, 152 insertions(+), 94 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ef0e3eb7c..690f25cee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,7 @@ kv_unstable_serde = ["kv_unstable_std", "value-bag/serde", "serde"] serde = { version = "1.0", optional = true, default-features = false } sval = { version = "2.1", optional = true, default-features = false } sval_ref = { version = "2.1", optional = true, default-features = false } -value-bag = { version = "1.4", optional = true, default-features = false } +value-bag = { version = "1.4", optional = true, default-features = false, features = ["inline-i128"] } [dev-dependencies] serde = { version = "1.0", features = ["derive"] } diff --git a/src/kv/value.rs b/src/kv/value.rs index c8cdf6dbb..f5f4fc057 100644 --- a/src/kv/value.rs +++ b/src/kv/value.rs @@ -260,62 +260,12 @@ impl ToValue for str { } } -impl ToValue for u128 { - fn to_value(&self) -> Value { - Value::from(self) - } -} - -impl ToValue for i128 { - fn to_value(&self) -> Value { - Value::from(self) - } -} - -impl ToValue for std::num::NonZeroU128 { - fn to_value(&self) -> Value { - Value::from(self) - } -} - -impl ToValue for std::num::NonZeroI128 { - fn to_value(&self) -> Value { - Value::from(self) - } -} - impl<'v> From<&'v str> for Value<'v> { fn from(value: &'v str) -> Self { Value::from_inner(value) } } -impl<'v> From<&'v u128> for Value<'v> { - fn from(value: &'v u128) -> Self { - Value::from_inner(value) - } -} - -impl<'v> From<&'v i128> for Value<'v> { - fn from(value: &'v i128) -> Self { - Value::from_inner(value) - } -} - -impl<'v> From<&'v std::num::NonZeroU128> for Value<'v> { - fn from(v: &'v std::num::NonZeroU128) -> Value<'v> { - // SAFETY: `NonZeroU128` and `u128` have the same ABI - Value::from_inner(unsafe { &*(v as *const std::num::NonZeroU128 as *const u128) }) - } -} - -impl<'v> From<&'v std::num::NonZeroI128> for Value<'v> { - fn from(v: &'v std::num::NonZeroI128) -> Value<'v> { - // SAFETY: `NonZeroI128` and `i128` have the same ABI - Value::from_inner(unsafe { &*(v as *const std::num::NonZeroI128 as *const i128) }) - } -} - impl ToValue for () { fn to_value(&self) -> Value { Value::from_inner(()) @@ -383,12 +333,12 @@ macro_rules! impl_value_to_primitive { } } -impl_to_value_primitive![usize, u8, u16, u32, u64, isize, i8, i16, i32, i64, f32, f64, char, bool,]; +impl_to_value_primitive![usize, u8, u16, u32, u64, u128, isize, i8, i16, i32, i64, i128, f32, f64, char, bool,]; #[rustfmt::skip] impl_to_value_nonzero_primitive![ - NonZeroUsize, NonZeroU8, NonZeroU16, NonZeroU32, NonZeroU64, - NonZeroIsize, NonZeroI8, NonZeroI16, NonZeroI32, NonZeroI64, + NonZeroUsize, NonZeroU8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU128, + NonZeroIsize, NonZeroI8, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI128, ]; impl_value_to_primitive![ @@ -503,6 +453,11 @@ pub trait Visitor<'v> { /// or serialized using its `sval::Value` or `serde::Serialize` implementation. fn visit_any(&mut self, value: Value) -> Result<(), Error>; + /// Visit an empty value. + fn visit_null(&mut self) -> Result<(), Error> { + self.visit_any(Value::null()) + } + /// Visit an unsigned integer. fn visit_u64(&mut self, value: u64) -> Result<(), Error> { self.visit_any(value.into()) @@ -515,12 +470,12 @@ pub trait Visitor<'v> { /// Visit a big unsigned integer. fn visit_u128(&mut self, value: u128) -> Result<(), Error> { - self.visit_any((&value).into()) + self.visit_any((value).into()) } /// Visit a big signed integer. fn visit_i128(&mut self, value: i128) -> Result<(), Error> { - self.visit_any((&value).into()) + self.visit_any((value).into()) } /// Visit a floating point. @@ -573,6 +528,10 @@ where (**self).visit_any(value) } + fn visit_null(&mut self) -> Result<(), Error> { + (**self).visit_null() + } + fn visit_u64(&mut self, value: u64) -> Result<(), Error> { (**self).visit_u64(value) } @@ -739,188 +698,287 @@ pub(in crate::kv) mod inner { #[derive(Clone)] pub enum Inner<'v> { + None, + Bool(bool), Str(&'v str), + Char(char), + I64(i64), + U64(u64), + F64(f64), + I128(i128), + U128(u128), + Debug(&'v dyn fmt::Debug), + Display(&'v dyn fmt::Display), } impl<'v> From<()> for Inner<'v> { fn from(v: ()) -> Self { - todo!() + Inner::None } } impl<'v> From for Inner<'v> { fn from(v: bool) -> Self { - todo!() + Inner::Bool(v) } } impl<'v> From for Inner<'v> { fn from(v: char) -> Self { - todo!() + Inner::Char(v) } } impl<'v> From for Inner<'v> { fn from(v: f32) -> Self { - todo!() + Inner::F64(v as f64) } } impl<'v> From for Inner<'v> { fn from(v: f64) -> Self { - todo!() + Inner::F64(v) } } impl<'v> From for Inner<'v> { fn from(v: i8) -> Self { - todo!() + Inner::I64(v as i64) } } impl<'v> From for Inner<'v> { fn from(v: i16) -> Self { - todo!() + Inner::I64(v as i64) } } impl<'v> From for Inner<'v> { fn from(v: i32) -> Self { - todo!() + Inner::I64(v as i64) } } impl<'v> From for Inner<'v> { fn from(v: i64) -> Self { - todo!() + Inner::I64(v as i64) } } impl<'v> From for Inner<'v> { fn from(v: isize) -> Self { - todo!() + Inner::I64(v as i64) } } impl<'v> From for Inner<'v> { fn from(v: u8) -> Self { - todo!() + Inner::U64(v as u64) } } impl<'v> From for Inner<'v> { fn from(v: u16) -> Self { - todo!() + Inner::U64(v as u64) } } impl<'v> From for Inner<'v> { fn from(v: u32) -> Self { - todo!() + Inner::U64(v as u64) } } impl<'v> From for Inner<'v> { fn from(v: u64) -> Self { - todo!() + Inner::U64(v as u64) } } impl<'v> From for Inner<'v> { fn from(v: usize) -> Self { - todo!() + Inner::U64(v as u64) } } - impl<'v> From<&'v i128> for Inner<'v> { - fn from(v: &'v i128) -> Self { - todo!() + impl<'v> From for Inner<'v> { + fn from(v: i128) -> Self { + Inner::I128(v) } } - impl<'v> From<&'v u128> for Inner<'v> { - fn from(v: &'v u128) -> Self { - todo!() + impl<'v> From for Inner<'v> { + fn from(v: u128) -> Self { + Inner::U128(v) } } impl<'v> From<&'v str> for Inner<'v> { fn from(v: &'v str) -> Self { - todo!() + Inner::Str(v) } } impl<'v> fmt::Debug for Inner<'v> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - todo!() + match self { + Inner::None => fmt::Debug::fmt(&None::<()>, f), + Inner::Bool(v) => fmt::Debug::fmt(v, f), + Inner::Str(v) => fmt::Debug::fmt(v, f), + Inner::Char(v) => fmt::Debug::fmt(v, f), + Inner::I64(v) => fmt::Debug::fmt(v, f), + Inner::U64(v) => fmt::Debug::fmt(v, f), + Inner::F64(v) => fmt::Debug::fmt(v, f), + Inner::I128(v) => fmt::Debug::fmt(v, f), + Inner::U128(v) => fmt::Debug::fmt(v, f), + Inner::Debug(v) => fmt::Debug::fmt(v, f), + Inner::Display(v) => fmt::Display::fmt(v, f), + } } } impl<'v> fmt::Display for Inner<'v> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - todo!() + match self { + Inner::None => fmt::Debug::fmt(&None::<()>, f), + Inner::Bool(v) => fmt::Display::fmt(v, f), + Inner::Str(v) => fmt::Display::fmt(v, f), + Inner::Char(v) => fmt::Display::fmt(v, f), + Inner::I64(v) => fmt::Display::fmt(v, f), + Inner::U64(v) => fmt::Display::fmt(v, f), + Inner::F64(v) => fmt::Display::fmt(v, f), + Inner::I128(v) => fmt::Display::fmt(v, f), + Inner::U128(v) => fmt::Display::fmt(v, f), + Inner::Debug(v) => fmt::Debug::fmt(v, f), + Inner::Display(v) => fmt::Display::fmt(v, f), + } } } impl<'v> Inner<'v> { pub fn from_debug(value: &'v T) -> Self { - todo!() + Inner::Debug(value) } pub fn from_display(value: &'v T) -> Self { - todo!() + Inner::Display(value) } pub fn from_dyn_debug(value: &'v dyn fmt::Debug) -> Self { - todo!() + Inner::Debug(value) } pub fn from_dyn_display(value: &'v dyn fmt::Display) -> Self { - todo!() + Inner::Display(value) } pub fn empty() -> Self { - todo!() + Inner::None } pub fn to_bool(&self) -> Option { - todo!() + match self { + Inner::Bool(v) => Some(*v), + _ => None, + } } pub fn to_char(&self) -> Option { - todo!() + match self { + Inner::Char(v) => Some(*v), + _ => None, + } } pub fn to_f64(&self) -> Option { - todo!() + match self { + Inner::F64(v) => Some(*v), + Inner::I64(v) => { + let v: i32 = (*v).try_into().ok()?; + v.try_into().ok() + }, + Inner::U64(v) => { + let v: u32 = (*v).try_into().ok()?; + v.try_into().ok() + }, + Inner::I128(v) => { + let v: i32 = (*v).try_into().ok()?; + v.try_into().ok() + }, + Inner::U128(v) => { + let v: u32 = (*v).try_into().ok()?; + v.try_into().ok() + }, + _ => None, + } } pub fn to_i64(&self) -> Option { - todo!() + match self { + Inner::I64(v) => Some(*v), + Inner::U64(v) => (*v).try_into().ok(), + Inner::I128(v) => (*v).try_into().ok(), + Inner::U128(v) => (*v).try_into().ok(), + _ => None, + } } pub fn to_u64(&self) -> Option { - todo!() + match self { + Inner::U64(v) => Some(*v), + Inner::I64(v) => (*v).try_into().ok(), + Inner::I128(v) => (*v).try_into().ok(), + Inner::U128(v) => (*v).try_into().ok(), + _ => None, + } } pub fn to_u128(&self) -> Option { - todo!() + match self { + Inner::U128(v) => Some(*v), + Inner::I64(v) => (*v).try_into().ok(), + Inner::U64(v) => (*v).try_into().ok(), + Inner::I128(v) => (*v).try_into().ok(), + _ => None, + } } pub fn to_i128(&self) -> Option { - todo!() + match self { + Inner::I128(v) => Some(*v), + Inner::I64(v) => (*v).try_into().ok(), + Inner::U64(v) => (*v).try_into().ok(), + Inner::U128(v) => (*v).try_into().ok(), + _ => None, + } } pub fn to_borrowed_str(&self) -> Option<&'v str> { - todo!() + match self { + Inner::Str(v) => Some(v), + _ => None, + } } #[cfg(test)] pub fn to_test_token(&self) -> Token { - todo!() + match self { + Inner::None => Token::None, + Inner::Bool(v) => Token::Bool(*v), + Inner::Str(v) => Token::Str(*v), + Inner::Char(v) => Token::Char(*v), + Inner::I64(v) => Token::I64(*v), + Inner::U64(v) => Token::U64(*v), + Inner::F64(v) => Token::F64(*v), + Inner::I128(v) => unimplemented!(), + Inner::U128(v) => unimplemented!(), + Inner::Debug(v) => unimplemented!(), + Inner::Display(v) => unimplemented!(), + } } } + #[cfg(test)] #[derive(Debug, PartialEq)] pub enum Token<'v> { None, From 05d7bed9e405d83e417d1748e03888976974ef4c Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 31 Jan 2024 10:42:32 +1000 Subject: [PATCH 3/5] fill in visitor for inline value --- Cargo.toml | 4 ++-- src/kv/value.rs | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 690f25cee..c30944c56 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,7 @@ kv_unstable_serde = ["kv_unstable_std", "value-bag/serde", "serde"] serde = { version = "1.0", optional = true, default-features = false } sval = { version = "2.1", optional = true, default-features = false } sval_ref = { version = "2.1", optional = true, default-features = false } -value-bag = { version = "1.4", optional = true, default-features = false, features = ["inline-i128"] } +value-bag = { version = "1.7", optional = true, default-features = false, features = ["inline-i128"] } [dev-dependencies] serde = { version = "1.0", features = ["derive"] } @@ -65,7 +65,7 @@ serde_json = "1.0" serde_test = "1.0" sval = { version = "2.1" } sval_derive = { version = "2.1" } -value-bag = { version = "1.4", features = ["test"] } +value-bag = { version = "1.7", features = ["test"] } # NOTE: log doesn't actually depent on this crate. However our dependencies, # serde and sval, dependent on version 1.0 of the crate, which has problem fixed diff --git a/src/kv/value.rs b/src/kv/value.rs index f5f4fc057..2a16f192c 100644 --- a/src/kv/value.rs +++ b/src/kv/value.rs @@ -611,6 +611,10 @@ pub(in crate::kv) mod inner { .map_err(crate::kv::Error::into_value) } + fn visit_empty(&mut self) -> Result<(), Error> { + self.0.visit_null().map_err(crate::kv::Error::into_value) + } + fn visit_u64(&mut self, value: u64) -> Result<(), Error> { self.0 .visit_u64(value) @@ -1005,8 +1009,20 @@ pub(in crate::kv) mod inner { } } - pub fn visit<'v>(inner: &Inner<'v>, visitor: impl Visitor<'v>) -> Result<(), crate::kv::Error> { - todo!() + pub fn visit<'v>(inner: &Inner<'v>, mut visitor: impl Visitor<'v>) -> Result<(), crate::kv::Error> { + match inner { + Inner::None => visitor.visit_null(), + Inner::Bool(v) => visitor.visit_bool(*v), + Inner::Str(v) => visitor.visit_borrowed_str(*v), + Inner::Char(v) => visitor.visit_char(*v), + Inner::I64(v) => visitor.visit_i64(*v), + Inner::U64(v) => visitor.visit_u64(*v), + Inner::F64(v) => visitor.visit_f64(*v), + Inner::I128(v) => visitor.visit_i128(*v), + Inner::U128(v) => visitor.visit_u128(*v), + Inner::Debug(v) => visitor.visit_any(Value::from_dyn_debug(*v)), + Inner::Display(v) => visitor.visit_any(Value::from_dyn_display(*v)), + } } } From e711b62c7efd2bb1a2cdb48160bf65b9ae0f3355 Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 31 Jan 2024 12:28:02 +1000 Subject: [PATCH 4/5] fix up some warnings --- src/kv/error.rs | 20 +++++++++++--------- src/kv/value.rs | 17 +---------------- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/kv/error.rs b/src/kv/error.rs index a6f48d0c6..d654be780 100644 --- a/src/kv/error.rs +++ b/src/kv/error.rs @@ -1,7 +1,5 @@ use std::fmt; -use crate::kv::value; - /// An error encountered while working with structured data. #[derive(Debug)] pub struct Error { @@ -13,7 +11,8 @@ enum Inner { #[cfg(feature = "std")] Boxed(std_support::BoxedError), Msg(&'static str), - Value(value::inner::Error), + #[cfg(feature = "value-bag")] + Value(crate::kv::value::inner::Error), Fmt, } @@ -25,21 +24,23 @@ impl Error { } } - // Not public so we don't leak the `value::inner` API - pub(super) fn from_value(err: value::inner::Error) -> Self { + // Not public so we don't leak the `crate::kv::value::inner` API + #[cfg(feature = "value-bag")] + pub(super) fn from_value(err: crate::kv::value::inner::Error) -> Self { Error { inner: Inner::Value(err), } } - // Not public so we don't leak the `value::inner` API - pub(super) fn into_value(self) -> value::inner::Error { + // Not public so we don't leak the `crate::kv::value::inner` API + #[cfg(feature = "value-bag")] + pub(super) fn into_value(self) -> crate::kv::value::inner::Error { match self.inner { Inner::Value(err) => err, #[cfg(feature = "kv_unstable_std")] - _ => value::inner::Error::boxed(self), + _ => crate::kv::value::inner::Error::boxed(self), #[cfg(not(feature = "kv_unstable_std"))] - _ => value::inner::Error::msg("error inspecting a value"), + _ => crate::kv::value::inner::Error::msg("error inspecting a value"), } } } @@ -50,6 +51,7 @@ impl fmt::Display for Error { match &self.inner { #[cfg(feature = "std")] Boxed(err) => err.fmt(f), + #[cfg(feature = "value-bag")] Value(err) => err.fmt(f), Msg(msg) => msg.fmt(f), Fmt => fmt::Error.fmt(f), diff --git a/src/kv/value.rs b/src/kv/value.rs index 2a16f192c..60828a27a 100644 --- a/src/kv/value.rs +++ b/src/kv/value.rs @@ -716,7 +716,7 @@ pub(in crate::kv) mod inner { } impl<'v> From<()> for Inner<'v> { - fn from(v: ()) -> Self { + fn from(_: ()) -> Self { Inner::None } } @@ -994,21 +994,6 @@ pub(in crate::kv) mod inner { U64(u64), } - #[derive(Debug)] - pub struct Error {} - - impl Error { - pub fn msg(msg: &'static str) -> Self { - todo!() - } - } - - impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - todo!() - } - } - pub fn visit<'v>(inner: &Inner<'v>, mut visitor: impl Visitor<'v>) -> Result<(), crate::kv::Error> { match inner { Inner::None => visitor.visit_null(), From 67885e05f630243938667319f684d890f146703e Mon Sep 17 00:00:00 2001 From: KodrAus Date: Wed, 31 Jan 2024 13:07:59 +1000 Subject: [PATCH 5/5] add some docs on how Value is implemented --- src/kv/value.rs | 55 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/src/kv/value.rs b/src/kv/value.rs index 60828a27a..971bf6bbf 100644 --- a/src/kv/value.rs +++ b/src/kv/value.rs @@ -333,7 +333,9 @@ macro_rules! impl_value_to_primitive { } } -impl_to_value_primitive![usize, u8, u16, u32, u64, u128, isize, i8, i16, i32, i64, i128, f32, f64, char, bool,]; +impl_to_value_primitive![ + usize, u8, u16, u32, u64, u128, isize, i8, i16, i32, i64, i128, f32, f64, char, bool, +]; #[rustfmt::skip] impl_to_value_nonzero_primitive![ @@ -584,6 +586,13 @@ where #[cfg(feature = "value-bag")] pub(in crate::kv) mod inner { + /** + An implementation of `Value` based on a library called `value_bag`. + + `value_bag` was written specifically for use in `log`'s value, but was split out when it outgrew + the codebase here. It's a general-purpose type-erasure library that handles mapping between + more fully-featured serialization frameworks. + */ use super::*; pub use value_bag::ValueBag as Inner; @@ -698,6 +707,19 @@ pub(in crate::kv) mod inner { #[cfg(not(feature = "value-bag"))] pub(in crate::kv) mod inner { + /** + This is a dependency-free implementation of `Value` when there's no serialization frameworks involved. + In these simple cases a more fully featured solution like `value_bag` isn't needed, so we avoid pulling it in. + + There are a few things here that need to remain consistent with the `value_bag`-based implementation: + + 1. Conversions should always produce the same results. If a conversion here returns `Some`, then + the same `value_bag`-based conversion must also. Of particular note here are floats to ints; they're + based on the standard library's `TryInto` conversions, which need to be convert to `i32` or `u32`, + and then to `f64`. + 2. Visitors should always be called in the same way. If a particular type of value calls `visit_i64`, + then the same `value_bag`-based visitor must also. + */ use super::*; #[derive(Clone)] @@ -900,19 +922,19 @@ pub(in crate::kv) mod inner { Inner::I64(v) => { let v: i32 = (*v).try_into().ok()?; v.try_into().ok() - }, + } Inner::U64(v) => { let v: u32 = (*v).try_into().ok()?; v.try_into().ok() - }, + } Inner::I128(v) => { let v: i32 = (*v).try_into().ok()?; v.try_into().ok() - }, + } Inner::U128(v) => { let v: u32 = (*v).try_into().ok()?; v.try_into().ok() - }, + } _ => None, } } @@ -974,10 +996,10 @@ pub(in crate::kv) mod inner { Inner::I64(v) => Token::I64(*v), Inner::U64(v) => Token::U64(*v), Inner::F64(v) => Token::F64(*v), - Inner::I128(v) => unimplemented!(), - Inner::U128(v) => unimplemented!(), - Inner::Debug(v) => unimplemented!(), - Inner::Display(v) => unimplemented!(), + Inner::I128(_) => unimplemented!(), + Inner::U128(_) => unimplemented!(), + Inner::Debug(_) => unimplemented!(), + Inner::Display(_) => unimplemented!(), } } } @@ -994,7 +1016,10 @@ pub(in crate::kv) mod inner { U64(u64), } - pub fn visit<'v>(inner: &Inner<'v>, mut visitor: impl Visitor<'v>) -> Result<(), crate::kv::Error> { + pub fn visit<'v>( + inner: &Inner<'v>, + mut visitor: impl Visitor<'v>, + ) -> Result<(), crate::kv::Error> { match inner { Inner::None => visitor.visit_null(), Inner::Bool(v) => visitor.visit_bool(*v), @@ -1120,6 +1145,16 @@ pub(crate) mod tests { } } + #[test] + fn test_to_float() { + // Only integers from i32::MIN..=u32::MAX can be converted into floats + assert!(Value::from(i32::MIN).to_f64().is_some()); + assert!(Value::from(u32::MAX).to_f64().is_some()); + + assert!(Value::from((i32::MIN as i64) - 1).to_f64().is_none()); + assert!(Value::from((u32::MAX as u64) + 1).to_f64().is_none()); + } + #[test] fn test_to_cow_str() { for v in str() {