Implement v8-like overload resolving.

Implement v8-like overload resolving.

This is needed to fix indexeddb_3_test and several bugs related to overload dispatching.

R=antonm@google.com
BUG=3353
TEST=indexeddb_3_test

Committed: https://code.google.com/p/dart/source/detail?r=8278

[Anton Muhin, 2012-06-04 14:27:37]

Pretty cool.

1st round of comments.

And, please, explain what happened to RequiredCppParameter?

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
File lib/dom/scripts/systemnative.py (right):

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:769: is_custom = 'Custom' in
info.overloads[0].ext_attrs
'Custom' in operation.ext_attrs?

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:790: version = [1]
it looks like you actually using version as a scalar value (version[0] only)

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:792: def GenerateChecksAndCall(operation,
argument_count, checks):
nit: maybe move this function to the very top of _GenerateDispatcher

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:802: call = call.Emit('      return $!CALL;\n')
cosmetic: that will give unnecessary indent if check is omitted.

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:823: checks = ['%s === null' % name for name in
argument_names]
checks for all arguments look wrong

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:824: for (position, argument) in
enumerate(operation.arguments):
shouldn't you go here backwards too?

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:834: for position in
range(len(operation.arguments) - 1, -1, -1):
I'd rather code that

for position, argument in list(enumerate(operation.arguments))[::-1]

but probably that's too much :)

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:836: if IsOptional(argument) and not 'Callback'
in argument.ext_attrs:
it looks like not 'Callback' should belong to IsOptional

https://chromiumcodereview.appspot.com/10513004/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemnative.py:839: GenerateChecksAndCall(operation,
len(operation.arguments), None)
[] instead of None?

https://chromiumcodereview.appspot.com/10513004/diff/1/tests/html/html.status
File tests/html/html.status (right):

https://chromiumcodereview.appspot.com/10513004/diff/1/tests/html/html.status...
tests/html/html.status:13: indexeddb_3_test: Skip  # Issue 3311
Why Skip?  It's almost always a bad idea.

[podivilov, 2012-06-04 16:12:55]

RequiredCppParameter was a hacky way to fix overload resolving for optional
callbacks. It was probably needed because of "arg.is_optional" vs "'Optional' in
arg.ext_attrs'" mess.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.py
File lib/dom/scripts/systemnative.py (right):

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:769: is_custom = 'Custom' in
info.overloads[0].ext_attrs
On 2012/06/04 14:27:37, antonmuhin wrote:
> 'Custom' in operation.ext_attrs?

Done.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:790: version = [1]
On 2012/06/04 14:27:37, antonmuhin wrote:
> it looks like you actually using version as a scalar value (version[0] only)

Yes, but version = 1 doesn't work since it's not visible from closure. Isn't it
the simplest workaround?

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:792: def GenerateChecksAndCall(operation,
argument_count, checks):
On 2012/06/04 14:27:37, antonmuhin wrote:
> nit: maybe move this function to the very top of _GenerateDispatcher

Done.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:823: checks = ['%s === null' % name for name in
argument_names]
On 2012/06/04 14:27:37, antonmuhin wrote:
> checks for all arguments look wrong

Why? We don't have arguments.length in Dart.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:824: for (position, argument) in
enumerate(operation.arguments):
On 2012/06/04 14:27:37, antonmuhin wrote:
> shouldn't you go here backwards too?

No, here the scheme is different. There are always checks for all parameters so
the order doesn't matter.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:834: for position in
range(len(operation.arguments) - 1, -1, -1):
On 2012/06/04 14:27:37, antonmuhin wrote:
> I'd rather code that
> 
> for position, argument in list(enumerate(operation.arguments))[::-1]
> 
> but probably that's too much :)

Done.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:836: if IsOptional(argument) and not 'Callback'
in argument.ext_attrs:
On 2012/06/04 14:27:37, antonmuhin wrote:
> it looks like not 'Callback' should belong to IsOptional

Not really. IsOptional returns true iff parameter should be declared as optional
in the interface. Callbacks could be optional in Dart, but are always passed to
native implementation.

http://codereview.chromium.org/10513004/diff/1/lib/dom/scripts/systemnative.p...
lib/dom/scripts/systemnative.py:839: GenerateChecksAndCall(operation,
len(operation.arguments), None)
On 2012/06/04 14:27:37, antonmuhin wrote:
> [] instead of None?

Done.

http://codereview.chromium.org/10513004/diff/1/tests/html/html.status
File tests/html/html.status (right):

http://codereview.chromium.org/10513004/diff/1/tests/html/html.status#newcode13
tests/html/html.status:13: indexeddb_3_test: Skip  # Issue 3311
On 2012/06/04 14:27:37, antonmuhin wrote:
> Why Skip?  It's almost always a bad idea.

It now hangs. But it's actually an improvement :)
See http://codereview.chromium.org/10513006/.

[sra1, 2012-06-04 18:43:10]

https://chromiumcodereview.appspot.com/10513004/diff/1007/tests/html/html.status
File tests/html/html.status (right):

https://chromiumcodereview.appspot.com/10513004/diff/1007/tests/html/html.sta...
tests/html/html.status:13: indexeddb_3_test: Skip  # Issue 3311
Why Skip?

[Anton Muhin, 2012-06-04 18:45:29]

https://chromiumcodereview.appspot.com/10513004/diff/1007/tests/html/html.status
File tests/html/html.status (right):

https://chromiumcodereview.appspot.com/10513004/diff/1007/tests/html/html.sta...
tests/html/html.status:13: indexeddb_3_test: Skip  # Issue 3311
This tests timeouts.  I believe Pavel has a fix LGTMed

On 2012/06/04 18:43:10, sra1 wrote:
> Why Skip?