-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deer
implement value deserializers
#1947
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
13657e9
feat: create value deserializer
indietyp 395ef96
Merge branch 'main' into bm/deer/value-deserializer
indietyp 0434de1
feat: `mod.rs` -> `module.rs`
indietyp 9e6aa8e
feat: implement `Number`, and string
indietyp 7fa81f2
feat: implement `Bytes`
indietyp 6b37afc
feat: `ObjectAccessDeserializer` + `ArrayAccessDeserializer`
indietyp b39d91a
fix: compile errors
indietyp 058f39b
test: most deserializers
indietyp 9a0c4fb
test: null + number
indietyp 293b082
test: bytes
indietyp abe0f47
fix: clippy
indietyp b2e6356
fix: miri
indietyp 4672ae9
test: `IntoDeserializer` trait
indietyp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
// Adapted from serde | ||
|
||
#[macro_export(local_inner_macros)] | ||
macro_rules! forward_to_deserialize_any { | ||
(<$visitor:ident: Visitor<$lifetime:tt>> $($func:ident)*) => { | ||
$(forward_to_deserialize_any_helper!{$func<$lifetime, $visitor>})* | ||
}; | ||
// This case must be after the previous one. | ||
($($func:ident)*) => { | ||
$(forward_to_deserialize_any_helper!{$func<'de, V>})* | ||
}; | ||
} | ||
|
||
#[doc(hidden)] | ||
#[macro_export] | ||
macro_rules! forward_to_deserialize_any_method { | ||
($func:ident < $l:tt, $v:ident > ()) => { | ||
#[inline] | ||
fn $func<$v>(self, visitor: $v) -> error_stack::Result<$v::Value, $crate::DeserializerError> | ||
where | ||
$v: $crate::Visitor<$l>, | ||
{ | ||
self.deserialize_any(visitor) | ||
} | ||
}; | ||
} | ||
|
||
#[doc(hidden)] | ||
#[macro_export(local_inner_macros)] | ||
macro_rules! forward_to_deserialize_any_helper { | ||
(bool < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_bool<$l, $v>()} | ||
}; | ||
(i8 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_i8<$l, $v>()} | ||
}; | ||
(i16 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_i16<$l, $v>()} | ||
}; | ||
(i32 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_i32<$l, $v>()} | ||
}; | ||
(i64 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_i64<$l, $v>()} | ||
}; | ||
(i128 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_i128<$l, $v>()} | ||
}; | ||
(isize < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_isize<$l, $v>()} | ||
}; | ||
(u8 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_u8<$l, $v>()} | ||
}; | ||
(u16 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_u16<$l, $v>()} | ||
}; | ||
(u32 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_u32<$l, $v>()} | ||
}; | ||
(u64 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_u64<$l, $v>()} | ||
}; | ||
(u128 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_u128<$l, $v>()} | ||
}; | ||
(usize < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_usize<$l, $v>()} | ||
}; | ||
(f32 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_f32<$l, $v>()} | ||
}; | ||
(f64 < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_f64<$l, $v>()} | ||
}; | ||
(char < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_char<$l, $v>()} | ||
}; | ||
(str < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_str<$l, $v>()} | ||
}; | ||
(string < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_string<$l, $v>()} | ||
}; | ||
(bytes < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_bytes<$l, $v>()} | ||
}; | ||
(bytes_buffer < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_bytes_buffer<$l, $v>()} | ||
}; | ||
(number < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_number<$l, $v>()} | ||
}; | ||
(null < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_null<$l, $v>()} | ||
}; | ||
(object < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_object<$l, $v>()} | ||
}; | ||
(array < $l:tt, $v:ident >) => { | ||
forward_to_deserialize_any_method! {deserialize_array<$l, $v>()} | ||
}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,195 @@ | ||
mod array; | ||
mod bytes; | ||
mod object; | ||
mod string; | ||
|
||
pub use array::ArrayAccessDeserializer; | ||
pub use bytes::{BorrowedBytesDeserializer, BytesBufferDeserializer, BytesDeserializer}; | ||
use error_stack::ResultExt; | ||
pub use object::ObjectAccessDeserializer; | ||
pub use string::{BorrowedStrDeserializer, StrDeserializer, StringDeserializer}; | ||
|
||
use crate::{error::DeserializerError, Context, Deserializer, Number, Visitor}; | ||
|
||
macro_rules! impl_owned { | ||
(@INTERNAL COPY, $ty:ty, $name:ident, $method:ident) => { | ||
#[derive(Debug, Copy, Clone)] | ||
pub struct $name<'a> { | ||
context: &'a Context, | ||
value: $ty | ||
} | ||
}; | ||
|
||
(@INTERNAL CLONE, $ty:ty, $name:ident, $method:ident) => { | ||
#[derive(Debug, Clone)] | ||
pub struct $name<'a> { | ||
context: &'a Context, | ||
value: $ty | ||
} | ||
}; | ||
|
||
(@INTERNAL IMPL, $ty:ty, $name:ident, $method:ident) => { | ||
impl<'a> $name<'a> { | ||
#[must_use] | ||
pub const fn new(value: $ty, context: &'a Context) -> Self { | ||
Self { value, context } | ||
} | ||
} | ||
|
||
impl<'de, 'a> Deserializer<'de> for $name<'a> { | ||
forward_to_deserialize_any!( | ||
null | ||
bool | ||
number | ||
i8 i16 i32 i64 i128 isize | ||
u8 u16 u32 u64 u128 usize | ||
f32 f64 | ||
char str string | ||
bytes bytes_buffer | ||
array object | ||
); | ||
|
||
fn context(&self) -> &Context { | ||
self.context | ||
} | ||
|
||
fn deserialize_any<V>(self, visitor: V) -> error_stack::Result<V::Value, DeserializerError> | ||
where | ||
V: Visitor<'de>, | ||
{ | ||
visitor.$method(self.value).change_context(DeserializerError) | ||
} | ||
} | ||
|
||
impl<'de> IntoDeserializer<'de> for $ty { | ||
type Deserializer<'a> = $name<'a> where Self: 'a; | ||
|
||
fn into_deserializer<'a>(self, context: &'a Context) -> Self::Deserializer<'a> | ||
where | ||
Self: 'a { | ||
$name::new(self, context) | ||
} | ||
} | ||
}; | ||
|
||
(copy: $ty:ty, $name:ident, $method:ident) => { | ||
impl_owned!(@INTERNAL COPY, $ty, $name, $method); | ||
impl_owned!(@INTERNAL IMPL, $ty, $name, $method); | ||
}; | ||
|
||
(!copy: $ty:ty, $name:ident, $method:ident) => { | ||
impl_owned!(@INTERNAL CLONE, $ty, $name, $method); | ||
impl_owned!(@INTERNAL IMPL, $ty, $name, $method); | ||
}; | ||
|
||
($ty:ty, $name:ident, $method:ident) => { | ||
impl_owned!(copy: $ty, $name, $method); | ||
}; | ||
} | ||
|
||
use impl_owned; | ||
|
||
pub trait IntoDeserializer<'de> { | ||
type Deserializer<'a>: Deserializer<'de> | ||
where | ||
Self: 'a; | ||
|
||
fn into_deserializer<'a>(self, context: &'a Context) -> Self::Deserializer<'a> | ||
where | ||
Self: 'a; | ||
} | ||
|
||
#[derive(Debug, Copy, Clone)] | ||
pub struct NoneDeserializer<'a> { | ||
context: &'a Context, | ||
} | ||
|
||
impl<'a> NoneDeserializer<'a> { | ||
#[must_use] | ||
pub const fn new(context: &'a Context) -> Self { | ||
Self { context } | ||
} | ||
} | ||
|
||
impl<'de> Deserializer<'de> for NoneDeserializer<'_> { | ||
forward_to_deserialize_any!( | ||
null | ||
bool | ||
number | ||
i8 i16 i32 i64 i128 isize | ||
u8 u16 u32 u64 u128 usize | ||
f32 f64 | ||
char str string | ||
bytes bytes_buffer | ||
array object | ||
); | ||
|
||
fn context(&self) -> &Context { | ||
self.context | ||
} | ||
|
||
fn deserialize_any<V>(self, visitor: V) -> error_stack::Result<V::Value, DeserializerError> | ||
where | ||
V: Visitor<'de>, | ||
{ | ||
visitor.visit_none().change_context(DeserializerError) | ||
} | ||
} | ||
|
||
#[derive(Debug, Copy, Clone)] | ||
pub struct NullDeserializer<'a> { | ||
context: &'a Context, | ||
} | ||
|
||
impl<'a> NullDeserializer<'a> { | ||
#[must_use] | ||
pub const fn new(context: &'a Context) -> Self { | ||
Self { context } | ||
} | ||
} | ||
|
||
impl<'de> Deserializer<'de> for NullDeserializer<'_> { | ||
forward_to_deserialize_any!( | ||
null | ||
bool | ||
number | ||
i8 i16 i32 i64 i128 isize | ||
u8 u16 u32 u64 u128 usize | ||
f32 f64 | ||
char str string | ||
bytes bytes_buffer | ||
array object | ||
); | ||
|
||
fn context(&self) -> &Context { | ||
self.context | ||
} | ||
|
||
fn deserialize_any<V>(self, visitor: V) -> error_stack::Result<V::Value, DeserializerError> | ||
where | ||
V: Visitor<'de>, | ||
{ | ||
visitor.visit_null().change_context(DeserializerError) | ||
} | ||
} | ||
|
||
impl_owned!(bool, BoolDeserializer, visit_bool); | ||
impl_owned!(char, CharDeserializer, visit_char); | ||
impl_owned!(u8, U8Deserializer, visit_u8); | ||
impl_owned!(u16, U16Deserializer, visit_u16); | ||
impl_owned!(u32, U32Deserializer, visit_u32); | ||
impl_owned!(u64, U64Deserializer, visit_u64); | ||
impl_owned!(u128, U128Deserializer, visit_u128); | ||
impl_owned!(usize, UsizeDeserializer, visit_usize); | ||
impl_owned!(i8, I8Deserializer, visit_i8); | ||
impl_owned!(i16, I16Deserializer, visit_i16); | ||
impl_owned!(i32, I32Deserializer, visit_i32); | ||
impl_owned!(i64, I64Deserializer, visit_i64); | ||
impl_owned!(i128, I128Deserializer, visit_i128); | ||
impl_owned!(isize, IsizeDeserializer, visit_isize); | ||
impl_owned!(f32, F32Deserializer, visit_f32); | ||
impl_owned!(f64, F64Deserializer, visit_f64); | ||
|
||
impl_owned!(!copy: Number, NumberDeserializer, visit_number); | ||
|
||
// TODO: test |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
use error_stack::{Result, ResultExt}; | ||
|
||
use crate::{error::DeserializerError, ArrayAccess, Context, Deserializer, Visitor}; | ||
|
||
// TODO: SliceDeserializer/IteratorDeserializer | ||
|
||
#[derive(Debug)] | ||
pub struct ArrayAccessDeserializer<'a, T> { | ||
context: &'a Context, | ||
value: T, | ||
} | ||
|
||
impl<'a, T> ArrayAccessDeserializer<'a, T> { | ||
#[must_use] | ||
pub const fn new(context: &'a Context, value: T) -> Self { | ||
Self { context, value } | ||
} | ||
} | ||
|
||
impl<'de, T> Deserializer<'de> for ArrayAccessDeserializer<'_, T> | ||
where | ||
T: ArrayAccess<'de>, | ||
{ | ||
forward_to_deserialize_any!( | ||
null | ||
bool | ||
number | ||
i8 i16 i32 i64 i128 isize | ||
u8 u16 u32 u64 u128 usize | ||
f32 f64 | ||
char str string | ||
bytes bytes_buffer | ||
array object | ||
); | ||
|
||
fn context(&self) -> &Context { | ||
self.context | ||
} | ||
|
||
fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, DeserializerError> | ||
where | ||
V: Visitor<'de>, | ||
{ | ||
visitor | ||
.visit_array(self.value) | ||
.change_context(DeserializerError) | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be sufficient to provide default impls. for all these in
Deserializer
instead of these macros or are we reyling on being able to specify generics in a complicated manner?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the pain; this is possible, but it would lead implementors to accidentally miss a function when implementing. This way, they need to implement every function without accidentally missing a deserializer function, and if they want to, they can use the macro to short-circuit. This is also what serde does.
So while technically possible, it would make potential new implementations more error-prone, IMO.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expected that to be the reason. A subtle bug due to missing a case seems way more problematic than having some macro indirection.
I do think that an extensive test suite should pick up any problems with that and make the default impls a better option. What about we keep this for now, but try to use default implementations at some point in the future when we're confident (i.e. we have extensive tests)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% confident with either (default implementation would be ok for us, but foreign crates implementing
Deserializer
would suffer if they do not have extensive tests). As you outlined above, the current version also has problems associated with it.I'd move this to the backlog for now, and maybe through discussion, we are able to come to a better solution! :D I am very much open to ideas.
But I do not expect that there's a large number of implementations for
Deserializer
(only data formats will implement them), see serde or any other serialization/deserialization crate, there are a dozenDeserializer
implementations but thousands ofDeserialize
implementations. So the tradeoff macro vs. default impl is hard to quantify 🤔 Maybe once we touch documentation an implementation guide could help with that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, sounds good. I am okay with merging this right now, then :)