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

Modernise ast #7185

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Modernise ast #7185

wants to merge 8 commits into from

Conversation

cristianoc
Copy link
Collaborator

@cristianoc cristianoc commented Dec 2, 2024

  • Move the legacy untyped ast to Parsetree0.
  • Parsetree can now be changed and modernised.
  • Ast_mapper_to0 and Ast_mapper_from0 added to convert both ways and maintained whenever Parsetree changes.
  • When a ppx is called, Parsetree is converted to legacy Parsetree0, then the ppx is run, then the result is converted back to Parsetree.

@cristianoc cristianoc marked this pull request as draft December 2, 2024 08:51
There's `Parsetree`, the current parse tree, and `Parsetree0`, the legacy parse tree.
Module `Parsetree.Legacy` contains functions to map to the legacy parse tree.
When a ppx is applied:
1 Parsetree is converted to Parsetree0
2 the ppx is run
3 the result is converted to Parsetree
@cristianoc cristianoc marked this pull request as ready for review December 3, 2024 09:37
@cristianoc cristianoc requested review from cknitt and zth December 3, 2024 09:37
Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Awesome! Let's get the AST cleanup started! 🎉

@cristianoc
Copy link
Collaborator Author

I'll merge as soon as it's tested on some ppx.

@zth
Copy link
Collaborator

zth commented Dec 3, 2024

If nobody gets to it before me I'll test it on one of my PPXes, but it might be a day or so before I get time.

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.

3 participants