-
Notifications
You must be signed in to change notification settings - Fork 223
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
Split off definition of AST into a separate crate #4852
Comments
5 tasks
Talked about this at standup: I'm alright with splitting off the monomorphized AST to see if it'll improve compile-times. No longer needing to recompile the evaluator after the frontend is changed would be nice. If we have more dependencies on data structures cross crates in the future (e.g. the initial AST and the formatter) we should consider breaking those up as well. |
github-merge-queue bot
pushed a commit
that referenced
this issue
Apr 22, 2024
…4862) # Description ## Problem\* Related to #4852 ## Summary\* This PR removes the wildcard import of the `ast` module into the root of the `noirc_frontend` crate. This encourages us to be more qualified usage of the ast to make it clearer about which we're working with. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Apr 25, 2024
# Description ## Problem\* Resolves #4910 ## Summary\* We add a new attribute `Inline(String)`. Currently we only support one `InlineType` variant of `Never`. This PR also moves `InlineType` into the monomorphization ast as its functionality is expected to be shared across various frontend passes as well as the SSA/ACIR gen. ## Additional Context I know there is some plans to split the AST off into its own crate as per (#4852). This change shouldn't affect make the split much more dififcult as the evaluator already depends on the AST as the issue mentions and this `InlineType` is pretty isolated in its usage. ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [X] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
noirc_evaluator
currently depends onnoirc_frontend
because it makes usage of the AST which is defined innoirc_frontend
. It would then help speed up our compilation times if we can split the AST definition into a separatenoirc_ast
crate whichnoirc_frontend
andnoirc_evaluator
can depend on.Beyond the compilation times, this would also allow us to split off some more crates from
noirc_frontend
for organisation such as the parser.For reference, Rust does the same and explicitly makes no stability guarantees about this crate: https://doc.rust-lang.org/stable/nightly-rustc/rustc_ast/index.html
cc @jfecher as I imagine you made a good deal of the decisions on the current structure plus you've expressed views on this in the past.
The text was updated successfully, but these errors were encountered: