On 2011/04/27 13:17:54, Rico wrote:
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.
I am closing this, created http://codereview.chromium.org/6902104/ with
actual
addition of checks for null and undefined in native functions.
http://codereview.chromium.org/6904047/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
