cddl-codegen icon indicating copy to clipboard operation
cddl-codegen copied to clipboard

Remove need for `apply_type_aliases` in parsing

Open SebastienGllmt opened this issue 3 years ago • 1 comments

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 avatar Sep 03 '22 01:09 SebastienGllmt

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

rooooooooob avatar Dec 19 '23 18:12 rooooooooob