Skip to content
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

Support more serialization/deserialization targets: Deserialize into owned strings if "alloc" feature is enabled #175

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ All notable changes will be documented in this file.
This document is written according to the [Keep a Changelog][kac] style.

1. [Version 1](#version-1)
1. [1.0](#10)
1. [1.1.0](#110)
1. [1.0.1](#101)
1. [1.0](#100)
1. [Version 0 (Prototyping)](#version-0-prototyping)
1. [0.22](#022)
1. [0.21](#021)
Expand Down Expand Up @@ -36,6 +38,15 @@ This document is written according to the [Keep a Changelog][kac] style.
will continue to receive maintenance, but its API is now stable and **will not**
change until const-generics allow `BitArray` to be rewritten.

### 1.1.0

Deserialize into owned strings if the `alloc` and `serde` features are enabled,
to support deserializing via libraries like `serde_json` which expect owned
strings. See [Pull Request #175] for more detail.

This requires bumping the MSRV to `1.60`, which is the version that optional
dependencies were added in.

### 1.0.1

The `bits![static mut …]` invocation has been made `unsafe` at the invocation
Expand Down
7 changes: 5 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

[package]
name = "bitvec"
version = "1.0.1"
version = "1.1.0"
edition = "2021"

categories = [
Expand Down Expand Up @@ -36,10 +36,13 @@ keywords = [
license = "MIT"
readme = "README.md"
repository = "https://github.com/bitvecto-rs/bitvec"
rust-version = "1.56"
rust-version = "1.60"

[features]
alloc = [
# If the serde feature is also enabled, enable alloc for it
# so that we can deserialize Strings.
"serde?/alloc"
Copy link
Author

@jsdw jsdw May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the min supported rustc version for Bitvec is 1.56, this "?" features isn't available yet.
Removing it would mean that enabling the "alloc" features also enabled the "serde" feature. Any thoughts on how best to tackle this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which version did this stabilize? While I had hoped to refrain from MSRV raises until I'm able to release 2.0 with the BitArray redesign, my MSRV policy does state that I may freely do so in the minor series.

If raising the MSRV to support optional transitive features really is sufficient (with the rest of this PR) to resolve the de/ser problem, then please raise MSRV in rust-toolchain.toml and README.md, and update to 1.1.0 in Cargo.toml.

I am going to continue working through my backlog in the 1.0 series. Thank you for your patience as I've ignored the project, and I look forward to releasing your 1.1 soon!

]
atomic = [
]
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ However, it does also have some small costs associated with its capabilities:

## Usage

**Minimum Supported Rust Version**: 1.56.0
**Minimum Supported Rust Version**: 1.60.0

`bitvec` strives to follow the sequence APIs in the standard library. However,
as most of its functionality is a reïmplementation that does not require the
Expand Down
2 changes: 1 addition & 1 deletion doc/serdes/utils.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ from a matching data buffer.

Serde only provides implementations for `[T; 0 ..= 32]`, because it must support
much older Rust versions (at time of writing, 1.15+) that do not have
const-generics. As `bitvec` has an MSRV of 1.56; it *does* have const-generics.
const-generics. As `bitvec` has an MSRV of 1.60; it *does* have const-generics.
This type reïmplements Serde’s array behavior for all arrays, so that `bitvec`
can transport any `BitArray` rather than only small bit-arrays.

Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
########################################################################

[toolchain]
channel = "1.56.0"
channel = "1.60.0"
profile = "default"
components = [
"clippy",
Expand Down
22 changes: 17 additions & 5 deletions src/serdes/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use serde::{
use wyz::comu::Const;

use super::FIELDS;
use super::utils::StringTarget;
#[cfg(feature = "alloc")]
use crate::{
boxed::BitBox,
Expand Down Expand Up @@ -167,7 +168,7 @@ where
/// As well as a final output value.
out: PhantomData<Result<Out, BitSpanError<T>>>,
/// The deserialized bit-ordering string.
order: Option<&'de str>,
order: Option<StringTarget<'de>>,
/// The deserialized head-bit index.
head: Option<BitIdx<T::Mem>>,
/// The deserialized bit-count.
Expand Down Expand Up @@ -208,8 +209,9 @@ where
let bits = self.bits.take().ok_or_else(|| E::missing_field("bits"))?;
let data = self.data.take().ok_or_else(|| E::missing_field("data"))?;

if order != any::type_name::<O>() {
return Err(E::invalid_type(Unexpected::Str(order), &self));
let expected_order = any::type_name::<O>();
if order != expected_order {
return Err(E::invalid_type(Unexpected::Str(&*order), &self));
}
(self.func)(data, head, bits as usize).map_err(|_| todo!())
}
Expand Down Expand Up @@ -258,8 +260,8 @@ where

fn visit_map<V>(mut self, mut map: V) -> Result<Self::Value, V::Error>
where V: MapAccess<'de> {
while let Some(key) = map.next_key::<&'de str>()? {
match key {
while let Some(key) = map.next_key::<StringTarget<'de>>()? {
match &*key {
"order" => {
if self.order.replace(map.next_value()?).is_some() {
return Err(<V::Error>::duplicate_field("order"));
Expand Down Expand Up @@ -316,6 +318,16 @@ mod tests {
Ok(())
}

#[test]
#[cfg(feature = "alloc")]
fn roundtrip_json() -> Result<(), alloc::boxed::Box<serde_json::Error>> {
let bits = bitvec![u8, Msb0; 1, 0, 1, 1, 0];
let encoded = serde_json::to_value(&bits)?;
let bits2 = serde_json::from_value::<BitVec<u8, Msb0>>(encoded)?;
assert_eq!(bits, bits2);
Ok(())
}

#[test]
fn tokens() {
let slice = bits![u8, Lsb0; 0, 1, 0, 0, 1];
Expand Down
16 changes: 14 additions & 2 deletions src/serdes/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,8 @@ where R: BitRegister
let mut width = None;
let mut index = None;

while let Some(key) = map.next_key::<&'de str>()? {
match key {
while let Some(key) = map.next_key::<StringTarget<'de>>()? {
match &*key {
"width" => {
if width.replace(map.next_value::<u8>()?).is_some() {
return Err(<V::Error>::duplicate_field("width"));
Expand All @@ -272,6 +272,18 @@ where R: BitRegister
}
}

/// This is a target used to deserialize strings into.
/// With the alloc feature enabled, we will deserialize into owned strings,
/// granting a little more deserialization flexibility.
#[cfg(feature = "alloc")]
pub type StringTarget<'de> = String;

/// This is a target used to deserialize strings into.
/// Without the alloc feature enabled, we will deserialize into borrowed strings,
/// which makes deserializing from some targets (like JSON) impossible.
#[cfg(not(feature = "alloc"))]
pub type StringTarget<'de> = &'de str;

#[cfg(test)]
mod tests {
use serde_test::{
Expand Down