-
Notifications
You must be signed in to change notification settings - Fork 56
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
Refactor $.compilation_unit
, optimize grammar
#269
Conversation
I've tested the hypothesis: this change makes it straightforward to allow top-level expressions (while still passing all test suites) |
cfc5fe7
to
4b2cbc2
Compare
Summary ------- `$.compilation_unit` is now a sequence of top-level stats separated by `$.semicolon` This change has two effects: - Grammar optimization: - ~10% faster generation time - lower number of parser states ([before](https://gist.github.com/susliko/f950b997a98c54bbfd88969a949346fd), [after](https://gist.github.com/susliko/236a85dce46219c5868c494d7f5cf629)) - parser size reduction from 43M to 36M - It seems to me, that handling `$._automatic_semicolon` on the top level is a prerequisite to support top-level expressions (tree-sitter#198) and leading infix operators (tree-sitter#141)
@@ -3,9 +3,9 @@ | |||
# This is an integration test to generally check the quality of parsing. | |||
|
|||
SCALA_SCALA_LIBRARY_EXPECTED=95 | |||
SCALA_SCALA_COMPILER_EXPECTED=87 | |||
SCALA_SCALA_COMPILER_EXPECTED=86 |
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.
There's a degradation in a single file:
StdAttachments.scala#L214 5 ms (ERROR [213, 13] - [214, 5]), which is minimized to
package x
trait A {
self: A =>
import a._
}
trait B {
self: B =>
class A()
val x = 1
}
It seams feasible to me to investigate the issue individually after this PR gets merged
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.
Good to see reduction in complexity. Looking forward to the further changes enabled by 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.
This looks great @susliko, thanks!
Summary
$.compilation_unit
is now a sequence of top-level statements separated by$.semicolon
This change has two effects:
$._automatic_semicolon
on the top level is a prerequisite to support top-level expressions (Top-level expressions cannot be parsed #198) and leading infix operators (Leading infix operator (infix_expression across multiple lines) #141)