-
Notifications
You must be signed in to change notification settings - Fork 199
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
feat: trait aliases #6431
base: master
Are you sure you want to change the base?
feat: trait aliases #6431
Conversation
…mpl's for trait aliases, wip implementing impl generator
…to-many case, modify parse_item_kind to return multiple ItemKind's, generate a trait impl from the trait parser (when the trait alias is parsed), test generated impl's, test several cases in unit tests
…issue with where clauses, add docs, cargo clippy/fmt
🚀 Deployed on https://67350688300b573b6536b8d3--noir-docs.netlify.app |
NoirTrait { | ||
// Trait aliases: | ||
// trait Foo<..> = A + B + E where ..; | ||
let (bounds, where_clause, items, is_alias) = if self.eat_assign() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: instead of desugaring a trait alias in the parser itself, we could introduce a new AST node for it (maybe TraitAlias
) and desugar it later on, maybe in the Elaborator or collector. That way:
- parsing is a bit simpler
- desugaring can be tested independently
- LSP or tools working with parsed code (like the formatter) don't need to know about how trait aliases are expanded, knowing that they need to skip the synthetic impl, and can focus on better handling this new node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and it could be good to add a formatter test for this new syntax, to see if it works out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally attempted to implement this by desugaring it in the Elaborator
(based on the is_alias
flag), but it was fairly complex to deal with post-parsing types.
- Testing this approach seems a bit simpler since we can check ~equality up to spans between the parsed trait impl and the expected desugaring.
I suppose adding an is_synthetic: bool
to NoirTraitImpl
would be easy if that helps handling it in the LSP?
Sure, I'll add a formatter test for this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asterite I've added is_synthetic: bool
to NoirTraitImpl
and formatter support for trait aliases, with tests for both.
…bool parameter to NoirTraitImpl for synthetic impl's and test it
FYI @noir-lang/developerrelations on Noir doc changes. |
1 similar comment
FYI @noir-lang/developerrelations on Noir doc changes. |
Description
Problem*
Resolves #6315
Summary*
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.