Bug 26382 - [ES5] Implement Function.prototype.bind
: [ES5] Implement Function.prototype.bind
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: ES5
:
:
  Show dependency treegraph
 
Reported: 2009-06-13 23:17 PST by
Modified: 2011-10-18 01:03 PST (History)


Attachments
Attempt to implement Function.prototype.bind() (13.14 KB, patch)
2010-04-21 14:35 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Attempt 2 to implement Function.prototype.bind() (20.23 KB, patch)
2010-04-23 07:57 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Native implementation of Function.prototype.bind() (20.22 KB, patch)
2010-04-25 15:47 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Optimization for bind() used just for binding 'this' (non JIT case) (3.74 KB, patch)
2010-04-25 15:53 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Native implementation of Function.prototype.bind(), v4 (20.10 KB, patch)
2010-05-02 18:28 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Optimization for bind() used just for binding 'this' (non JIT case) (3.73 KB, patch)
2010-05-02 18:31 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Optimization for bind() used just for binding 'this' (JIT case) (4.71 KB, patch)
2010-05-02 18:33 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Optimization for bind() used just for binding 'this' (non JIT case) (3.68 KB, patch)
2010-05-02 19:24 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Native implementation of Function.prototype.bind(), v5 (21.85 KB, patch)
2010-05-05 15:09 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Optimization for bind() used just for binding 'this' (JIT case), v2 (8.24 KB, patch)
2010-05-05 15:32 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
Native implementation of Function.prototype.bind(), v6 (25.81 KB, patch)
2010-05-12 08:37 PST, Caio Marcelo de Oliveira Filho
no flags Review Patch | Details | Formatted Diff | Diff
New patch, reentrant implementation. (77.69 KB, patch)
2011-09-22 12:20 PST, Gavin Barraclough
sam: review+
webkit-ews: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Build fixes, GC fixes, added more test cases, fix for recursive hasInstance (85.90 KB, patch)
2011-09-22 14:04 PST, Gavin Barraclough
sam: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-06-13 23:17:56 PST
ES5 adds a bind() method to Function.prototype as pioneered by the Prototype Library.

  Function.prototype.bind (thisArg [, arg1 [, arg2, …]]) 

It is defined in §15.3.4.5 of http://www.ecma-international.org/publications/files/drafts/tc39-2009-025.pdf

--------------------------------------------------

The bind method takes one or more arguments, thisArg and (optionally) arg1, arg2, etc, and returns a new function object by performing the following steps: 

1. Let Target be the this value. 

2. If IsCallable(Target) is false, throw a TypeError exception. 

3. Let A be a new (possibly empty) internal list of all of the argument values provided after thisArg (arg1, arg2 etc), in order. 

4. Let F be a new native ECMAScript object . 

5. Set the [[TargetFunction]] internal property of F to Target. 

6. Set the [[BoundThis]] internal property of F to the value of thisArg. 

7. Set the [[BoundArgs]] internal property of F to A. 

8. Set the [[Class]] internal property of F to "Function". 

9. Set the [[Prototype]] internal property of F to the standard built-in Function prototype object as  specified in 15.3.3.1. 

10. Set the [[Call]] internal property of F as described in 15.3.4.5.1. 

11. Set the [[Construct]] internal property of F as described in 15.3.4.5.2. 

12. Set the [[HasInstance]] internal property of F as described in 15.3.4.5.3. 

13. The [[Scope]] internal property of F is unused and need not exist. 

14. If the [[Class]] internal property of Target is "Function", then 

a. Let L be the length property of Target minus the length of A. 
b. Set the length own property of F to either 0 or L, whichever is larger.  

15. Else set the length own property of F to 0. 

16. The length own property of F is given attributes as specified in 15.3.5.1. 

17. Set the [[Extensible]] internal property of F to true. 

