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

Simplify sytax trees API #448

Closed
matklad opened this issue Jan 7, 2019 · 4 comments · Fixed by #449
Closed

Simplify sytax trees API #448

matklad opened this issue Jan 7, 2019 · 4 comments · Fixed by #449
Labels
fun A technically challenging issue with high impact

Comments

@matklad
Copy link
Member

matklad commented Jan 7, 2019

Looks like I've found a way to significanlty simplify our syntax tree API: instead of using Foo<'a>/FooNode and SyntaxNodeRef<'a>/SyntaxNode we'll be using &'a Foo/TreePtr<Foo> and &'a SyntaxNode/TreePtr<SyntaxNode>, where TreePtr has a deref impl.

This is analogous how one may work with &'a Datum/Arc<Datum>, and indeed TreePtr is Arc underneath.

This is both a good news (should be a massive simplification of API) and a bad news (rewrite everything)

It might also be the case that I am just crazy and sleep deprivated and the idea does not work after all :-)

I'll write a full description tomorrow, here's the PoC: rust-analyzer/rowan@c3f719d

@matklad matklad added the fun A technically challenging issue with high impact label Jan 7, 2019
@csmoe
Copy link
Member

csmoe commented Jan 7, 2019

assign to me!

@matklad
Copy link
Member Author

matklad commented Jan 7, 2019

@csmoe I’d prefer to handle this myself, given the amount of changes here :-)

@csmoe
Copy link
Member

csmoe commented Jan 7, 2019

@matklad okay :)

@matklad
Copy link
Member Author

matklad commented Jan 7, 2019

Some docs about the new impl: rust-analyzer/rowan#9

bors bot added a commit that referenced this issue Jan 8, 2019
449: switch to new rowan API r=matklad a=matklad

closes #448

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@bors bors bot closed this as completed in #449 Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fun A technically challenging issue with high impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants