Skip to content

Conversation

@npmccallum
Copy link
Contributor

@npmccallum npmccallum commented Feb 11, 2022

By default, presume that a choice enum's type implements FixedTag. For
many cases, this means that no #[asn1(...)] is required at all. In
particular, this means that deriving Choice for SEQUENCE types
without a context-specific tag is now possible.

Signed-off-by: Nathaniel McCallum [email protected]

@npmccallum npmccallum mentioned this pull request Feb 11, 2022
5 tasks
@tarcieri
Copy link
Member

I think the Choice macro could benefit from a change like this, but I'm not sure making SEQUENCE the implicit default is actually the best fix here.

Instead, I think it would make sense to avoid making any presumptions about the ASN.1 type, but try to treat it as if it impls Decode + FixedTag at decoding time, and Encode + Tagged at encoding time, and use that to either infer or query the appropriate tag for decoding/encoding.

@npmccallum npmccallum changed the title der: default to SEQUENCE tags in derive(Choice) der: use type's tag by default on derive(Choice) Feb 11, 2022
@npmccallum
Copy link
Contributor Author

@tarcieri I believe the latest version of this fixes your concerns.

Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

@npmccallum looks better now!

One suggestion

By default, presume that a choice enum's type implements FixedTag. For
many cases, this means that no `#[asn1(...)]` is required at all. In
particular, this means that deriving `Choice` for `SEQUENCE` types
without a context-specific tag is now possible.

Signed-off-by: Nathaniel McCallum <[email protected]>
@tarcieri tarcieri merged commit b51d331 into RustCrypto:master Feb 11, 2022
@npmccallum npmccallum deleted the seqdef branch March 10, 2022 20:46
@tarcieri tarcieri mentioned this pull request May 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants