Addressed comments

http://codereview.chromium.org/6904047/diff/1/test/mjsunit/function-call.js
File test/mjsunit/function-call.js (right):

http://codereview.chromium.org/6904047/diff/1/test/mjsunit/function-call.js#newcode1
test/mjsunit/function-call.js:1: // Copyright 2011 the V8 project
authors. All rights reserved.
On 2011/04/27 11:26:50, Lasse Reichstein wrote:
Put this file into mjsunit/bugs/bug-<bugnumber>.js
Then move it to regress when we fix the problem.
I will implement this NOW.

http://codereview.chromium.org/6904047/diff/1/test/mjsunit/function-call.js#newcode30
test/mjsunit/function-call.js:30: ['Object.prototype.toLocaleString',
On 2011/04/27 11:26:50, Lasse Reichstein wrote:
Just make this an array of the functions, not strings. I.e., remove
the quotes.

Done.

http://codereview.chromium.org/6904047/diff/1/test/mjsunit/function-call.js#newcode37
test/mjsunit/function-call.js:37: 'Function.prototype.toString',
On 2011/04/27 11:26:50, Lasse Reichstein wrote:
Function.prototype.{call,apply,bind} also throw on more than just null
and
undefined. Either include them all, or drop toString - it's not
ToObject that's
the cause of the throwing, and the exception message is likely to be
different.
Ditto for other non-generic functions (e.g.
String.prototype.toString).
As discussed offline I am adding these to a different test (but still
passing in undefined and null) to actually test that they do not
misbehave when null and undefined can be passed in as this (which could
never happen before)

http://codereview.chromium.org/6904047/diff/1/test/mjsunit/function-call.js#newcode143
test/mjsunit/function-call.js:143: 'RegExp.prototype.toString',
On 2011/04/27 11:26:50, Lasse Reichstein wrote:
All Number, Date and RegExp methods are non-generic, and fails on
everything
that's not a Number object or value, a Date object or a RegExp object,
respectively.
It's not about being null or undefined, and it's impossible to tell
whether the
TypeError is thrown because the thisValue is null/undefined or it's
the Global
object.
I.e., the test doesn't  really test what it's supposed to.
This is a general problem for all non-generic methods. I.e., restrict
the test
to only those that use ToObject or CheckObjectCoercible on this.
Again, I will still test this since the change to call will enable
functionality that was not possible before.

http://codereview.chromium.org/6904047/diff/1/test/mjsunit/function-call.js#newcode153
test/mjsunit/function-call.js:153:
eval(should_throw_on_null_and_undefined[i] + '.call(null)');
On 2011/04/27 11:26:50, Lasse Reichstein wrote:
Check both .call and .apply.
Also consider checking functions that take a callback function and
thisArg (e.g.
Array.prototype.{every,some,forEach.map,filter}, and those taking a
callback and
always passing undefined as this
(Array.prototype.{reduce,reduceRight}).
String.prototype.replace doesn't specify which "this" object it
passes.

Done.

http://codereview.chromium.org/6904047/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to