-
Notifications
You must be signed in to change notification settings - Fork 44
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(ecmascript): Implement Iterator functions that don't need a helper #591
Conversation
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.
Wow, amazing work! Thank you!
I found some places where we can improve, nothing major though. Mostly related to avoiding duplicate scoping and such. One outright bug in the reduce method as well, but this is absolutely great! Looking forward to getting this merged soon! <3
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_constructor.rs
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_constructor.rs
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Outdated
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Outdated
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Outdated
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Outdated
Show resolved
Hide resolved
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_prototype.rs
Outdated
Show resolved
Hide resolved
…ed IteratorHelper
nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/iterator_constructor.rs
Outdated
Show resolved
Hide resolved
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 fixed the lifetime issues (just more required unbinding that I'd forgotten about :D ) and the one nit I had about the error message.
Will merge once tests say everything is okay. Great work, thank you very much! <3
Implements
Iterator
object,Iterator.prototype.every
,Iterator.prototype.find
,Iterator.prototype.reduce
,Iterator.prototype.some
, andIterator.prototype.toArray
.Refactors
set_default_global_bindings
to use a macro.