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

AST parent pointer #134

Closed
wants to merge 1 commit into from
Closed

Conversation

SebastienGllmt
Copy link
Contributor

@SebastienGllmt SebastienGllmt commented Sep 12, 2022

Problem

Right now, there is no good way to go up the tree given a node in the AST. This kind of operation can be done outside the library, but it's hacky so I'd prefer to just add it as an optional feature here.

Proposed solution

Add a ast-parent flag that when set, building the AST will do an additional pass after the AST is completed to fill in the parent pointers.

Related work

Note that unfortunately this change will have a very large conflict with #96. Unfortunately that PR is still a draft so there was no good way to build my work on top of that PR.

Issues

Note that the visitor pattern is for some reason missing a few cases:

visit_cddl
visit_generic_args
visit_generic_params
visit_group_entry
visit_operator
visit_rule

TODO:

  • add missing visitor calls
  • Finish coding parent visitor
  • Run parent visitor to fill the AST if the compiler flag is set
  • Write tests
  • Check for build errors on different compiler flags (ex: if ast-span is off)

@anweiss
Copy link
Owner

anweiss commented Sep 26, 2022

Thanks @SebastienGllmt. You can disregard #96 for now in the context of this PR. It's been a while since I looked at that. I'd rather try and do something like what is being proposed in #125.

@SebastienGllmt SebastienGllmt marked this pull request as ready for review October 6, 2022 08:19
@SebastienGllmt
Copy link
Contributor Author

Marking this ready to review since I won't have bandwidth to personally work on this for the next while as I mentioned (but happy to review)

The two features we wanted for our use case was:

  1. Knowing whether or not a node is part of an embedded structure (ex: if `foo .bytes bar is a top-level node, or if it's a field inside a group). We wanted this because sometimes the code we have to generate is different if a type is embedded inside another or not
  2. Finding the rule name a node belongs to. We wanted this to programmatically generate names (ex: any node embedded inside a large structure can be represented by a Rust struct whose name is prefixed by the rule name)

@anweiss
Copy link
Owner

anweiss commented Oct 6, 2022

Thanks a ton for this @SebastienGllmt! This is currently being worked on in https://github.com/anweiss/cddl/tree/dcSpark-ast-parent-pointer, which is based on this PR.

@anweiss
Copy link
Owner

anweiss commented Oct 14, 2022

Closing in favor of #147

@anweiss anweiss closed this Oct 14, 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.

2 participants