Feature/improve serde (#477)

* Made serde able to be used with alloc and no_std targets * Processed feedback
bincode-org · Jan 19, 2022 · 983680c · 983680c
1 parent 29d1d5c
commit 983680c
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 34 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -85,7 +85,11 @@
             "alloc,derive",
             "std",
             "std,derive",
+            "serde_no_std",
+            "serde_alloc",
             "serde",
+            "serde_no_std,derive",
+            "serde_alloc,derive",
             "serde,derive"
           ]
         }

diff --git a/Cargo.toml b/Cargo.toml
@@ -30,19 +30,23 @@ atomic = []
 derive = ["bincode_derive"]
 
 # BlockedTODO: https://github.com/rust-lang/cargo/issues/8832
-# We really want these features to automatically be enabled when both "serde" and either "alloc"/"std" is enabled
-# But this is currently not possible
-serde = ["std", "serde_incl", "serde_incl/std"]
+# We want to enable these features automatically based on "alloc" or "std"
+#   std = ["alloc", "serde?/std"]
+#   alloc = ["serde?/alloc"]
+# now we have to do this hack:
+serde = ["serde_incl/std", "std", "serde_alloc"] # std
+serde_alloc = ["serde_incl/alloc", "alloc"] # alloc
+serde_no_std = ["serde_incl"] # no_std
 
 [dependencies]
 bincode_derive = { path = "derive", version = "2.0.0-beta.0", optional = true }
-serde_incl = { package = "serde", version = "1.0.130", optional = true }
+serde_incl = { package = "serde", version = "1.0", default-features = false, optional = true }
 
 # Used for tests
 [dev-dependencies]
-serde_derive = "1.0.130"
-serde_json = "1.0.68"
-tempfile = "3.2.0"
+serde_derive = "1.0"
+serde_json = { version = "1.0", default-features = false }
+tempfile = "3.2"
 criterion = "0.3"
 rand = "0.8"
 uuid = { version = "0.8", features = ["serde"] }

diff --git a/src/error.rs b/src/error.rs
@@ -51,9 +51,9 @@ pub enum EncodeError {
         time: std::time::SystemTime,
     },
 
-    /// Serde provided bincode with a sequence without a length, which is not supported in bincode
     #[cfg(feature = "serde")]
-    SequenceMustHaveLength,
+    /// A serde-specific error that occured while decoding.
+    Serde(crate::features::serde::EncodeError),
 }
 
 impl core::fmt::Display for EncodeError {
@@ -157,21 +157,9 @@ pub enum DecodeError {
     #[cfg(feature = "alloc")]
     OtherString(alloc::string::String),
 
-    /// Bincode does not support serde's `any` decoding feature
     #[cfg(feature = "serde")]
-    SerdeAnyNotSupported,
-
-    /// Bincode does not support serde identifiers
-    #[cfg(feature = "serde")]
-    SerdeIdentifierNotSupported,
-
-    /// Bincode does not support serde's `ignored_any`
-    #[cfg(feature = "serde")]
-    SerdeIgnoredAnyNotSupported,
-
-    /// Serde tried decoding a borrowed value from an owned reader. Use `serde_decode_borrowed_from_*` instead
-    #[cfg(feature = "serde")]
-    CannotBorrowOwnedData,
+    /// A serde-specific error that occured while decoding.
+    Serde(crate::features::serde::DecodeError),
 }
 
 impl core::fmt::Display for DecodeError {

diff --git a/src/features/serde/de_borrowed.rs b/src/features/serde/de_borrowed.rs
@@ -1,3 +1,4 @@
+use super::DecodeError as SerdeDecodeError;
 use crate::{
     config::Config,
     de::{BorrowDecode, BorrowDecoder, Decode},
@@ -33,7 +34,7 @@ impl<'a, 'de, DE: BorrowDecoder<'de>> Deserializer<'de> for SerdeDecoder<'a, 'de
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::SerdeAnyNotSupported)
+        Err(SerdeDecodeError::AnyNotSupported.into())
     }
 
     fn deserialize_bool<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
@@ -128,6 +129,15 @@ impl<'a, 'de, DE: BorrowDecoder<'de>> Deserializer<'de> for SerdeDecoder<'a, 'de
         visitor.visit_borrowed_str(str)
     }
 
+    #[cfg(not(feature = "alloc"))]
+    fn deserialize_string<V>(self, _: V) -> Result<V::Value, Self::Error>
+    where
+        V: serde_incl::de::Visitor<'de>,
+    {
+        Err(SerdeDecodeError::CannotBorrowOwnedData.into())
+    }
+
+    #[cfg(feature = "alloc")]
     fn deserialize_string<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
@@ -143,6 +153,15 @@ impl<'a, 'de, DE: BorrowDecoder<'de>> Deserializer<'de> for SerdeDecoder<'a, 'de
         visitor.visit_borrowed_bytes(bytes)
     }
 
