[Webkit-unassigned] [Bug 81588] Array.prototype.toString should be generic

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=81588





--- Comment #19 from Gavin Barraclough <barraclough at apple.com>  2012-04-07 19:44:07 PST ---
(In reply to comment #17)
> I think this patch might not be sufficient. If someone overrides Array.prototype.join, arrays need to reflect that. For example:
> 
> Array.prototype.join = function() { return 'join' }
> shouldBeEqualToString('[0, 1, 2].toString()', 'join');

Agreed - please add this test case.  Also, due to the 'if (thisValue.isCell()) {'check, I think this will do the wrong think for non-cell this values, e.g.:

Number.prototype.join = function() { return "number join"; }
Array.prototype.toString.call(42);

> It seems like the right thing to do is to not special case real arrays and always call join as the spec says.

>From a quick glance it looks like toString is more optimized that join, we could look at similarly optimizing join, but without doing so we wouldn't want to ditch the optimization from toString, and changing functionality and introducing optimizations in the same patch is a bad plan.  So it would be best for this patch to fix toString without either removing optimizations from toString or introducing them to join.

The 'inherits' check in the current patch is also bogus (not all functions inherit from JSFunction).  If we did need to check for functions, the right thing to do would be to would be to check .isFunction - however in this case the spec does not ask for the value to be a function, rather that it is callable (these may be one and the same for objects defined in the ES spec, but is not the same thing in the DOM).  The way to check for this is to check the returned CallType for CallTypeNone.  Rather than creating a new Identifier every time toString is called, one should be added to CommonIdentifiers.  The function call is passing undefined as the this value, this is also wrong.  When implementing something like this, I'd suggest the easiest way is to start by copying the text from the spec, and using this a comments to follow, to help make sure your steps are all correct!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.