Skip to content

Conversation

@bstrie
Copy link
Contributor

@bstrie bstrie commented Jan 21, 2022

This also adds a context-specific tag test to the code for #[derive(Choice)], and cleans up the tests in these files.

If you'd like I would be happy to add more tests to cover the interactions between the various options, but I admit that I'm not an ASN.1 expert so I would need some guidance as to how these options interact so that I could formulate the correct desugaring.

Closes #322.

@npmccallum
Copy link
Contributor

@bstrie I think you mean context specific, not context sensitive.

@tarcieri
Copy link
Member

I think you mean context specific, not context sensitive

Thanks to the Chomsky Hierarchy I have made this mistake several times myself 😅

@bstrie bstrie changed the title Implement context-sensitive tags on #[derive(Sequence)] Implement context-specific tags on #[derive(Sequence)] Jan 21, 2022
@npmccallum
Copy link
Contributor

@bstrie I think you need to fix the commit message wording too.

@npmccallum
Copy link
Contributor

@bstrie Also, one of the tests is failing.

@tarcieri
Copy link
Member

tarcieri commented Jan 21, 2022

One of the errors is on IssuingDistributionPointsExample:

    impl ::der::Sequence<'_> for IssuingDistributionPointExample {
        fn fields<F, T>(&self, f: F) -> ::der::Result<T>
        where
            F: FnOnce(&[&dyn der::Encodable]) -> ::der::Result<T>,
        {
            ()
        }
    }

...not sure what's happening there but the empty tuple is definitely not the right result.

Looks like something similar on the other error:

    impl<'a> ::der::Sequence<'a> for OneAsymmetricKey<'a> {
        fn fields<F, T>(&self, f: F) -> ::der::Result<T>
        where
            F: FnOnce(&[&dyn der::Encodable]) -> ::der::Result<T>,
        {
            ()
        }
    }

Perhaps the unparseable tokens are being lost?

error: proc-macro derive produced unparseable tokens

This also adds a context-specific tag test to the code for #[derive(Choice)], and cleans up the tests in these files.
@bstrie
Copy link
Contributor Author

bstrie commented Jan 25, 2022

Code updated to pass all the tests (sorry about not doing so the first time, I forgot to test with --all-features). The tests that were failing were already using context-specific tags (which I guess were just being ignored before now) in conjunction with various other field attributes. Taking into account all the supported combinations of the currently-implemented attributes was a bit tricky and required some subtle changes to make sure all the references lined up correctly, but in the end I think I managed to find a nice arrangement that doesn't involve adding state to the Lowerer or else compromising the unit-testability of the Lowerer methods. Once again I tested this with #323 to confirm that it addresses #322 .


#[derive(Sequence)]
#[asn1(tag_mode = "IMPLICIT")]
pub struct TypeCheckExpandedSequenceFieldAttributeCombinations<'a> {
Copy link
Member

Choose a reason for hiding this comment

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

Oh my 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We call that "self-documenting code". :P

@tarcieri tarcieri merged commit 0c416e9 into RustCrypto:master Jan 25, 2022
npmccallum pushed a commit to npmccallum/formats that referenced this pull request Jan 27, 2022
This also adds a context-specific tag test to the code for #[derive(Choice)], and cleans up the tests in these files.
@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.

der crate fails to emit implicit context-specific tags on SET

3 participants