Skip to content

Commit ac64a9d

Browse files
alexalex
authored
Fixed perfect derives of implicit/explicit tagging of fields (#502)
1 parent 96c42b2 commit ac64a9d

File tree

5 files changed

+145
-19
lines changed

5 files changed

+145
-19
lines changed

‎README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ asn1 = { version = "0.19", default-features = false }
3232
These can all be better accomplished with the `asn1::Explicit` and
3333
`asn1::Implicit` types.
3434

35+
#### Fixes
36+
37+
- Fixed ["perfect derives"](https://smallcultfollowing.com/babysteps/blog/2022/04/12/implied-bounds-and-perfect-derive/)
38+
in conjunction with `#[implicit]` and `#[explicit]`.
39+
([#502](https://github.com/alex/rust-asn1/pull/502))
40+
3541
### [0.19.0]
3642

3743
#### :rotating_light: Breaking changes

‎asn1_derive/src/lib.rs‎

Lines changed: 82 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ pub fn derive_asn1_read(input: proc_macro::TokenStream) -> proc_macro::TokenStre
1717
all_field_types(&input.data),
1818
syn::parse_quote!(asn1::Asn1Readable<#lifetime_name>),
1919
syn::parse_quote!(asn1::Asn1DefinedByReadable<#lifetime_name, asn1::ObjectIdentifier>),
20+
false,
2021
);
2122
let (impl_generics, _, where_clause) = generics.split_for_impl();
2223

@@ -65,6 +66,7 @@ pub fn derive_asn1_write(input: proc_macro::TokenStream) -> proc_macro::TokenStr
6566
all_field_types(&input.data),
6667
syn::parse_quote!(asn1::Asn1Writable),
6768
syn::parse_quote!(asn1::Asn1DefinedByWritable<asn1::ObjectIdentifier>),
69+
true,
6870
);
6971
let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
7072

@@ -93,7 +95,7 @@ pub fn derive_asn1_write(input: proc_macro::TokenStream) -> proc_macro::TokenStr
9395

9496
impl #impl_generics asn1::Asn1Writable for &#name #ty_generics #where_clause {
9597
fn write(&self, w: &mut asn1::Writer) -> asn1::WriteResult {
96-
#name::write(self, w)
98+
(*self).write(w)
9799
}
98100
}
99101
}
@@ -273,48 +275,69 @@ fn add_lifetime_if_none(generics: &mut syn::Generics) -> syn::Lifetime {
273275
generics.lifetimes().next().unwrap().lifetime.clone()
274276
}
275277

276-
fn all_field_types(data: &syn::Data) -> Vec<(syn::Type, bool)> {
278+
fn all_field_types(data: &syn::Data) -> Vec<(syn::Type, OpType, bool)> {
277279
let mut field_types = vec![];
278280
match data {
279281
syn::Data::Struct(v) => {
280-
add_field_types(&mut field_types, &v.fields);
282+
add_field_types(&mut field_types, &v.fields, None);
281283
}
282284
syn::Data::Enum(v) => {
283285
for variant in &v.variants {
284-
add_field_types(&mut field_types, &variant.fields);
286+
let (op_type, _) = extract_field_properties(&variant.attrs);
287+
add_field_types(&mut field_types, &variant.fields, Some(op_type));
285288
}
286289
}
287290
syn::Data::Union(_) => panic!("Unions not supported"),
288291
}
289292
field_types
290293
}
291294

292-
fn add_field_types(field_types: &mut Vec<(syn::Type, bool)>, fields: &syn::Fields) {
295+
fn add_field_types(
296+
field_types: &mut Vec<(syn::Type, OpType, bool)>,
297+
fields: &syn::Fields,
298+
op_type: Option<OpType>,
299+
) {
293300
match fields {
294301
syn::Fields::Named(v) => {
295302
for f in &v.named {
296-
add_field_type(field_types, f);
303+
add_field_type(field_types, f, op_type.clone());
297304
}
298305
}
299306
syn::Fields::Unnamed(v) => {
300307
for f in &v.unnamed {
301-
add_field_type(field_types, f);
308+
add_field_type(field_types, f, op_type.clone());
302309
}
303310
}
304311
syn::Fields::Unit => {}
305312
}
306313
}
307314

308-
fn add_field_type(field_types: &mut Vec<(syn::Type, bool)>, f: &syn::Field) {
309-
let (op_type, _) = extract_field_properties(&f.attrs);
310-
field_types.push((f.ty.clone(), matches!(op_type, OpType::DefinedBy(_))));
315+
fn add_field_type(
316+
field_types: &mut Vec<(syn::Type, OpType, bool)>,
317+
f: &syn::Field,
318+
op_type: Option<OpType>,
319+
) {
320+
// If we have an op_type here, it means it came from an enum variant. In
321+
// that case, even though it wasn't marked "required", it is for the
322+
// purposes of how we're using it.
323+
let (op_type, default) = if let Some(OpType::Explicit(mut args)) = op_type {
324+
args.required = true;
325+
(OpType::Explicit(args), None)
326+
} else if let Some(OpType::Implicit(mut args)) = op_type {
327+
args.required = true;
328+
(OpType::Implicit(args), None)
329+
} else {
330+
extract_field_properties(&f.attrs)
331+
};
332+
field_types.push((f.ty.clone(), op_type, default.is_some()));
311333
}
312334

313335
fn add_bounds(
314336
generics: &mut syn::Generics,
315-
field_types: Vec<(syn::Type, bool)>,
337+
field_types: Vec<(syn::Type, OpType, bool)>,
316338
bound: syn::TypeParamBound,
317339
defined_by_bound: syn::TypeParamBound,
340+
add_ref: bool,
318341
) {
319342
let where_clause = if field_types.is_empty() {
320343
return;
@@ -327,33 +350,74 @@ fn add_bounds(
327350
})
328351
};
329352

330-
for (f, is_defined_by) in field_types {
353+
for (f, op_type, has_default) in field_types {
354+
let (bounded_ty, required_bound) = match (op_type, add_ref) {
355+
(OpType::Regular, _) => (f, bound.clone()),
356+
(OpType::DefinedBy(_), _) => (f, defined_by_bound.clone()),
357+
358+
(OpType::Implicit(OpTypeArgs { value, required }), false) => {
359+
let ty = if required || has_default {
360+
syn::parse_quote!(asn1::Implicit::<#f, #value>)
361+
} else {
362+
syn::parse_quote!(asn1::Implicit::<<#f as asn1::OptionExt>::T, #value>)
363+
};
364+
365+
(ty, bound.clone())
366+
}
367+
(OpType::Implicit(OpTypeArgs { value, required }), true) => {
368+
let ty = if required || has_default {
369+
syn::parse_quote!(for<'asn1_internal> asn1::Implicit::<&'asn1_internal #f, #value>)
370+
} else {
371+
syn::parse_quote!(for<'asn1_internal> asn1::Implicit::<&'asn1_internal <#f as asn1::OptionExt>::T, #value>)
372+
};
373+
374+
(ty, bound.clone())
375+
}
376+
377+
(OpType::Explicit(OpTypeArgs { value, required }), false) => {
378+
let ty = if required || has_default {
379+
syn::parse_quote!(asn1::Explicit::<#f, #value>)
380+
} else {
381+
syn::parse_quote!(asn1::Explicit::<<#f as asn1::OptionExt>::T, #value>)
382+
};
383+
384+
(ty, bound.clone())
385+
}
386+
(OpType::Explicit(OpTypeArgs { value, required }), true) => {
387+
let ty = if required || has_default {
388+
syn::parse_quote!(for<'asn1_internal> asn1::Explicit::<&'asn1_internal #f, #value>)
389+
} else {
390+
syn::parse_quote!(for<'asn1_internal> asn1::Explicit::<&'asn1_internal <#f as asn1::OptionExt>::T, #value>)
391+
};
392+
393+
(ty, bound.clone())
394+
}
395+
};
396+
331397
where_clause
332398
.predicates
333399
.push(syn::WherePredicate::Type(syn::PredicateType {
334400
lifetimes: None,
335-
bounded_ty: f,
401+
bounded_ty,
336402
colon_token: Default::default(),
337403
bounds: {
338404
let mut p = syn::punctuated::Punctuated::new();
339-
if is_defined_by {
340-
p.push(defined_by_bound.clone());
341-
} else {
342-
p.push(bound.clone());
343-
}
405+
p.push(required_bound);
344406
p
345407
},
346408
}))
347409
}
348410
}
349411

412+
#[derive(Clone)]
350413
enum OpType {
351414
Regular,
352415
Explicit(OpTypeArgs),
353416
Implicit(OpTypeArgs),
354417
DefinedBy(syn::Ident),
355418
}
356419

420+
#[derive(Clone)]
357421
struct OpTypeArgs {
358422
value: proc_macro2::Literal,
359423
required: bool,

‎src/lib.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,17 @@ pub fn writable_defined_by_item<T: Asn1Writable, U: Asn1DefinedByWritable<T>>(v:
232232
v.item()
233233
}
234234

235+
/// Utility for use in `asn1_derive`. Not considered part of the public API.
236+
#[doc(hidden)]
237+
pub trait OptionExt {
238+
type T;
239+
}
240+
241+
#[doc(hidden)]
242+
impl<T> OptionExt for Option<T> {
243+
type T = T;
244+
}
245+
235246
#[cfg(test)]
236247
mod tests {
237248
#[test]

‎src/types.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ pub struct SequenceOfWriter<'a, T, V: Borrow<[T]> = &'a [T]> {
15461546
_phantom: PhantomData<&'a T>,
15471547
}
15481548

1549-
impl<T: Asn1Writable, V: Borrow<[T]>> SequenceOfWriter<'_, T, V> {
1549+
impl<T, V: Borrow<[T]>> SequenceOfWriter<'_, T, V> {
15501550
pub fn new(vals: V) -> Self {
15511551
SequenceOfWriter {
15521552
vals,

‎tests/derive_test.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,4 +782,49 @@ fn test_perfect_derive() {
782782
}
783783

784784
assert_roundtrips::<S<Op>>(&[(Ok(S { value: 12 }), b"\x30\x03\x02\x01\x0c")]);
785+
786+
#[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Debug, Eq)]
787+
struct TaggedRequiredFields<T: X> {
788+
#[implicit(1, required)]
789+
a: T::Type,
790+
#[explicit(2, required)]
791+
b: T::Type,
792+
}
793+
794+
assert_roundtrips::<TaggedRequiredFields<Op>>(&[(
795+
Ok(TaggedRequiredFields { a: 1, b: 3 }),
796+
b"\x30\x08\x81\x01\x01\xa2\x03\x02\x01\x03",
797+
)]);
798+
799+
#[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Debug, Eq)]
800+
struct TaggedOptionalFields<T: X> {
801+
#[implicit(1)]
802+
a: Option<T::Type>,
803+
#[explicit(2)]
804+
b: Option<T::Type>,
805+
}
806+
807+
assert_roundtrips::<TaggedOptionalFields<Op>>(&[
808+
(
809+
Ok(TaggedOptionalFields {
810+
a: Some(1),
811+
b: Some(3),
812+
}),
813+
b"\x30\x08\x81\x01\x01\xa2\x03\x02\x01\x03",
814+
),
815+
(Ok(TaggedOptionalFields { a: None, b: None }), b"\x30\x00"),
816+
]);
817+
818+
#[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Debug, Eq)]
819+
enum TaggedEnum<T: X> {
820+
#[implicit(0)]
821+
Implicit(T::Type),
822+
#[explicit(1)]
823+
Explicit(T::Type),
824+
}
825+
826+
assert_roundtrips::<TaggedEnum<Op>>(&[
827+
(Ok(TaggedEnum::Implicit(1)), b"\x80\x01\x01"),
828+
(Ok(TaggedEnum::Explicit(1)), b"\xa1\x03\x02\x01\x01"),
829+
]);
785830
}

0 commit comments

Comments
 (0)