+    #[cfg(not(feature = "alloc"))]
+    fn deserialize_byte_buf<V>(self, _: V) -> Result<V::Value, Self::Error>
+    where
+        V: serde_incl::de::Visitor<'de>,
+    {
+        Err(SerdeDecodeError::CannotBorrowOwnedData.into())
+    }
+
+    #[cfg(feature = "alloc")]
     fn deserialize_byte_buf<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
@@ -339,14 +358,14 @@ impl<'a, 'de, DE: BorrowDecoder<'de>> Deserializer<'de> for SerdeDecoder<'a, 'de
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::SerdeIdentifierNotSupported)
+        Err(SerdeDecodeError::IdentifierNotSupported.into())
     }
 
     fn deserialize_ignored_any<V>(self, _: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::SerdeIgnoredAnyNotSupported)
+        Err(SerdeDecodeError::IgnoredAnyNotSupported.into())
     }
 }
 

diff --git a/src/features/serde/de_owned.rs b/src/features/serde/de_owned.rs
@@ -1,3 +1,4 @@
+use super::DecodeError as SerdeDecodeError;
 use crate::{
     config::Config,
     de::{Decode, Decoder},
@@ -34,7 +35,7 @@ impl<'a, 'de, DE: Decoder> Deserializer<'de> for SerdeDecoder<'a, DE> {
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::SerdeAnyNotSupported)
+        Err(SerdeDecodeError::AnyNotSupported.into())
     }
 
     fn deserialize_bool<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
@@ -130,20 +131,29 @@ impl<'a, 'de, DE: Decoder> Deserializer<'de> for SerdeDecoder<'a, DE> {
     }
 
     #[cfg(not(feature = "alloc"))]
-    fn deserialize_str<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
+    fn deserialize_str<V>(self, _: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::CannotBorrowOwnedData)
+        Err(SerdeDecodeError::CannotBorrowOwnedData.into())
     }
 
+    #[cfg(feature = "alloc")]
     fn deserialize_string<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
     {
         visitor.visit_string(Decode::decode(&mut self.de)?)
     }
 
+    #[cfg(not(feature = "alloc"))]
+    fn deserialize_string<V>(self, _: V) -> Result<V::Value, Self::Error>
+    where
+        V: serde_incl::de::Visitor<'de>,
+    {
+        Err(SerdeDecodeError::CannotAllocate.into())
+    }
+
     #[cfg(feature = "alloc")]
     fn deserialize_bytes<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
     where
@@ -157,15 +167,23 @@ impl<'a, 'de, DE: Decoder> Deserializer<'de> for SerdeDecoder<'a, DE> {
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::CannotBorrowOwnedData)
+        Err(SerdeDecodeError::CannotBorrowOwnedData.into())
     }
 
+    #[cfg(feature = "alloc")]
     fn deserialize_byte_buf<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
     {
         visitor.visit_byte_buf(Decode::decode(&mut self.de)?)
     }
+    #[cfg(not(feature = "alloc"))]
+    fn deserialize_byte_buf<V>(self, _: V) -> Result<V::Value, Self::Error>
+    where
+        V: serde_incl::de::Visitor<'de>,
+    {
+        Err(SerdeDecodeError::CannotAllocate.into())
+    }
 
     fn deserialize_option<V>(mut self, visitor: V) -> Result<V::Value, Self::Error>
     where
@@ -353,14 +371,14 @@ impl<'a, 'de, DE: Decoder> Deserializer<'de> for SerdeDecoder<'a, DE> {
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::SerdeIdentifierNotSupported)
+        Err(SerdeDecodeError::IdentifierNotSupported.into())
     }
 
     fn deserialize_ignored_any<V>(self, _: V) -> Result<V::Value, Self::Error>
     where
         V: serde_incl::de::Visitor<'de>,
     {
-        Err(DecodeError::SerdeIgnoredAnyNotSupported)
+        Err(SerdeDecodeError::IgnoredAnyNotSupported.into())
     }
 }
 

diff --git a/src/features/serde/mod.rs b/src/features/serde/mod.rs
@@ -40,6 +40,14 @@
 //! # }
 //! ```
 //!
+//! # `alloc` and `no_std`
+//!
+//! The `serde` feature enables both `alloc` and `std` at this point in time.
+//! To use bincode and serde on no_std targets, try one of the following features:
+//!
+//! - `serde_alloc`: enables `serde` and `alloc`
+//! - `serde_no_std`: enables `serde` without `alloc` or `std`
+//!
 //! # Known issues
 //!
 //! Currently the `serde` feature will automatically enable the `alloc` and `std` feature. If you're running in a `#[no_std]` environment consider using bincode's own derive macros.
@@ -66,6 +74,32 @@ pub use self::de_borrowed::*;
 pub use self::de_owned::*;
 pub use self::ser::*;
 
