-
-
Notifications
You must be signed in to change notification settings - Fork 406
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
Implement for..in #976
Implement for..in #976
Conversation
Codecov Report
@@ Coverage Diff @@
## master #976 +/- ##
==========================================
+ Coverage 60.07% 60.20% +0.12%
==========================================
Files 167 169 +2
Lines 11132 11359 +227
==========================================
+ Hits 6688 6839 +151
- Misses 4444 4520 +76
Continue to review full report at Codecov.
|
This is really a WIP and I plan to add unit tests as soon as #974 and #972 are merged, but I submit it anyway to have some guidance. I am not quite sure of what I have done, especially around the lexer and parser. @jcdickinson If you have some time, I would be happy to have your feedback |
This looks good to me, but I haven't touched the codebase in a while so I'm out of touch with the new parser. |
I didn't check the code yet, but this is looking very good: Test262 conformance changes:
|
I have implemented break and continue in for..in, and I have added some unit tests. It is now ready for review. |
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.
It's looking good. I did a small review, but I need to spend a bit more time with it. I'm not sure about unifying for...in
and for...of
, mostly because that will probably not be space efficient.
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 is impressive. The conformance improvement is vastly good, and I have checked the implementation, and I see improvements in many other loops. Thanks for implementing the changes I proposed!
This Pull Request closes #670.
It changes the following:
in
as a binary operator in afor
statementfor..in
managementForOfLoop
node (renamedForInOfLoop
) to execute thefor..in
logic