Skip to content
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

Enum length-check fixes + Enum optional field support #217

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

rooooooooob
Copy link
Collaborator

Fixes #175

Now properly checks all lengths for all variants to ensure that overlapping types parse the correct variant instead of prematurely thinking it's a subset of one.

Also fixes having CBORReadLen contributions from previous variants that tried to parse from contributing to later variant parses (possibly causing issues if it meant it already hit the limit).

Includes support for optional fields within enums that get inlined.

Tests for both cases.

Fixes #175

Now properly checks all lengths for all variants to ensure that
overlapping types parse the correct variant instead of prematurely
thinking it's a subset of one.

Also fixes having CBORReadLen contributions from previous variants that
tried to parse from contributing to later variant parses (possibly
causing issues if it meant it already hit the limit).

Includes support for optional fields within enums that get inlined.

Tests for both cases.
@@ -111,4 +111,33 @@ bounds_group_choice = [
hash //
; @name c
1, x: hash, y: hash
]

; just testing this compiles (for now? possibly have full tests in a later commit)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this comment meant to be merged in as-is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure if it was worth it to put in preserve-encodings tests since all the deser code should be tested in the other tests (the base group choice one + other tests in general) so the big worry was just about it compiling. I guess it can't hurt to have better coverage though so I added two new cases for those two definitions in the last commit.

@SebastienGllmt SebastienGllmt merged commit e3ba977 into master Dec 15, 2023
1 check passed
@SebastienGllmt SebastienGllmt deleted the overlapping-enum-opt-fields branch December 16, 2023 00:58
rooooooooob added a commit to dcSpark/cardano-multiplatform-lib that referenced this pull request Feb 27, 2024
Regen using dcSpark/cddl-codegen#225 to fix new clippy lint causing
several open PRs to fail

This also includes some enum deserialization check improvements
introduced in dcSpark/cddl-codegen#217

Also some minor updates to the .cddl spec to fix names in a few places
rooooooooob added a commit to dcSpark/cardano-multiplatform-lib that referenced this pull request Mar 7, 2024
Regen using dcSpark/cddl-codegen#225 to fix new clippy lint causing
several open PRs to fail

This also includes some enum deserialization check improvements
introduced in dcSpark/cddl-codegen#217

Also some minor updates to the .cddl spec to fix names in a few places
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.

Array/map not checked when inlined into enum
2 participants