18. Call the [[DefineOwnProperty]] internal method of F with arguments "caller", PropertyDescriptor {[[Value: null, [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false}, and false. 

19. Call the [[DefineOwnProperty]] internal method of F with arguments "arguments", PropertyDescriptor {[[Value: null, [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false}, and false. 

20. Return F. 

Note: The length property of the bind method is 1. 

Note: Function objects created using Function.prototype.bind do not have a prototype property. 

[[Call]] 

When the [[Call]] internal method of a function object, F, which was created using the bind function is called with a this value and a list of arguments ExtraArgs the following steps are taken: 

1. Let boundArgs be the value of F‘s [[BoundArgs]] internal property. 

2. Let boundThis be the value of F‘s [[BoundThis]] internal property. 

3. Let target be the value of F‘s [[TargetFunction]] internal property. 

4. Let args be a new list containing the same values as the list boundArgs in the same order followed by the same values as the list ExtraArgs in the same order. 

5. Return the result of  calling the [[Call]] internal method of target providing boundThis as the this value and providing args as the arguments. 

[[Construct]] 

When the [[Construct]] internal method of a function object, F that was created using the bind function is called with a list of arguments ExtraArgs the following steps are taken: 

1. Let target be the value of F‘s [[TargetFunction]] internal property. 

2. If target has no [[Construct]] internal method, a TypeError exception is thrown. 

3. Let boundArgs be the value of F‘s [[BoundArgs]] internal property. 

4. Let args be a new list containing the same values as the list boundArgs in the same order followed by the same values as the list ExtraArgs in the same order. 

5. Return the result of  calling the [[Construct]] internal method of  target providing Args as the arguments. 

[[HasInstance]] (V) 

When the [[HasInstance]] internal method of a function object F, that was created using the bind function is called with argument V the following steps are taken: 

1. Let target be the value of F‘s [[TargetFunction]] internal property. 

2. If target has no [[HasInstance]] internal method, a TypeError exception is thrown. 

3. Return the result of  calling the [[HasInstance]] internal method of target providing V as the argument. 

--------------------------------------------------

In order to facilitate the implementation of .bind(), Object now has a number of internal properties. These are defined in §8.6.2 of the spec.

Property: [[TargetFunction]]
Value Type Domain: Object

The target function of a function object created using the standard built-in Function.prototype.bind method. Only ECMAScript objects created using Function.prototype.bind have a [[TargetFunction]] internal property. 

Property: [[BoundThis]]
Value Type Domain: any

The pre-bound this value of a function Object created using the standard built-in Function.prototype.bind method. Only ECMAScript objects created using Function.prototype.bind have a [[BoundThis]] internal property. 

Property: [[BoundArguments]]
Value Type Domain: List of any

The pre-bound argument values of a function Object created using the standard built-in Function.prototype.bind method. Only ECMAScript objects created using Function.prototype.bind have a [[BoundArguments]] internal property. 

See http://www.prototypejs.org/api/function/bind for the inspiration which led to .bind() becoming part of the spec.
------- Comment #1 From 2009-12-07 13:33:51 PST -------
*** Bug 32243 has been marked as a duplicate of this bug. ***
------- Comment #2 From 2010-01-18 20:39:27 PST -------
Would be great to have this. And here's a similar Mozilla ticket for reference — https://bugzilla.mozilla.org/show_bug.cgi?id=429507
------- Comment #3 From 2010-04-21 14:35:29 PST -------
Created an attachment (id=53992) [details]
Attempt to implement Function.prototype.bind()
------- Comment #4 From 2010-04-23 07:57:15 PST -------
Created an attachment (id=54158) [details]
Attempt 2 to implement Function.prototype.bind()

This clean up some minor issues, adds tests and more build systems support. The implementation for a very simple case (just binding 'this' in a small function without parameters), is around 4 times slower than using JS directly. When there's complexity, this difference is smaller: taking around 50% more time in more complex cases (with curried parameter and a larger function). Talking to Oliver Hunt, if I understood correctly this overhead might be due the re-entering the VM.
------- Comment #5 From 2010-04-24 19:06:10 PST -------
I don't think we should land a version of Function.prototype.bind() if it is slower than a JS-implemented version. We should look into ways to make this faster, or find a way to implement fully or partially in JS but still look native in the required ways.
------- Comment #6 From 2010-04-24 19:31:05 PST -------
The fastest implementation of `bind` I was able to come up with back in the days was the one with load-time + run-time forking (to avoid unnecessary arguments concatenation). Does native `bind` perform better than this "optimized" version?

Function.prototype.bind = (function(){

  var _slice = Array.prototype.slice;

  return function(context) {
    var fn = this,
        args = _slice.call(arguments, 1);

    if (args.length) { 
      return function() {
        return arguments.length
          ? fn.apply(context, args.concat(_slice.call(arguments)))
          : fn.apply(context, args);
      }
    } 
    return function() {
      return arguments.length
        ? fn.apply(context, arguments)
        : fn.call(context);
    }; 
  }
})();


There are also some tests in this thread: https://prototype.lighthouseapp.com/projects/8886/tickets/215-optimize-bind-bindaseventlistener
------- Comment #7 From 2010-04-24 20:30:23 PST -------
Thanks for looking into it.

For the record, the simplest case (and the one with worst performance) I was testing was comparing 'g' and 'h'

    g = function() { return obj.f(); }
    h = obj.f.bind(obj);  // using the native implementation

Agreed that we should land a solution only if it is not slower. So, I'll look into Maciej's suggestions to improve the performance and report back here.
------- Comment #8 From 2010-04-25 11:00:04 PST -------
There's a benchmark for the bind() function, which includes both Prototype.js trunk implementation and the one pointed out by kangax:

http://www.broofa.com/Tools/JSLitmus/tests/PrototypeBind.html

I've changed my patch to rename the native bind to 'bindNative' (so no collisions with any other implementation), and added a case for it.

My conclusion is that the situation isn't as bad as we thought. The difference between my previous tests is that real world 'bind()' implementations perform worse than special tuned functions that I was using.

Results in the following chart -> 
http://chart.apis.google.com/chart?chtt=Prototype%20bind()%20Performance|Ops/sec%20(Safari%20533.7%20on%20Linux)&chts=000000,10&cht=bhg&chd=t:8258481,7556237,6140297,1255825,873981,641343,1546185,981289,788777,20532293,966813,778470,20099368,2898464,2094766,4424939,3588929,509668,4555667,2904887,2065063&chds=0,20532293&chxt=x&chxl=0:|0|20.5M&chsp=0,1&chm=tnative%20bind(obj)(8.3M),000000,0,0,10|tnative%20bind(obj\,1\,2)(7.6M),000000,0,1,10|tnative%20bind(obj\,1\,2[\,3\,4])(6.1M),000000,0,2,10|t1.6.0.3%20bind(obj)(1.3M),000000,0,3,10|t1.6.0.3%20bind(obj\,1\,2)(874K),000000,0,4,10|t1.6.0.3%20bind(obj\,1\,2[\,3\,4])(641.3K),000000,0,5,10|ttrunk%20bind(obj)(1.5M),000000,0,6,10|ttrunk%20bind(obj\,1\,2)(981.3K),000000,0,7,10|ttrunk%20bind(obj\,1\,2[\,3\,4])(788.8K),000000,0,8,10|tTobie's%20bind(obj)(20.5M),000000,0,9,10|tTobie's%20bind(obj\,1\,2)(966.8K),000000,0,10,10|tTobie's%20bind(obj\,1\,2[\,3\,4])(778.5K),000000,0,11,10|tBroofa's%20bind(obj)(20.1M),000000,0,12,10|tBroofa's%20bind(obj\,1\,2)(2.9M),000000,0,13,10|tBroofa's%20bind(obj\,1\,2[\,3\,4])(2.1M),000000,0,14,10|tKangax's%20bind(obj)(4.4M),000000,0,15,10|tKangax's%20bind(obj\,1\,2)(3.6M),000000,0,16,10|tKangax's%20bind(obj\,1\,2[\,3\,4])(509.7K),000000,0,17,10|tImproved%20bind(obj)(4.6M),000000,0,18,10|tImproved%20bind(obj\,1\,2)(2.9M),000000,0,19,10|tImproved%20bind(obj\,1\,2[\,3\,4])(2.1M),000000,0,20,10&chbh=15,0,5&chs=250x490
------- Comment #9 From 2010-04-25 11:21:38 PST -------
Okay, here's my thought for getting a non-reentrant version of bind, it only works for non-partially applied bound functions (but according to Caio's numbers the native impl of partially applied bind is faster than the js version).  Basically we do this:
    DEFINE_OPCODE(op_call) {
...
    call_start:
        JSValue v = callFrame->r(func).jsValue();

        CallData callData;
        CallType callType = v.getCallData(callData);

        if (callType == CallTypeJS) {
...

        if (callType == CallTypeHost) {
             if (callData.boundFunction) {
                  callFrame->r(func) = callData.boundFunction;
                  // we actually need to work out what thisRegister is :D
                  callFrame->r(thisRegister) = static_cast<JSBoundFunction*>(v.asCell())->thisArgument();
                  goto call_start;
             }
             ...

That should save the reentry cost -- in the jit we'd probably just want to do the handling on return from host call stub.
------- Comment #10 From 2010-04-25 13:05:49 PST -------
For reference, the same chart as before but with JSC compiled without JIT:

http://chart.apis.google.com/chart?chtt=Prototype%20bind()%20Performance|Ops/sec%20(Safari%20533.7%20on%20Linux)&chts=000000,10&cht=bhg&chd=t:7791053,7395421,5339330,1009812,752429,533684,1062186,766510,586194,8581717,767408,585147,8457131,2138261,1198116,3382643,2843309,474214,3431348,2124399,1180846&chds=0,8581717&chxt=x&chxl=0:|0|8.6M&chsp=0,1&chm=tnative%20bind(obj)(7.8M),000000,0,0,10|tnative%20bind(obj\,1\,2)(7.4M),000000,0,1,10|tnative%20bind(obj\,1\,2[\,3\,4])(5.3M),000000,0,2,10|t1.6.0.3%20bind(obj)(1M),000000,0,3,10|t1.6.0.3%20bind(obj\,1\,2)(752.4K),000000,0,4,10|t1.6.0.3%20bind(obj\,1\,2[\,3\,4])(533.7K),000000,0,5,10|ttrunk%20bind(obj)(1.1M),000000,0,6,10|ttrunk%20bind(obj\,1\,2)(766.5K),000000,0,7,10|ttrunk%20bind(obj\,1\,2[\,3\,4])(586.2K),000000,0,8,10|tTobie's%20bind(obj)(8.6M),000000,0,9,10|tTobie's%20bind(obj\,1\,2)(767.4K),000000,0,10,10|tTobie's%20bind(obj\,1\,2[\,3\,4])(585.1K),000000,0,11,10|tBroofa's%20bind(obj)(8.5M),000000,0,12,10|tBroofa's%20bind(obj\,1\,2)(2.1M),000000,0,13,10|tBroofa's%20bind(obj\,1\,2[\,3\,4])(1.2M),000000,0,14,10|tKangax's%20bind(obj)(3.4M),000000,0,15,10|tKangax's%20bind(obj\,1\,2)(2.8M),000000,0,16,10|tKangax's%20bind(obj\,1\,2[\,3\,4])(474.2K),000000,0,17,10|tImproved%20bind(obj)(3.4M),000000,0,18,10|tImproved%20bind(obj\,1\,2)(2.1M),000000,0,19,10|tImproved%20bind(obj\,1\,2[\,3\,4])(1.2M),000000,0,20,10&chbh=15,0,5&chs=250x490
------- Comment #11 From 2010-04-25 15:47:53 PST -------
Created an attachment (id=54243) [details]
Native implementation of Function.prototype.bind()

Same as before, just fixes order of functions.
------- Comment #12 From 2010-04-25 15:53:47 PST -------
Created an attachment (id=54245) [details]
Optimization for bind() used just for binding 'this' (non JIT case)

This patch follows the suggestion of Oliver Hunt in the bug comments. For the non-JIT case, this will make the native implementation win all the other JavaScript-only implementations we found out. In my local tests it even gets a better result than the explicit 'function() { obj.method(); }' call.

We are using http://www.broofa.com/Tools/JSLitmus/tests/PrototypeBind.html as the benchmark.
------- Comment #13 From 2010-05-02 18:28:27 PST -------
Created an attachment (id=54896) [details]
Native implementation of Function.prototype.bind(), v4

Use Cell space available, and avoid allocating space for holding arguments if they are not used.
------- Comment #14 From 2010-05-02 18:31:12 PST -------
Created an attachment (id=54897) [details]
Optimization for bind() used just for binding 'this' (non JIT case)

Updated because of changes in previous patch.
------- Comment #15 From 2010-05-02 18:33:05 PST -------
Created an attachment (id=54898) [details]
Optimization for bind() used just for binding 'this' (JIT case)

Uses same idea from previous optimization in the JIT context. With this patch, in the linked benchmark, the native is faster all the three scenarios.
------- Comment #16 From 2010-05-02 19:24:38 PST -------
Created an attachment (id=54900) [details]
Optimization for bind() used just for binding 'this' (non JIT case)

After some discussion with Oliver and testing, it isn't always safe to rewrite the 'callee' register, since it might be reused by the bytecode. For the non-JIT case the fix is simple, just change the local variable used instead of the call frame.

The JIT version is already correct in this case, but suffer other problem: that slow cases can look at the callee register again (ex.: compileOpCallSlowCase() when OPTIMIZED_CALL is disabled).
------- Comment #17 From 2010-05-05 15:09:56 PST -------
Created an attachment (id=55158) [details]
Native implementation of Function.prototype.bind(), v5

Adds a missing 'OverridesMarkChildren' to the StructureFlags of JSBoundFunction. This is needed because JSBoundFunction implements markChildren method.

Also added new test cases to guarantee that CallFrame rewriting done by possible optimizations will not break the code.
------- Comment #18 From 2010-05-05 15:32:43 PST -------
Created an attachment (id=55161) [details]
Optimization for bind() used just for binding 'this' (JIT case), v2

This an improvement of earlier implementation of the JIT rewriting optimization.

Functionally it should be complete, still have a FIXME to check and possibly removing a redundant information passed to the stub. Note that I'm still working only in one case of Call until we settle in a solution.

After some tests and discussion with Oliver, we understood that it was not always safe to rewrite the register containing the 'callee' since it could be a non-temporary register. The solution in this patch is to force every call emitted to use a temporary for 'callee'. This causes impact on the size of bytecode generated, but guarantee that it's safe to rewrite the call frame, making the optimization correct.

I still want to give a different take on optimizing this case, and try to save/restore the 'callee' instead of guarantee that it is always a temporary. Then compare both solutions.
------- Comment #19 From 2010-05-12 08:37:45 PST -------
Created an attachment (id=55852) [details]
Native implementation of Function.prototype.bind(), v6

Fixes building in 64bits: JSBoundFunction was too large to fit in the cell, so moved other members inside the private data. Tested that on MacOSX.

Also cached the CallData information of target function, this gave a small boost in performance.
------- Comment #20 From 2010-05-19 17:06:10 PST -------
I'd really prefer that we didn't add overhead to normal calls in order to make bind faster. We're working really hard right now to remove overhead from normal calls, and it's paying off in terms of performance.
------- Comment #21 From 2010-05-19 17:12:53 PST -------
(In reply to comment #20)
> I'd really prefer that we didn't add overhead to normal calls in order to make bind faster. We're working really hard right now to remove overhead from normal calls, and it's paying off in terms of performance.

Geoff actually pointed out that the new specialised thunk stuff i added would potentially make the jit code much easier.
------- Comment #22 From 2010-05-19 17:13:41 PST -------
I think a better solution would be for the binding function to fix up callframe[callee] and callframe[this], and then jump to the bound function.
------- Comment #23 From 2011-03-31 10:38:33 PST -------
ping

The lack of this is starting to cause compat issue.
------- Comment #24 From 2011-03-31 10:45:13 PST -------
(In reply to comment #23)
> ping
> 
> The lack of this is starting to cause compat issue.

Given that JS can be used to make a reasonably performant  implementation and most sites that use Function.prototype.bind  do actually start of with an existence check and then add the js impl this is not high priority.
------- Comment #25 From 2011-04-01 07:06:20 PST -------
(In reply to comment #24)
> (In reply to comment #23)
> > ping
> > 
> > The lack of this is starting to cause compat issue.
> 
> Given that JS can be used to make a reasonably performant  implementation and most sites that use Function.prototype.bind  do actually start of with an existence check and then add the js impl this is not high priority.

JS can reasonably emulate the [[Call]] behavior of .bind() but not its [[Construct]] behavior. This is important because the [[Construct]] behavior of .bind() is the only way to implement a correct var-args construction (new) operation in ES5.
------- Comment #26 From 2011-04-27 11:51:58 PST -------
another ping

The lack of this is starting to cause compatibility issues in some of the browser-based tooling being done at Eclipse.
------- Comment #27 From 2011-04-28 07:43:16 PST -------
(In reply to comment #26)
> The lack of this is starting to cause compatibility issues in some of the browser-based tooling being done at Eclipse.

To be clear the shim is fine for our case. Just comes as a surprise that we need it for JSC given that all other current VMs provide support. Despite our best effort to inform we get folk using Function.bind and testing in other browsers and then have failing unit tests during our nightly builds.
------- Comment #28 From 2011-04-28 08:32:31 PST -------
Hello Geoffrey,

(In reply to comment #22)
> I think a better solution would be for the binding function to fix up callframe[callee] and callframe[this], and then jump to the bound function.

Is this still valid? Can you elaborate a bit on that?
------- Comment #29 From 2011-04-28 09:35:16 PST -------
(In reply to comment #25)
> 
> JS can reasonably emulate the [[Call]] behavior of .bind() but not 
> its [[Construct]] behavior. This is important because the [[Construct]] 
> behavior of .bind() is the only way to implement a correct var-args 
> construction (new) operation in ES5.

To expand on that, given the ES5 specified Function.prototype.bind, one can implement a "construct(f, args)" operation as follows:

    > function construct(f, args) {
    >   var bound = Function.prototype.bind.apply(f, [null].concat(args));
    >   return new bound();
    > }

    > construct(Date, [1957, 5, 27])
    Thu Jun 27 1957 00:00:00 GMT-0700 (PDT)

Without the specified primitive implementation of bind, I know of no way for a pure JavaScript shim to correctly emulate this behavior, and therefore no way for an ES-next to ES5 compiler to correctly compile "new f(...args)" to ES5.
------- Comment #30 From 2011-09-22 12:20:57 PST -------
Created an attachment (id=108382) [details]
New patch, reentrant implementation.

A fresh implementation.  This has three interesting features:
* Based on ToT!
* Subclasses JSFunction (this allows the JIT to make fast calls to bound functions)
* Cell size is no longer restricted (doesn't need a data object).
------- Comment #31 From 2011-09-22 12:24:25 PST -------
Attachment 108382 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/JavaScriptCore/runtime/ConstructData.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSFunction.h:47:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:47:  The parameter name "descriptor" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:57:  The parameter name "globalObject" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:57:  The parameter name "nativeFunction" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:58:  The parameter name "globalObject" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:58:  The parameter name "nativeExecutable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:134:  The parameter name "mode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSBoundFunction.h:45:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSBoundFunction.h:45:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 12 in 57 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #32 From 2011-09-22 12:33:50 PST -------
(From update of attachment 108382 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=108382&action=review

> Source/JavaScriptCore/runtime/JSBoundFunction.cpp:92
> +    // FIXME: our instanceof implementation will have already (incorrectly) performed
> +    // a [[Get]] of .prototype from the bound function object, which is incorrect!
> +    proto = m_targetFunction->get(exec, exec->propertyNames().prototype);

You should add a test showing this is incorrect.

> Source/JavaScriptCore/runtime/JSBoundFunction.h:66
> +    WriteBarrier<JSObject> m_targetFunction;
> +    WriteBarrier<Unknown> m_boundThis;
> +    WriteBarrier<Unknown> m_boundArgs;

How are these marked?
------- Comment #33 From 2011-09-22 12:38:37 PST -------
(From update of attachment 108382 [details])
Attachment 108382 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9798358
------- Comment #34 From 2011-09-22 12:50:02 PST -------
(From update of attachment 108382 [details])
Attachment 108382 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9792455
------- Comment #35 From 2011-09-22 14:04:56 PST -------
Created an attachment (id=108399) [details]
Build fixes, GC fixes, added more test cases, fix for recursive hasInstance
------- Comment #36 From 2011-09-22 14:09:05 PST -------
Attachment 108399 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

Source/JavaScriptCore/runtime/ConstructData.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSFunction.h:47:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:47:  The parameter name "descriptor" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:57:  The parameter name "globalObject" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:57:  The parameter name "nativeFunction" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:58:  The parameter name "globalObject" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:58:  The parameter name "nativeExecutable" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSFunction.h:135:  The parameter name "mode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSBoundFunction.h:45:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/JSBoundFunction.h:45:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 12 in 63 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #37 From 2011-09-22 14:21:29 PST -------
Fixed in r95751.
------- Comment #38 From 2011-09-23 03:21:30 PST -------
(From update of attachment 108399 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=108399&action=review

> LayoutTests/fast/js/Object-getOwnPropertyNames-expected.txt:46
> -PASS getSortedOwnPropertyNames(Function.prototype) is ['apply', 'call', 'constructor', 'length', 'name', 'toString']
> +FAIL getSortedOwnPropertyNames(Function.prototype) should be apply,call,constructor,length,name,toString. Was apply,bind,call,constructor,length,name,toString.

Shouldn't we update the test to expect the bind property?
------- Comment #39 From 2011-09-23 12:14:42 PST -------
(In reply to comment #38)
> (From update of attachment 108399 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108399&action=review
> 
> > LayoutTests/fast/js/Object-getOwnPropertyNames-expected.txt:46
> > -PASS getSortedOwnPropertyNames(Function.prototype) is ['apply', 'call', 'constructor', 'length', 'name', 'toString']
> > +FAIL getSortedOwnPropertyNames(Function.prototype) should be apply,call,constructor,length,name,toString. Was apply,bind,call,constructor,length,name,toString.
> 
> Shouldn't we update the test to expect the bind property?

Ooops, yes, meant to do so, clearly forgot to.  Will do.