Skip to content

Commit

Permalink
Rewrite: seperated Decode and BorrowDecode (#526)
Browse files Browse the repository at this point in the history 
* Rewrite: seperated Decode and BorrowDecode

* Fixed cargo.toml issues

* Fixed clippy warning

* Removed the `impl_tuples` macro call with manually exported code

* Replaced the generated code in `impl_tuples` with the macro instead

* Implemented BorrowDecode for Box<[T]>

* Added a test to see if zoxide can be ported to bincode 2

* Added a test for Arc<str>

* Made several `Encode` implementations require `T: ?Sized`

* Implemented Decode for Arc<str>

* Added BlockedTODO links to commented out code

* Fixed clippy and lint issues

* Updated virtue dependency in fuzz lockfile
  • Loading branch information
@VictorKoenders
VictorKoenders authored Jun 4, 2022
1 parent 5b19220 commit dcda3a8
Show file tree
Hide file tree
Showing 26 changed files with 936 additions and 313 deletions.
2 changes: 1 addition & 1 deletion derive/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ description = "Implementation of #[derive(Encode, Decode)] for bincode"
proc-macro = true

[dependencies]
virtue = "0.0.7"
virtue = "0.0.8"
46 changes: 46 additions & 0 deletions derive/src/attribute.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@ use virtue::utils::{parse_tagged_attribute, ParsedAttribute};

pub struct ContainerAttributes {
pub crate_name: String,
pub bounds: Option<(String, Literal)>,
pub decode_bounds: Option<(String, Literal)>,
pub borrow_decode_bounds: Option<(String, Literal)>,
pub encode_bounds: Option<(String, Literal)>,
}

impl Default for ContainerAttributes {
fn default() -> Self {
Self {
crate_name: "::bincode".to_string(),
bounds: None,
decode_bounds: None,
encode_bounds: None,
borrow_decode_bounds: None,
}
}
}
Expand All @@ -30,6 +38,44 @@ impl FromAttribute for ContainerAttributes {
return Err(Error::custom_at("Should be a literal str", val.span()));
}
}
ParsedAttribute::Property(key, val) if key.to_string() == "bounds" => {
let val_string = val.to_string();
if val_string.starts_with('"') && val_string.ends_with('"') {
result.bounds =
Some((val_string[1..val_string.len() - 1].to_string(), val));
} else {
return Err(Error::custom_at("Should be a literal str", val.span()));
}
}
ParsedAttribute::Property(key, val) if key.to_string() == "decode_bounds" => {
let val_string = val.to_string();
if val_string.starts_with('"') && val_string.ends_with('"') {
result.decode_bounds =
Some((val_string[1..val_string.len() - 1].to_string(), val));
} else {
return Err(Error::custom_at("Should be a literal str", val.span()));
}
}
ParsedAttribute::Property(key, val) if key.to_string() == "encode_bounds" => {
let val_string = val.to_string();
if val_string.starts_with('"') && val_string.ends_with('"') {
result.encode_bounds =
Some((val_string[1..val_string.len() - 1].to_string(), val));
} else {
return Err(Error::custom_at("Should be a literal str", val.span()));
}
}
ParsedAttribute::Property(key, val)
if key.to_string() == "borrow_decode_bounds" =>
{
let val_string = val.to_string();
if val_string.starts_with('"') && val_string.ends_with('"') {
result.borrow_decode_bounds =
Some((val_string[1..val_string.len() - 1].to_string(), val));
} else {
return Err(Error::custom_at("Should be a literal str", val.span()));
}
}
ParsedAttribute::Tag(i) => {
return Err(Error::custom_at("Unknown field attribute", i.span()))
}
Expand Down
45 changes: 34 additions & 11 deletions derive/src/derive_enum.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,22 @@ impl DeriveEnum {
generator
.impl_for(format!("{}::Encode", crate_name))
.modify_generic_constraints(|generics, where_constraints| {
for g in generics.iter_generics() {
if let Some((bounds, lit)) =
(self.attributes.encode_bounds.as_ref()).or(self.attributes.bounds.as_ref())
{
where_constraints.clear();
where_constraints
.push_constraint(g, format!("{}::Encode", crate_name))
.unwrap();
.push_parsed_constraint(bounds)
.map_err(|e| e.with_span(lit.span()))?;
} else {
for g in generics.iter_generics() {
where_constraints
.push_constraint(g, format!("{}::Encode", crate_name))
.unwrap();
}
}
})
Ok(())
})?
.generate_fn("encode")
.with_generic_deps("E", [format!("{}::enc::Encoder", crate_name)])
.with_self_arg(FnSelfArg::RefSelf)
Expand Down Expand Up @@ -206,7 +216,7 @@ impl DeriveEnum {
Ok(())
}

