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

Remove need for apply_type_aliases in parsing #94

Open
SebastienGllmt opened this issue Sep 3, 2022 · 1 comment
Open

Remove need for apply_type_aliases in parsing #94

SebastienGllmt opened this issue Sep 3, 2022 · 1 comment
Labels
Quality-of-life feature Feature that helps, but is outside of the CDDL specification

Comments

@SebastienGllmt
Copy link
Collaborator

Currently in parsing.rs we have two places where we use apply_type_aliases because we don't order definitions via a dependency graph

It would be nice to remove this, but this depends on #93

@SebastienGllmt SebastienGllmt added the CDDL feature Feature that is required for proper parsing of CDDL files label Sep 3, 2022
@SebastienGllmt SebastienGllmt added Quality-of-life feature Feature that helps, but is outside of the CDDL specification and removed CDDL feature Feature that is required for proper parsing of CDDL files labels Feb 7, 2023
@rooooooooob
Copy link
Collaborator

I don't think removing this has to do with dep graph ordering. Even though we have the dep graph ordering now, for just referring to a type it's fine if you don't do anything for aliases and just let rust handle it (but we do have some non-applied aliases that get auto-swapped), but you still need local type information e.g. how it's going to be encoded in many places. This wasn't really the case years ago but now with preserve-encodings=true (and probably other features) it's important to know this. We usually use ConceptualRustType::resolve_alias_shallow() and RustType::resolve_aliases() to do this. A possibility could be to remove the ConceptualRustType::Alias variant and instead have an alias field but I'm not sure if it's worth it. You'd have the opposite problem where anywhere it's declared you'd need to look up the alias name.

tl;dr; we apply this more because some aliases are not ones that generate a type A = B; declaration and instead just swap all A's for B. We also need to know what type B is not just that it's A too. e.g. for encoding vars

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quality-of-life feature Feature that helps, but is outside of the CDDL specification
Projects
None yet
Development

No branches or pull requests

2 participants