+/// A serde-specific error that occured while decoding.
+#[derive(Debug, PartialEq)]
+#[non_exhaustive]
+pub enum DecodeError {
+    /// Bincode does not support serde's `any` decoding feature
+    AnyNotSupported,
+
+    /// Bincode does not support serde identifiers
+    IdentifierNotSupported,
+
+    /// Bincode does not support serde's `ignored_any`
+    IgnoredAnyNotSupported,
+
+    /// Serde tried decoding a borrowed value from an owned reader. Use `serde_decode_borrowed_from_*` instead
+    CannotBorrowOwnedData,
+
+    /// Could not allocate data like `String` and `Vec<u8>`
+    #[cfg(not(feature = "alloc"))]
+    CannotAllocate,
+
+    /// Custom serde error but bincode is unable to allocate a string. Set a breakpoint where this is thrown for more information.
+    #[cfg(not(feature = "alloc"))]
+    CustomError,
+}
+
+#[cfg(feature = "alloc")]
 impl serde_incl::de::Error for crate::error::DecodeError {
     fn custom<T>(msg: T) -> Self
     where
@@ -76,6 +110,50 @@ impl serde_incl::de::Error for crate::error::DecodeError {
     }
 }
 
+#[cfg(not(feature = "std"))]
+impl serde_incl::de::StdError for crate::error::DecodeError {}
+
+#[cfg(not(feature = "alloc"))]
+impl serde_incl::de::Error for crate::error::DecodeError {
+    fn custom<T>(_: T) -> Self
+    where
+        T: core::fmt::Display,
+    {
+        DecodeError::CustomError.into()
+    }
+}
+
+#[allow(clippy::from_over_into)]
+impl Into<crate::error::DecodeError> for DecodeError {
+    fn into(self) -> crate::error::DecodeError {
+        crate::error::DecodeError::Serde(self)
+    }
+}
+
+/// A serde-specific error that occured while encoding.
+#[derive(Debug, PartialEq)]
+#[non_exhaustive]
+pub enum EncodeError {
+    /// Serde provided bincode with a sequence without a length, which is not supported in bincode
+    SequenceMustHaveLength,
+
+    /// [Serializer::collect_str] got called but bincode was unable to allocate memory.
+    #[cfg(not(feature = "alloc"))]
+    CannotCollectStr,
+
+    /// Custom serde error but bincode is unable to allocate a string. Set a breakpoint where this is thrown for more information.
+    #[cfg(not(feature = "alloc"))]
+    CustomError,
+}
+
+#[allow(clippy::from_over_into)]
+impl Into<crate::error::EncodeError> for EncodeError {
+    fn into(self) -> crate::error::EncodeError {
+        crate::error::EncodeError::Serde(self)
+    }
+}
+
+#[cfg(feature = "alloc")]
 impl serde_incl::ser::Error for crate::error::EncodeError {
     fn custom<T>(msg: T) -> Self
     where
@@ -87,6 +165,19 @@ impl serde_incl::ser::Error for crate::error::EncodeError {
     }
 }
 
+#[cfg(not(feature = "std"))]
+impl serde_incl::de::StdError for crate::error::EncodeError {}
+
+#[cfg(not(feature = "alloc"))]
+impl serde_incl::ser::Error for crate::error::EncodeError {
+    fn custom<T>(_: T) -> Self
+    where
+        T: core::fmt::Display,
+    {
+        EncodeError::CustomError.into()
+    }
+}
+
 /// Wrapper struct that implements [Decode] and [Encode] on any type that implements serde's [DeserializeOwned] and [Serialize] respectively.
 ///
 /// This works for most types, but if you're dealing with borrowed data consider using [BorrowCompat] instead.

diff --git a/src/features/serde/ser.rs b/src/features/serde/ser.rs
@@ -1,3 +1,4 @@
+use super::EncodeError as SerdeEncodeError;
 use crate::{
     config::Config,
     enc::{Encode, Encoder},
@@ -165,7 +166,7 @@ where
     }
 
     fn serialize_seq(mut self, len: Option<usize>) -> Result<Self::SerializeSeq, Self::Error> {
-        let len = len.ok_or(EncodeError::SequenceMustHaveLength)?;
+        let len = len.ok_or_else(|| SerdeEncodeError::SequenceMustHaveLength.into())?;
         len.encode(&mut self.enc)?;
         Ok(Compound { enc: self.enc })
     }
@@ -196,7 +197,7 @@ where
     }
 
     fn serialize_map(mut self, len: Option<usize>) -> Result<Self::SerializeMap, Self::Error> {
-        let len = len.ok_or(EncodeError::SequenceMustHaveLength)?;
+        let len = len.ok_or_else(|| SerdeEncodeError::SequenceMustHaveLength.into())?;
         len.encode(&mut self.enc)?;
         Ok(Compound { enc: self.enc })
     }
@@ -219,6 +220,14 @@ where
         variant_index.encode(&mut self.enc)?;
         Ok(Compound { enc: self.enc })
     }
+
+    #[cfg(not(feature = "alloc"))]
+    fn collect_str<T: ?Sized>(self, _: &T) -> Result<Self::Ok, Self::Error>
+    where
+        T: core::fmt::Display,
+    {
+        Err(SerdeEncodeError::CannotCollectStr.into())
+    }
 }
 
 type Compound<'a, ENC> = SerdeEncoder<'a, ENC>;