pub fn generate_decode(&self, generator: &mut Generator) -> Result<()> {
pub fn generate_decode(self, generator: &mut Generator) -> Result<()> {
let crate_name = self.attributes.crate_name.as_str();

// Remember to keep this mostly in sync with generate_borrow_decode
Expand All @@ -216,10 +226,16 @@ impl DeriveEnum {
generator
.impl_for(format!("{}::Decode", crate_name))
.modify_generic_constraints(|generics, where_constraints| {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::Decode", crate_name)).unwrap();
if let Some((bounds, lit)) = (self.attributes.decode_bounds.as_ref()).or(self.attributes.bounds.as_ref()) {
where_constraints.clear();
where_constraints.push_parsed_constraint(bounds).map_err(|e| e.with_span(lit.span()))?;
} else {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::Decode", crate_name)).unwrap();
}
}
})
Ok(())
})?
.generate_fn("decode")
.with_generic_deps("D", [format!("{}::de::Decoder", crate_name)])
.with_arg("decoder", "&mut D")
Expand Down Expand Up @@ -293,6 +309,7 @@ impl DeriveEnum {
}
Ok(())
})?;
self.generate_borrow_decode(generator)?;
Ok(())
}

Expand All @@ -304,10 +321,16 @@ impl DeriveEnum {

generator.impl_for_with_lifetimes(format!("{}::BorrowDecode", crate_name), ["__de"])
.modify_generic_constraints(|generics, where_constraints| {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::enc::BorrowDecode", crate_name)).unwrap();
if let Some((bounds, lit)) = (self.attributes.borrow_decode_bounds.as_ref()).or(self.attributes.bounds.as_ref()) {
where_constraints.clear();
where_constraints.push_parsed_constraint(bounds).map_err(|e| e.with_span(lit.span()))?;
} else {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::de::BorrowDecode<'__de>", crate_name)).unwrap();
}
}
})
Ok(())
})?
.generate_fn("borrow_decode")
.with_generic_deps("D", [format!("{}::de::BorrowDecoder<'__de>", crate_name)])
.with_arg("decoder", "&mut D")
Expand Down
59 changes: 39 additions & 20 deletions derive/src/derive_struct.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,26 @@ pub(crate) struct DeriveStruct {

impl DeriveStruct {
pub fn generate_encode(self, generator: &mut Generator) -> Result<()> {
let DeriveStruct { fields, attributes } = self;
let crate_name = attributes.crate_name;

let crate_name = &self.attributes.crate_name;
generator
.impl_for(&format!("{}::Encode", crate_name))
.modify_generic_constraints(|generics, where_constraints| {
for g in generics.iter_generics() {
if let Some((bounds, lit)) =
(self.attributes.encode_bounds.as_ref()).or(self.attributes.bounds.as_ref())
{
where_constraints.clear();
where_constraints
.push_constraint(g, format!("{}::Encode", crate_name))
.unwrap();
.push_parsed_constraint(bounds)
.map_err(|e| e.with_span(lit.span()))?;
} else {
for g in generics.iter_generics() {
where_constraints
.push_constraint(g, format!("{}::Encode", crate_name))
.unwrap();
}
}
})
Ok(())
})?
.generate_fn("encode")
.with_generic_deps("E", [format!("{}::enc::Encoder", crate_name)])
.with_self_arg(virtue::generate::FnSelfArg::RefSelf)
Expand All @@ -31,7 +39,7 @@ impl DeriveStruct {
crate_name
))
.body(|fn_body| {
for field in fields.names() {
for field in self.fields.names() {
let attributes = field
.attributes()
.get_attribute::<FieldAttributes>()?
Expand All @@ -56,16 +64,21 @@ impl DeriveStruct {

pub fn generate_decode(self, generator: &mut Generator) -> Result<()> {
// Remember to keep this mostly in sync with generate_borrow_decode
let DeriveStruct { fields, attributes } = self;
let crate_name = attributes.crate_name;
let crate_name = &self.attributes.crate_name;

generator
.impl_for(format!("{}::Decode", crate_name))
.modify_generic_constraints(|generics, where_constraints| {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::Decode", crate_name)).unwrap();
if let Some((bounds, lit)) = (self.attributes.decode_bounds.as_ref()).or(self.attributes.bounds.as_ref()) {
where_constraints.clear();
where_constraints.push_parsed_constraint(bounds).map_err(|e| e.with_span(lit.span()))?;
} else {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::Decode", crate_name)).unwrap();
}
}
})
Ok(())
})?
.generate_fn("decode")
.with_generic_deps("D", [format!("{}::de::Decoder", crate_name)])
.with_arg("decoder", "&mut D")
Expand All @@ -82,7 +95,7 @@ impl DeriveStruct {
// b: bincode::Decode::decode(decoder)?,
// ...
// }
for field in fields.names() {
for field in &self.fields.names() {
let attributes = field.attributes().get_attribute::<FieldAttributes>()?.unwrap_or_default();
if attributes.with_serde {
struct_body
Expand All @@ -106,21 +119,27 @@ impl DeriveStruct {
})?;
Ok(())
})?;
self.generate_borrow_decode(generator)?;
Ok(())
}

pub fn generate_borrow_decode(self, generator: &mut Generator) -> Result<()> {
// Remember to keep this mostly in sync with generate_decode
let DeriveStruct { fields, attributes } = self;
let crate_name = attributes.crate_name;
let crate_name = self.attributes.crate_name;

generator
.impl_for_with_lifetimes(format!("{}::BorrowDecode", crate_name), ["__de"])
.modify_generic_constraints(|generics, where_constraints| {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::BorrowDecode", crate_name)).unwrap();
if let Some((bounds, lit)) = (self.attributes.borrow_decode_bounds.as_ref()).or(self.attributes.bounds.as_ref()) {
where_constraints.clear();
where_constraints.push_parsed_constraint(bounds).map_err(|e| e.with_span(lit.span()))?;
} else {
for g in generics.iter_generics() {
where_constraints.push_constraint(g, format!("{}::de::BorrowDecode<'__de>", crate_name)).unwrap();
}
}
})
Ok(())
})?
.generate_fn("borrow_decode")
.with_generic_deps("D", [format!("{}::de::BorrowDecoder<'__de>", crate_name)])
.with_arg("decoder", "&mut D")
Expand All @@ -131,7 +150,7 @@ impl DeriveStruct {
fn_body.group(Delimiter::Parenthesis, |ok_group| {
ok_group.ident_str("Self");
ok_group.group(Delimiter::Brace, |struct_body| {
for field in fields.names() {
for field in self.fields.names() {
let attributes = field.attributes().get_attribute::<FieldAttributes>()?.unwrap_or_default();
if attributes.with_serde {
struct_body
Expand Down
5 changes: 3 additions & 2 deletions fuzz/Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 14 additions & 4 deletions fuzz/fuzz_targets/compat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,17 @@ use std::num::{NonZeroI128, NonZeroI32, NonZeroU128, NonZeroU32};
use std::path::PathBuf;
use std::time::{Duration, SystemTime};

#[derive(bincode::Decode, bincode::Encode, PartialEq, Debug, serde::Serialize, serde::Deserialize, Eq, PartialOrd, Ord)]
#[derive(
bincode::Decode,
bincode::Encode,
PartialEq,
Debug,
serde::Serialize,
serde::Deserialize,
Eq,
PartialOrd,
Ord,
)]
enum AllTypes {
BTreeMap(BTreeMap<u8, AllTypes>),
BTreeSet(BTreeSet<AllTypes>),
Expand Down Expand Up @@ -47,14 +57,14 @@ fuzz_target!(|data: &[u8]| {
let bincode_v2: Result<(AllTypes, _), _> = bincode::decode_from_slice(data, config);

match (&bincode_v1, &bincode_v2) {
(Err(e), _) if e.to_string() == "the size limit has been reached" => {},
(_, Err(bincode::error::DecodeError::LimitExceeded)) => {},
(Err(e), _) if e.to_string() == "the size limit has been reached" => {}
(_, Err(bincode::error::DecodeError::LimitExceeded)) => {}
(Ok(bincode_v1), Ok((bincode_v2, _))) if bincode_v1 != bincode_v2 => {
println!("Bytes: {:?}", data);
println!("Bincode V1: {:?}", bincode_v1);
println!("Bincode V2: {:?}", bincode_v2);
panic!("failed equality check");
},
}
(Ok(_), Err(_)) | (Err(_), Ok(_)) => {
println!("Bytes: {:?}", data);
println!("Bincode V1: {:?}", bincode_v1);
Expand Down
Loading

0 comments on commit dcda3a8

Please sign in to comment.