Rewrite parser using LALRPOP #103
Conversation
|
Very half done right now, but thought I'd post my progress anyway... |
| @@ -0,0 +1,233 @@ | |||
| use std::iter; | |||
There was a problem hiding this comment.
This file as broken - was trying to see if I could adapt the one from parser2 to work with the current lexer and AST.
|
The most interesting part is probably the grammar definition: https://github.com/Marwes/gluon/pull/103/files#diff-1dbca9e85701699d5fce975a5912be7dR1 Still doesn't handle everything though. |
8405d9b to
08bf5ed
Compare
| OpenParen, | ||
| CloseParen, | ||
| OpenBracket, | ||
| CloseBracket, |
There was a problem hiding this comment.
Why did you change these definitions?
|
Left a bunch of comments, not all of them immediately important so feel free to ignore those for the moment. Looks like you are on the right track for the most part but if you want to do some refactorings on the current code then maybe split that into a separate commit (though it can be in the same PR), makes it a bit easier to review. |
|
Sorry for the massiveness - still pretty explaratory. Just wanted to check I wasn't strying too far off the deep-end. I'm thinking I might try to keep heading towards adapting the old lexer. What do you think? Is the rewrite a decent enough improvement to push forward, considering the pain of adapting the old lexer? |
|
I can start off by making PRs to the current data structures to make it easier to integrate it with LALRPOP, now that I know what is needed. |
Looks good 👍
The only truly tricky part of the lexer is the layout function which I think should be kept (at least for the moment). If you want to rewrite the rest (and it looks like you have done maybe 80% of that?) that's great but just doe whatever is easier there. I'd expect the rewrite of those parts to be a nice speed improvement as well. |
08bf5ed to
5f97fcb
Compare
|
Something you may not have noticed is that the current parser have a bit of functionality built in to allow it to recover from errors and still produce an AST (even if its incomplete). It is currently only used to accept empty field accesses like Is it possible have that work with LALRPOP as well? Taking it more generally, is it possible to recover from errors in LALRPOP (for the purposes of detecting multiple errors and/or to do completion or other analysis on an incomplete AST)? |
|
Not sure - I assumed that was why we had |
|
Still haven't really run the parser yet to see what it produces. |
|
|
5f97fcb to
a2c704b
Compare
|
Looks like there is a concept of fallible rules in LALRPOP though it is currently undocumented https://github.com/nikomatsakis/lalrpop/issues/40. Looks like =>? is the syntax for it but I am unsure how to actually use it. Hopefully Niko can respond and give an example. |
|
I think I'm going to try to get back to this one now! |
|
Good luck! Hopefully there are no other refactoring sidetracks needed! |
f8b823a to
7586a0f
Compare
d69725d to
a9cc210
Compare
|
Would it be possible to merge this with the token unwrap? We could make an issue to fix it before the next release. |
|
As long as no tests fails I think that's fine. Got some stuff that is
blocked by this PR as well.
…On 30 Nov 2016 09:42, "Brendan Zabarauskas" ***@***.***> wrote:
Would it be possible to merge this with the token unwrap? We could make an
issue to fix it before the next release.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#103 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA6bgO9uLIFP_FYR4NG_k3kWAwxkwnXxks5rDTbcgaJpZM4Je2Sv>
.
|
a9cc210 to
c05428e
Compare
Fixes #69