Boa: `Abstract Equality Comparison impl` implementation

[hello2dj]

• [x] https://github.com/jasonwilliams/boa/pull/383

[] == ![] should equal true not false.

more info:
ECMAScript specification

[Razican]

Good catch thanks! I'll try to check how to fix it and propose it as a good first issue :)

[Razican]

This is implemented in https://github.com/jasonwilliams/boa/blob/master/boa/src/builtins/value/mod.rs#L882-L903

It seems that this is not 100% spec compliant. So the idea is to change that code to be follow the algorithm in the spec.

[hello2dj]

i am newbee. But I want to try.

[HalidOdat]

i am newbee. But I want to try

Sure, go for it.

[Razican]

i am newbee. But I want to try.

Let us know if you need any help!

[hello2dj]

I'm working on it now, i have a quetion, this code https://github.com/jasonwilliams/boa/blob/master/boa/src/exec/mod.rs#L394-L406
why use borrow_mut.

In particular Eq, Ord and Hash must be equivalent for borrowed and owned values: x.borrow() == y.borrow() should give the same result as x == y.
moreinfo

[Razican]

why use borrow_mut.

Actually, I have no clue. Maybe @jasonwilliams can give a hint here. In any case, if you can make it work without a mut, that would be great!

[hello2dj]

This is implemented in https://github.com/jasonwilliams/boa/blob/master/boa/src/builtins/value/mod.rs#L882-L903

It seems that this is not 100% spec compliant. So the idea is to change that code to be follow the algorithm in the spec.

only changing this code is not enough. we need to implement Abstract Equality Comparison. And this need call to_primitive which implment on Interpreter. Personally, I think Abstract Equality Comparison is a recursive algorithm, but the current structure of ValueData is not friendly to implement that.

So, In my opinion, we need to implement that on Interpreter, not ValueData

more info:
ECMAScript specification
to_primitive

[Razican]

This is implemented in /boa/src/builtins/value/mod.rs@master#L882-L903
It seems that this is not 100% spec compliant. So the idea is to change that code to be follow the algorithm in the spec.

only changing this code is not enough. we need to implement Abstract Equality Comparison. And this need call to_primitive which implment on Interpreter. Personally, I think Abstract Equality Comparison is a recursive algorithm, but the current structure of ValueData is not friendly to implement that.

So, In my opinion, we need to implement that on Interpreter, not ValueData

more info:
ECMAScript specification
to_primitive

Good point, we don't have a spec-compliant equality comparison. We should properly implement the spec in this area, for both the strict equality and the abstract equality.

About where to implement it, maybe toPrimitive should be in the interpreter, but maybe the function should be passed as a parameter to the abstract equality function in the value. @HalidOdat what do you think?

Also, merging #383 will change this a bit too.

[hello2dj]

This is implemented in /boa/src/builtins/value/mod.rs@master#L882-L903
It seems that this is not 100% spec compliant. So the idea is to change that code to be follow the algorithm in the spec.

only changing this code is not enough. we need to implement Abstract Equality Comparison. And this need call to_primitive which implment on Interpreter. Personally, I think Abstract Equality Comparison is a recursive algorithm, but the current structure of ValueData is not friendly to implement that.
So, In my opinion, we need to implement that on Interpreter, not ValueData
more info:
ECMAScript specification
to_primitive

Good point, we don't have a spec-compliant equality comparison. We should properly implement the spec in this area, for both the strict equality and the abstract equality.

About where to implement it, maybe toPrimitive should be in the interpreter, but maybe the function should be passed as a parameter to the abstract equality function in the value. @HalidOdat what do you think?

Also, merging #383 will change this a bit too.

Yes, that is what I need. I should wait for https://github.com/jasonwilliams/boa/pull/383.

[HalidOdat]

About where to implement it, maybe toPrimitive should be in the interpreter, but maybe the function should be passed as a parameter to the abstract equality function in the value. @HalidOdat what do you think?

Alternatively we cold make the toPrimitive a free function that takes a Value and the context (interpreter), instead of it living in interpreter.

V8 implementation here

[Razican]

About where to implement it, maybe toPrimitive should be in the interpreter, but maybe the function should be passed as a parameter to the abstract equality function in the value. @HalidOdat what do you think?

Alternatively we cold make the toPrimitive a free function that takes a Value and the context (interpreter), instead of it living in interpreter.

V8 implementation here

Hmmm I think that what we are trying to convert to a primitive is the value, right? I think it would have more semantic sense in Value than in the wild.

I should wait for #383.

It has been merged! :tada: as soon as we decide on a design, I think it's safe to start implementing it :)

[HalidOdat]

I think it would have more semantic sense in Value than in the wild.

Yes. This is probably the best choice.

[hello2dj]

I think it would have more semantic sense in Value than in the wild.

Yes. This is probably the best choice.

we also need a static method which is the same function as Interpreter.call. This may be a huge work. According to my current understanding of boa, I can't do that.

[Razican]

we also need a static method which is the same function as Interpreter.call. This may be a huge work. According to my current understanding of boa, I can't do that.

What do you mean? Where do we need it?

[hello2dj]

@Razican sorry, My description is too vague. Abstract Equality Comparison -> to_primitive -> 'ordinary_to_primitive' -> 'Interpreter.call'

more info
boa to_primitive
v8 Abstract Equality Comparison

[Razican]

@Razican sorry, My description is too vague. Abstract Equality Comparison -> to_primitive -> 'ordinary_to_primitive' -> 'Interpreter.call'

Can't we pass a reference to the Interpreter around?

[hello2dj]

@Razican sorry, My description is too vague. Abstract Equality Comparison -> to_primitive -> 'ordinary_to_primitive' -> 'Interpreter.call'

Can't we pass a reference to the Interpreter around?

I will try it

[Razican]

I will try it

Thanks! :D

[hello2dj]

I implement a recursive version expect BigInt & NaN which boa doesn't implement them(or I am wrong). I need to do some testing on it and optimize it.

[Razican]

I implement a recursive version expect BigInt & NaN which boa doesn't implement them(or I am wrong). I need to do some testing on it and optimize it.

This sounds great! As you say BigInt is not yet supported by Boa, it's being implemented by @HalidOdat in #358.

About NaN, we do support it via a f64::NAN, so it's considered a normal numeric type.

[hello2dj]

NAN

I mean parser doesn't receive NaN, you can try

console.log(typeof NaN);  // `number`  is right

boa executes this script will panic. I will open an issue.

[Razican]

NAN

I mean parser doesn't receive NaN, you can try

console.log(typeof NaN);  // `number`  is right

boa executes this script will panic. I will open an issue.

Good catch! I'll give it a look to see what's happening.