Skip to content

Rewrite: seperated Decode and BorrowDecode #526

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

Merged
merged 16 commits into from
Jun 4, 2022
Merged
Show file tree
Hide file tree
Changes from 11 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
2 changes: 1 addition & 1 deletion derive/Cargo.toml
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
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
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
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
3 changes: 1 addition & 2 deletions fuzz/Cargo.lock

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
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