Overloaded functions are currently unnecessarily duplicate argument checks. For example, let's take CanvasRenderingContext2d's stroke function. It has two overloads: void stroke(DOMPath path); void stroke(); This generates the main bindings function: EncodedJSValue JSC_HOST_CALL jsCanvasRenderingContext2DPrototypeFunctionStroke(ExecState* state) { VM& vm = state->vm(); auto throwScope = DECLARE_THROW_SCOPE(vm); UNUSED_PARAM(throwScope); size_t argsCount = std::min<size_t>(1, state->argumentCount()); if (argsCount == 0) { return jsCanvasRenderingContext2DPrototypeFunctionStroke2(state); } if (argsCount == 1) { return jsCanvasRenderingContext2DPrototypeFunctionStroke1(state); } return throwVMTypeError(state, throwScope); } and then two the helpers, jsCanvasRenderingContext2DPrototypeFunctionStroke1 and jsCanvasRenderingContext2DPrototypeFunctionStroke2. Taking a look at jsCanvasRenderingContext2DPrototypeFunctionStroke1, we see the argumentCount is once again checked, despite it already having been done in the main function: static inline JSC::EncodedJSValue jsCanvasRenderingContext2DPrototypeFunctionStroke1Caller(JSC::ExecState* state, JSCanvasRenderingContext2D* castedThis, JSC::ThrowScope& throwScope) { UNUSED_PARAM(state); UNUSED_PARAM(throwScope); auto& impl = castedThis->wrapped(); if (UNLIKELY(state->argumentCount() < 1)) return throwVMError(state, throwScope, createNotEnoughArgumentsError(state)); auto path = convert<IDLInterface<DOMPath>>(*state, state->uncheckedArgument(0), [](JSC::ExecState& state, JSC::ThrowScope& scope) { throwArgumentTypeError(state, scope, 0, "path", "CanvasRenderingContext2D", "stroke", "DOMPath"); }); RETURN_IF_EXCEPTION(throwScope, encodedJSValue()); impl.stroke(*path); return JSValue::encode(jsUndefined()); } We could further improve things by taking advantage of the cases where disambiguation takes place, and retain the type information into the helpers.
Created attachment 311093 [details] Patch
Comment on attachment 311093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311093&action=review r=me with nit. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4056 > + foreach my $function (@functions) { FYI, I'd personally if we used IDL naming consistently in the bindings generator. i.e. function -> operation. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4746 > + return if $function->{overloads} && @{$function->{overloads}} > 1; Maybe a comment to explain why we do not need argument count check for overloads?
(In reply to Chris Dumez from comment #2) > Comment on attachment 311093 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=311093&action=review > > r=me with nit. > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4056 > > + foreach my $function (@functions) { > > FYI, I'd personally if we used IDL naming consistently in the bindings > generator. i.e. function -> operation. I'll do this in another pass. Get them all at once. > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4746 > > + return if $function->{overloads} && @{$function->{overloads}} > 1; > > Maybe a comment to explain why we do not need argument count check for > overloads? Will do.
Committed r217369: <http://trac.webkit.org/changeset/217369>
<rdar://problem/32479792>