-
-
Notifications
You must be signed in to change notification settings - Fork 418
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(boa): in operator #350
Conversation
I'm waiting for #304 to land before proceeding with this, just to avoid more merge conflicts :) |
Wow, that's quite the change - no worries! |
#304 has been merged :D but now this gives a lot of merge conflicts. Could you rebase? |
For parsing: The expression! macro is extended to support a Keyword typed value in its op arguments. This followed the spec that some expressions are separated by Punctuators (like == etc.) but others are separated by Keywords (like this one, or others such as instanceof). This needed an implementation of PartialEq for Keyword and Punctuator in order to fix a type error caused by the repeated match guard. An as_binop method was added to Keyword to mimic the one in Punctuator. This may be better suited to a trait implementation of TryFrom for the BinOp enum, with implementations for TryFrom<Keyword> and TryFrom<Punctuator>. For interpreting: Some abstract operations were implemented that are used in the semantics of the operator in the spec. An as_object cast method was addedto ValueData in order to get a ref to that data as an object in order to use the Object methods on a ValueData value. Tests: Add unit tests to interpreter and parser for the new operator. Solves boa-dev#88
ada5c94
to
b428acf
Compare
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.
Hey, looks pretty good! Check my comments to see if it can be improved even further. I'm not 100% convinced on the new exxpression!()
macro and the PartialEq
implementation between keywords and punctuators, I'll give it another look during the day :)
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.
Looks great! Maybe check my comment on how we can improve it even further.
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.
Looks pretty good to me :) check my comments just in case.
Add a test for props up the prototype chain. It is currently failing, but should be passing for this to be complete.
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.
Looks good to me! Thanks for the extra bug fix. :)
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.
Let's merge!
For parsing:
The expression! macro is extended to support a Keyword typed value in its
op arguments. This followed the spec that some expressions are separated
by Punctuators (like == etc.) but others are separated by Keywords (like
this one, or others such as instanceof).
This needed an implementation of PartialEq for Keyword and Punctuator in
order to fix a type error caused by the repeated match guard.
An as_binop method was added to Keyword to mimic the one in Punctuator.
This may be better suited to a trait implementation of TryFrom for the
BinOp enum, with implementations for TryFrom and
TryFrom.
For interpreting:
Some abstract operations were implemented that are used in the semantics
of the operator in the spec.
An as_object cast method was addedto ValueData in order to get a ref to
that data as an object in order to use the Object methods on a
ValueData value.
Tests:
Add unit tests to interpreter and parser for the new operator.
Solves #88