Feature Function.prototype.call

[RageKnify]

This Pull Request fixes/closes #797.
It changes the following:

-Implement Function.prototype.call
-Add a test to ensure it works

[codecov]

Codecov Report

Merging #805 into master will increase coverage by 0.04%.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##           master     #805      +/-   ##
==========================================
+ Coverage   59.62%   59.67%   +0.04%     
==========================================
  Files         155      155              
  Lines        9964     9973       +9     
==========================================
+ Hits         5941     5951      +10     
+ Misses       4023     4022       -1     

Impacted Files	Coverage Δ
boa/src/builtins/regexp/mod.rs	70.25% <25.00%> (-0.91%)	⬇️
boa/src/builtins/function/mod.rs	63.80% <85.71%> (+1.56%)	⬆️
boa/src/builtins/object/mod.rs	61.05% <0.00%> (+2.10%)	⬆️
boa/src/value/display.rs	88.63% <0.00%> (+2.27%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff25a8a...2646951. Read the comment docs.

[RageKnify]

Forgot to end the error message and write a test for it, I'm doing it right now.

[HalidOdat]

Maybe we should test it with more that one argumet

[RageKnify]

Maybe we should test it with more that one argumet

Reasonable, any idea for a function to test?

[HalidOdat]

Reasonable, any idea for a function to test?

We can test any function that takes 1 or more arumunts ( not including this ), how about Number.prototype.toString( [radix] ) it takes an optional base we can make the base 16 and assert that the number is in hexadecimal notation.

[RageKnify]

I came up with:

function f(a, b){
this.a = a;
this.b = b;
}
let o = {a: 0, b: 0}
f.call(o, 1, 2)

Then we check o.a == 1 and o.b == 2, what do you think @HalidOdat ?

[HalidOdat]

Then we check o.a == 1 and o.b == 2, what do you think @HalidOdat ?

Yes. This looks good :)

[HalidOdat]

Looks perfect to me!

[Razican]

Looks good to me too :D

[Razican]

Somehow it seems we didn't see that this was failing the test262 suite. I think we should revert it until we find the issue. What do you think @HalidOdat @RageKnify ?

[HalidOdat]

I think it maybe the missing // TODO?: 3. Perform PrepareForTailCall.

[HalidOdat]

I think we should revert it until we find the issue. What do you think @HalidOdat @RageKnify ?

Yes. I think so too.

EDIT: Or we can comment it out. with a todo, and ignore the tests

[RageKnify]

I'd like to identify which test of test262 is causing the failure. But I'm assuming its a long recursion that requires TailCall optimization, not sure how boa would handle that...

[Razican]

I'd like to identify which test of test262 is causing the failure. But I'm assuming its a long recursion that requires TailCall optimization, not sure how boa would handle that...

Something that can be done to see the last test to be executed is to uncomment this line locally and run it like this.

The error message was this:

memory allocation of 51539607528 bytes failed/home/runner/work/_temp/445e69cc-9b3d-4bcd-887f-44df8e8c1c9b.sh: line 2:  3393 Aborted                 (core dumped) cargo run --release --bin boa_tester -- -o ../gh-pages/test262

In any case, if it can't be fixed easily, we should revert this until we get a proper fix, we can't have the CI and the test262 results failing on every commit.

[RageKnify]

I've identified the test, test/built-ins/Array/prototype/map/15.4.4.19-3-28.js:

// Copyright (c) 2012 Ecma International.  All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-array.prototype.map
description: Array.prototype.map - value of 'length' is boundary value (2^32)
---*/

function callbackfn(val, idx, obj) {
  return val > 10;
}

var obj = {
  0: 12,
  length: 4294967296
};
assert.throws(RangeError, function() {
  var newArr = Array.prototype.map.call(obj, callbackfn);
});

I don't think the problem is recursion, it is Array.prototype.map not being spec compliant, it's trying to create a huge Vec and dies.
https://tc39.es/ecma262/#sec-array.prototype.map
~~Point 6.c (HasProperty(O, Pk)) isn't respected in:

boa/boa/src/builtins/array/mod.rs

Lines 598 to 607 in eb1fd28

	let values: Vec<Value> = (0..length)
	.map(|idx| {
	let element = this.get_field(idx);
	let args = [element, Value::from(idx), new.clone()];
	interpreter
	.call(&callback, &this_val, &args)
	.unwrap_or_else(|_| Value::undefined())
	})
	.collect();

~~The fix doesn't seem too complicated.

PS: I'm busy right now but I hope to get it fixed in the next 6 hours.~~

PPS: Testing the same code on Chrome the problem in the JS code seems to be that 4294967296 is 2^32, which results in RangeError: Invalid array length

So following the spec the calls should be Array.prototype.map -> ArraySpeciesCreate(O, len) -> ArrayCreate(len) and ArrayCreate has: 3. If length > 2^32 - 1, throw a RangeError exception.