Skip to content
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

Merged
merged 5 commits into from
May 2, 2020
Merged

Conversation

brian-gavin
Copy link
Contributor

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

@Razican
Copy link
Member

Razican commented Apr 28, 2020

I'm waiting for #304 to land before proceeding with this, just to avoid more merge conflicts :)

@brian-gavin
Copy link
Contributor Author

Wow, that's quite the change - no worries!

@Razican
Copy link
Member

Razican commented Apr 28, 2020

#304 has been merged :D but now this gives a lot of merge conflicts. Could you rebase?

@Razican Razican added the enhancement New feature or request label Apr 28, 2020
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
Copy link
Member

@Razican Razican left a 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 :)

boa/src/syntax/ast/keyword.rs Outdated Show resolved Hide resolved
boa/src/syntax/ast/op.rs Outdated Show resolved Hide resolved
boa/src/syntax/ast/punc.rs Outdated Show resolved Hide resolved
Copy link
Member

@HalidOdat HalidOdat left a 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.

boa/src/exec/mod.rs Show resolved Hide resolved
Copy link
Member

@Razican Razican left a 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.

boa/src/syntax/parser/expression/mod.rs Outdated Show resolved Hide resolved
boa/src/syntax/parser/expression/mod.rs Outdated Show resolved Hide resolved
Add a test for props up the prototype chain. It is currently failing,
but should be passing for this to be complete.
Copy link
Member

@HalidOdat HalidOdat left a 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. :)

@HalidOdat HalidOdat added this to the v0.8.0 milestone May 2, 2020
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge!

@Razican Razican merged commit 55ef44c into boa-dev:master May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants