Implement overload resolution algorithm: http://heycam.github.io/webidl/#es-overloads
Created attachment 284978 [details] WIP patch Early WIP Patch but should pass the tests.
Created attachment 284979 [details] WIP patch
Comment on attachment 284979 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=284979&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1648 > +# http://heycam.github.io/webidl/#dfn-effective-overload-set I like the idea of linking to the URL of an algorithm, but in this case I am concerned that this URL will not be stable and also the fact that the algorithm might change and this won’t point to what is implemented here. We should consider what URLs to put here and why we are putting them here and maybe even write some words explaining that.
Some github-based specs have commit-based URLS. For example, https://streams.spec.whatwg.org/commit-snapshots/ba5cc069302db2e325815af7d2cd1e743e12773f/ Maybe WebIDL GitHub spec could add it.
Created attachment 285022 [details] WIP Patch Shaping up nicely. Implements most of the spec but now still needs some clean up / simplification.
Created attachment 285032 [details] WIP Patch
Created attachment 285037 [details] WIP Patch
Created attachment 285042 [details] WIP Patch
Created attachment 285051 [details] WIP Patch
Created attachment 285053 [details] Patch
Comment on attachment 285053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285053&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1776 > +# http://heycam.github.io/webidl/#es-overloads I kept the spec links for now. I do feel that it helps understanding the code for as long as those links stay valid. Also, I was not able to find links to specific GIT revisions. If you feel strongly about removing those, I can move them to the ChangeLog.
FYI, I filed a few bugs / requests for clarification against the spec: - https://github.com/heycam/webidl/issues/138 - https://github.com/heycam/webidl/issues/139 - https://github.com/heycam/webidl/issues/140
EWS is all green, patch is ready for review.
Comment on attachment 285053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285053&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1776 >> +# http://heycam.github.io/webidl/#es-overloads > > I kept the spec links for now. I do feel that it helps understanding the code for as long as those links stay valid. Also, I was not able to find links to specific GIT revisions. > If you feel strongly about removing those, I can move them to the ChangeLog. Lets talk in the future about my concern here; I don’t want to hold up this patch but I want us to leave behind good quality links that are not confusing for the person looking in the future. There’s nothing wrong with having links but it’s better if the links are not going to get stale. And also, if the link points to an evergreen document then it will be confusing if the implementation matches a particular moment in time. I am not proposing links to specific git revisions. But I am proposing adding a few well chosen words to give context to explain what the link is. I have experience with confusing 5-to-10-year-old links in WebKit code to websites that don’t even exist that used to host specifications. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1801 > + return ($type eq "Dictionary" || $codeGenerator->IsDictionaryType($type)); No need for these parentheses. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1805 > + return ($isNullable || &$isDictionaryParameter($type, $optionality, $isNullable)); Ditto. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1809 > + return ($type eq "RegExp" || $type eq "object"); Ditto. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1813 > + return ($type eq "object" || $type eq "Error"); Ditto. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1818 > + return ($type eq "DOMException") Ditto. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1822 > + return ($type eq "object" || $codeGenerator->IsFunctionOnlyCallbackInterface($type)); Ditto. > LayoutTests/webgl/1.0.2/resources/webgl_test_files/conformance/textures/tex-image-with-invalid-data.html:92 > +// We do not do strict type checkinf for ArrayBufferView type. Typo: checking with an f
Comment on attachment 285053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285053&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1801 >> + return ($type eq "Dictionary" || $codeGenerator->IsDictionaryType($type)); > > No need for these parentheses. I could have sworn I had to add them or this subroutine did not do the right thing... Anyway, I'll try and remove them and make sure it generates the same code.
Created attachment 285115 [details] Patch
Comment on attachment 285115 [details] Patch Clearing flags on attachment: 285115 Committed r204028: <http://trac.webkit.org/changeset/204028>
All reviewed patches have been landed. Closing bug.
*** Bug 160385 has been marked as a duplicate of this bug. ***