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

.cbor handling incorrect for standalone types #91

Open
SebastienGllmt opened this issue Sep 2, 2022 · 2 comments · May be fixed by #112
Open

.cbor handling incorrect for standalone types #91

SebastienGllmt opened this issue Sep 2, 2022 · 2 comments · May be fixed by #112
Labels
bug Something isn't working CDDL feature Feature that is required for proper parsing of CDDL files

Comments

@SebastienGllmt
Copy link
Collaborator

SebastienGllmt commented Sep 2, 2022

#50 adds support for the .cbor notation

It does so by having foo = bytes .cbor bar simply make foo be an alias for bar at the type level and handling the cbor-in-cbor encoding in the (de)serialization logic.

This works fine when embedded as part of a larger object, but when the type is used standalone, it just leaves you with type foo = bar with no (de)serialization wrapping a-la #88

I also noticed the same issue happens with TaggedData (foo = #6.24(bytes) generates the wrong code when standalone)

Proposed Solution

I think probably the best way to fix this is to extend the upstream cddl library to give it parent pointers for all the AST types. Basically we would add a new feature flag that, when enabled, adds a parent member variable that gets filled in on a 2nd pass of the AST using the visitor utilities in the library

Then, from our library, we can use these parent pointers in parsing.rs to know if a type is top-level (needs to be wrapped) or if it's embedded (use current technique)

Having this kind of parent pointer would also improve the logic implemented in #84 as we could:

  1. Remove the struct I defined in the PR
  2. Crawl up the parent stack to fallback to the root rule name if no @name definition exists
@SebastienGllmt
Copy link
Collaborator Author

I made anweiss/cddl#134 to start work on the upstream feature we need

@anweiss
Copy link

anweiss commented Oct 14, 2022

FYSA ... anweiss/cddl#147

@SebastienGllmt SebastienGllmt added the CDDL feature Feature that is required for proper parsing of CDDL files label Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CDDL feature Feature that is required for proper parsing of CDDL files
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants