der: support for handling DEFAULT values in SEQUENCEs #202
Conversation
der/derive/src/attributes.rs
Outdated
| abort!(attr.value, "duplicate ASN.1 `tag_mode` attribute"); | ||
| } | ||
|
|
||
| default = Some(Ident::new(&default_attr_str, attrs[counter].span())); |
There was a problem hiding this comment.
This works for the Ident case, although I think to do this "properly" it should be a Path.
I think you can use the Parse trait to parse a Path from the LitStr (retaining span info automatically), and perhaps the value portion of AttrNameValue should retain the original LitStr.
der/derive/src/attributes.rs
Outdated
| } else if let Some(_default_attr_str) = attr.parse_value::<String>("default") { | ||
| if default.is_some() { | ||
| abort!(attr.value, "duplicate ASN.1 `tag_mode` attribute"); | ||
| } |
There was a problem hiding this comment.
| } else if let Some(_default_attr_str) = attr.parse_value::<String>("default") { | |
| if default.is_some() { | |
| abort!(attr.value, "duplicate ASN.1 `tag_mode` attribute"); | |
| } | |
| } else if attr.parse_value::<String>("default").is_some() { | |
| if default.is_some() { | |
| abort!(attr.value, "duplicate ASN.1 `default` attribute"); | |
| } |
|
|
||
| let mut tag_mode = None; | ||
|
|
||
| let mut default = None; |
There was a problem hiding this comment.
I think the added whitespace here is unnecessary... these should be grouped together as the fields-under-construction
| #[asn1(type = "BIT STRING")] | ||
| pub subject_public_key: &'a [u8], | ||
| pub subject_public_key: BitString<'a>, |
There was a problem hiding this comment.
Because it made the default stuff messy for not particular value to my eye. We could add it back with cost of some additional verbosity in the default bits. If we do restore, I'd prefer to wait until context specific proc macro support lands (or address in tandem with that).
There was a problem hiding this comment.
I'd really prefer to retain the functionality, although I'd be happy with fixing it in a follow-up PR after merging this one
tarcieri
changed the title
| quote! { let #ident = #field_decoder.try_into()?; } | ||
| let ty = self.field_type.clone(); | ||
| if let Some(default) = &self.attrs.default { | ||
| quote!(let mut #ident = Some(decoder.decode::<#ty>()?.unwrap_or_else(#default));) |
There was a problem hiding this comment.
Thought I'd merge this and try to clean up my remaining nits, but this seems a bit problematic. I'm not sure this is what you actually intended?
It seems like when default is present, it should attempt to decode the given type as Option<#ty>, and then call unwrap_or_default on the result, if the Option decodes successfully.
There was a problem hiding this comment.
OK. I think the Option<> on the default'ed fields will likely go away as Context Specific is worked. The goal here was to always return a value where there was a default, perhaps stretching Option a bit.
There was a problem hiding this comment.
I see now. The Option<> became unnecessary as default support was added. Originally, the absence of the field signified default and the caller had to supply the default value. Removing Option was natural next step.
PR #202 removed support for the `#[asn1(type = "...")]` attribute when deriving `Sequence`. This PR restores it, and also slightly changes the way the `default` attribute is handled, with added tests.
PR #202 removed support for the `#[asn1(type = "...")]` attribute when deriving `Sequence`. This PR restores it, and also slightly changes the way the `default` attribute is handled, with added tests. Additionally, this commit introduces a reference type for `OPTIONAL` similar to the one for `CONTEXT-SPECIFIC` called `OptionalRef` that can be used as an `Encodable` trait object for `Option<&T>`. This is used by the custom derive when encoding fields with defaults.
PR #202 removed support for the `#[asn1(type = "...")]` attribute when deriving `Sequence`. This PR restores it, and also slightly changes the way the `default` attribute is handled, with added tests. Additionally, this commit introduces a reference type for `OPTIONAL` similar to the one for `CONTEXT-SPECIFIC` called `OptionalRef` that can be used as an `Encodable` trait object for `Option<&T>`. This is used by the custom derive when encoding fields with defaults.
No description provided.