V8 overload support ported to JSC
Created attachment 55732 [details] Patch
Comment on attachment 55732 [details] Patch Please cq+ if ok.
Created attachment 55882 [details] Patch
Comment on attachment 55882 [details] Patch WebCore/bindings/scripts/CodeGenerator.pm:378 + $function->{overloadIndex} = @{$nameToFunctionsMap{$name}}; This perl is slightly mysterious to me. This looks reasonable. It's a bit of a hack to skip the functions when the overload index is greater than one, but I think that's ok for now.
Comment on attachment 55882 [details] Patch > Index: WebCore/bindings/scripts/test/JS/JSTestObj.cpp > =================================================================== > --- WebCore/bindings/scripts/test/JS/JSTestObj.cpp (revision 59097) > +++ WebCore/bindings/scripts/test/JS/JSTestObj.cpp (working copy) > @@ -133,7 +133,7 @@ bool JSTestObjConstructor::getOwnPropert > #define THUNK_GENERATOR(generator) > #endif > > -static const HashTableValue JSTestObjPrototypeTableValues[26] = > +static const HashTableValue JSTestObjPrototypeTableValues[27] = > { > { "voidMethod", DontDelete | Function, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionVoidMethod), (intptr_t)0 THUNK_GENERATOR(0) }, > { "voidMethodWithArgs", DontDelete | Function, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionVoidMethodWithArgs), (intptr_t)3 THUNK_GENERATOR(0) }, > @@ -160,6 +160,7 @@ static const HashTableValue JSTestObjPro > { "methodWithOptionalArg", DontDelete | Function, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithOptionalArg), (intptr_t)1 THUNK_GENERATOR(0) }, > { "methodWithNonOptionalArgAndOptionalArg", DontDelete | Function, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithNonOptionalArgAndOptionalArg), (intptr_t)2 THUNK_GENERATOR(0) }, > { "methodWithNonOptionalArgAndTwoOptionalArgs", DontDelete | Function, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithNonOptionalArgAndTwoOptionalArgs), (intptr_t)3 THUNK_GENERATOR(0) }, > + { "overloadedMethod", DontDelete | Function, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadedMethod), (intptr_t)2 THUNK_GENERATOR(0) }, > { 0, 0, 0, 0 THUNK_GENERATOR(0) } > }; > > @@ -801,6 +802,80 @@ JSValue JSC_HOST_CALL jsTestObjPrototype > return jsUndefined(); > } > > +JSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethod1(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args) > +{ > + UNUSED_PARAM(args); > + if (!thisValue.inherits(&JSTestObj::s_info)) > + return throwError(exec, TypeError); > + JSTestObj* castedThis = static_cast<JSTestObj*>(asObject(thisValue)); > + TestObj* imp = static_cast<TestObj*>(castedThis->impl()); > + TestObj* objArg = toTestObj(args.at(0)); > + const String& strArg = ustringToString(args.at(1).toString(exec)); > + > + imp->overloadedMethod(objArg, strArg); > + return jsUndefined(); > +} > + > +JSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethod2(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args) > +{ > + UNUSED_PARAM(args); > + if (!thisValue.inherits(&JSTestObj::s_info)) > + return throwError(exec, TypeError); > + JSTestObj* castedThis = static_cast<JSTestObj*>(asObject(thisValue)); > + TestObj* imp = static_cast<TestObj*>(castedThis->impl()); > + TestObj* objArg = toTestObj(args.at(0)); > + > + int argsCount = args.size(); > + if (argsCount < 2) { > + imp->overloadedMethod(objArg); > + return jsUndefined(); > + } > + > + int intArg = args.at(1).toInt32(exec); > + > + imp->overloadedMethod(objArg, intArg); > + return jsUndefined(); > +} > + > +JSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethod3(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args) > +{ > + UNUSED_PARAM(args); > + if (!thisValue.inherits(&JSTestObj::s_info)) > + return throwError(exec, TypeError); > + JSTestObj* castedThis = static_cast<JSTestObj*>(asObject(thisValue)); > + TestObj* imp = static_cast<TestObj*>(castedThis->impl()); > + const String& strArg = ustringToString(args.at(0).toString(exec)); > + > + imp->overloadedMethod(strArg); > + return jsUndefined(); > +} > + > +JSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethod4(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args) > +{ > + UNUSED_PARAM(args); > + if (!thisValue.inherits(&JSTestObj::s_info)) > + return throwError(exec, TypeError); > + JSTestObj* castedThis = static_cast<JSTestObj*>(asObject(thisValue)); > + TestObj* imp = static_cast<TestObj*>(castedThis->impl()); > + int intArg = args.at(0).toInt32(exec); > + > + imp->overloadedMethod(intArg); > + return jsUndefined(); > +} > + > +JSValue JSC_HOST_CALL testObjPrototypeFunctionOverloadedMethod(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args) > +{ > + if ((args.size() == 2 && (args.at(0).isNull() || asObject(args.at(0))->inherits(JSTestObj::s_info) && (args.at(1).isNull() || args.at(1).isUndefined() || args.at(1).isString() || args.at(1).isObject()))) > + return testObjPrototypeFunctionOverloadedMethod1Callback(args); > + if ((args.size() == 1 && (args.at(0).isNull() || asObject(args.at(0))->inherits(JSTestObj::s_info)) || (args.size() == 2 && (args.at(0).isNull() || asObject(args.at(0))->inherits(JSTestObj::s_info))) > + return testObjPrototypeFunctionOverloadedMethod2Callback(args); > + if ((args.size() == 1 && (args.at(0).isNull() || args.at(0).isUndefined() || args.at(0).isString() || args.at(0).isObject()))) > + return testObjPrototypeFunctionOverloadedMethod3Callback(args); > + if (args.size() == 1) > + return testObjPrototypeFunctionOverloadedMethod4Callback(args); > + return throwError(exec, TypeError); > +} > + > JSC::JSValue toJS(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, TestObj* object) > { It is possible I am reading this wrong, but where are the implementations of testObjPrototypeFunctionOverloadedMethod1Callback and friends. Are they supposed to be called jsTestObjPrototypeFunctionOverloadedMethod1 and friends. Are they supposed to take more parameters? Can we optimize the if statement in this function so it does not do the same work multiple times (calling things like inherits)?
(In reply to comment #5) > It is possible I am reading this wrong, but where are the implementations of testObjPrototypeFunctionOverloadedMethod1Callback and friends. Nice catch. This slipped in from V8. I'll fix that. I would have probably caught that once I turned on some of the overloads in the IDLs. > Can we optimize the if statement in this function so it does not do the same work multiple times (calling things like inherits)? This can probably be done, but its very hard and with significant added complexity to the generator. In real use cases (WebGL, Canvas and XHR overloads), the primary determinant is the length of the args vector, and therefore only a small portion of the if statements are actually passed through. Also, in these overrides, the overhead of override selection is dwarfed by what these overrides usually do (e.g. copy large arrays). Therefore such an optimization won't have a big impact.
Comment on attachment 55882 [details] Patch r-ing now that I know it is wrong. I will look at making it a bit smarter in the future I guess.
Created attachment 55988 [details] Patch
Created attachment 55989 [details] Patch
Comment on attachment 55989 [details] Patch Fixed method names. Please cq+ if ok.
Comment on attachment 55989 [details] Patch Thanks!
Thank you too!
Comment on attachment 55989 [details] Patch Clearing flags on attachment: 55989 Committed r59533: <http://trac.webkit.org/changeset/59533>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/59533 might have broken GTK Linux 32-bit Release The following changes are on the blame list: http://trac.webkit.org/changeset/59530 http://trac.webkit.org/changeset/59531 http://trac.webkit.org/changeset/59532 http://trac.webkit.org/changeset/59533