New dispatch from dom into native code.

lib/dom/scripts/systemnative.py

Index: lib/dom/scripts/systemnative.py
diff --git a/lib/dom/scripts/systemnative.py b/lib/dom/scripts/systemnative.py
index 4d45361ccdc11233f7aa608863259f83359b93ba..2b00ccfadfec6ee2158bcc41f6f3600814aa5ef5 100644
--- a/lib/dom/scripts/systemnative.py
+++ b/lib/dom/scripts/systemnative.py
@@ -839,16 +839,58 @@ class NativeImplementationGenerator(object):
         NAME=info.name,
         PARAMETERS=info.ParametersImplementationDeclaration())
 
-    # Process in order of ascending number of arguments to ensure missing
-    # optional arguments are processed early.
-    overloads = sorted(info.overloads,
-                       key=lambda overload: len(overload.arguments))
     self._native_version = 0
-    fallthrough = self.GenerateDispatch(body, info, '    ', 0, overloads)
+    overloads = self.CombineOverloads(info.overloads)
+    fallthrough = self.GenerateDispatch(body, info, '    ', overloads)
     if fallthrough:
       body.Emit('    throw "Incorrect number or type of arguments";\n');
 
-  def GenerateDispatch(self, emitter, info, indent, position, overloads):
+  def CombineOverloads(self, overloads):

[Anton Muhin, 2012/05/18 11:33:40]

that still looks weird to me to recombine overloads after expansion, but, ok, up
to you

[sra1, 2012/05/23 00:30:39]

Think of it as an optimization.  Without it, the generated code is bigger, there
are more native methods, but the types of the extracted arguments in the extra
native methods are the same, so the final functioning is the same.

+    # Combine overloads that can be implemented by the same native method.  This
+    # undoes the expansion of optional arguments into multiple overloads unless
+    # IDL merging has made the overloads necessary.  Starting with overload with
+    # no optional arguments and grow it by adding optional arguments, then the
+    # longest overload can serve for all the shorter ones.
+    out = []
+    seed_index = 0
+    while seed_index < len(overloads):
+      seed = overloads[seed_index]
+      if len(seed.arguments) > 0 and seed.arguments[-1].is_optional:
+        # Must start with no optional arguments.
+        out.append(seed)
+        seed_index += 1
+        continue
+
+      prev = seed
+      probe_index = seed_index + 1
+      while probe_index < len(overloads):
+        probe = overloads[probe_index]
+        if len(probe.arguments) != len(prev.arguments) + 1:
+          break
+        if probe.arguments[0:-1] != prev.arguments:

[Anton Muhin, 2012/05/18 11:33:40]

nit: probe.arguments[:-1] should do too.

[sra1, 2012/05/23 00:30:39]

Done.

+          break
+        if not probe.arguments[-1].is_optional:
+          break
+        probe_index += 1
+        prev = probe
+      out.append(prev)
+      seed_index = probe_index
+
+    return out
+
+  def PrintOverloadsComment(self, emitter, info, indent, note, overloads):
+    emitter.Emit('$(INDENT)//$NOTE\n', INDENT=indent, NOTE=note)
+    for operation in overloads:
+      params = ', '.join([
+          ('[Optional] ' if arg.is_optional else '') + DartType(arg.type.id) + ' '
+          + arg.id for arg in operation.arguments])
+      emitter.Emit('$(INDENT)// $NAME($PARAMS)\n',
+                   INDENT=indent,
+                   NAME=info.name,
+                   PARAMS=params)
+    emitter.Emit('$(INDENT)//\n', INDENT=indent)
+
+  def GenerateDispatch(self, emitter, info, indent, overloads):
     """Generates a dispatch to one of the overloads.
 
     Arguments:
@@ -856,7 +898,7 @@ class NativeImplementationGenerator(object):
       info: the compound information about the operation and its overloads.
       indent: an indentation string for generated code.
       position: the index of the parameter to dispatch on.
-      overloads: a list of the remaining IDLOperations to dispatch.
+      overloads: a list of the IDLOperations to dispatch.
 
     Returns True if the dispatch can fall through on failure, False if the code
     always dispatches.
@@ -868,103 +910,76 @@ class NativeImplementationGenerator(object):
     def TypeCheck(name, type):
       return '%s is %s' % (name, type)
 
-    def ShouldGenerateSingleOperation():
-      if position == len(info.param_infos):
-        if len(overloads) > 1:
-          raise Exception('Duplicate operations ' + str(overloads))
-        return True
+    def IsNullable(type):
+      #return type != 'int' and type != 'num'
+      return True
 
+    def ShouldGenerateSingleOperation():
       # Check if we dispatch on RequiredCppParameter arguments.  In this
       # case all trailing arguments must be RequiredCppParameter and there
       # is no need in dispatch.
       # TODO(antonm): better diagnositics.
-      if position >= len(overloads[0].arguments):
-        def IsRequiredCppParameter(arg):
-          return 'RequiredCppParameter' in arg.ext_attrs
-        last_overload = overloads[-1]
-        if (len(last_overload.arguments) > position and
-            IsRequiredCppParameter(last_overload.arguments[position])):
-          for overload in overloads:
-            args = overload.arguments[position:]
-            if not all([IsRequiredCppParameter(arg) for arg in args]):
-              raise Exception('Invalid overload for RequiredCppParameter')
+      def IsRequiredCppParameter(arg):
+        return 'RequiredCppParameter' in arg.ext_attrs
+      def HasRequiredCppParameters(op):
+        matches = filter(IsRequiredCppParameter, op.arguments)

[Anton Muhin, 2012/05/18 11:33:40]

that is somewhat complicated to read, maybe use itertools.dropwhile?

+        if matches:
+          # Validate all following arguments are RequiredCppParameter.
+          rematches = filter(IsRequiredCppParameter,
+                             op.arguments[len(op.arguments) - len(matches):])
+          if len(matches) != len(rematches):
+            raise Exception('Invalid overload for RequiredCppParameter')
           return True
-
-      return False
+        else:
+          return False
+      return any(HasRequiredCppParameters(op) for op in overloads)
 
     if ShouldGenerateSingleOperation():
       self.GenerateSingleOperation(emitter, info, indent, overloads[-1])
       return False
 
-    # FIXME: Consider a simpler dispatch that iterates over the
-    # overloads and generates an overload specific check.  Revisit
-    # when we move to named optional arguments.
-
-    # Partition the overloads to divide and conquer on the dispatch.
-    positive = []
-    negative = []
-    first_overload = overloads[0]
-    param = info.param_infos[position]
-
-    if position < len(first_overload.arguments):
-      # FIXME: This will not work if the second overload has a more
-      # precise type than the first.  E.g.,
-      # void foo(Node x);
-      # void foo(Element x);
-      type = DartType(first_overload.arguments[position].type.id)
-      test = TypeCheck(param.name, type)
-      pred = lambda op: len(op.arguments) > position and DartType(op.arguments[position].type.id) == type
-    else:
-      type = None
-      test = NullCheck(param.name)
-      pred = lambda op: position >= len(op.arguments)
-
-    for overload in overloads:
-      if pred(overload):
-        positive.append(overload)
+    # Print just the interesting sets of overloads.
+    if len(overloads) > 1 or len(info.overloads) > 1:
+      self.PrintOverloadsComment(emitter, info, indent, '', info.overloads)
+      if overloads != info.overloads:
+        self.PrintOverloadsComment(emitter, info, indent, ' -- reduced:',
+                                   overloads)
+
+    # Match each operation in turn.
+    # TODO: Optimize the dispatch to avoid repeated tests.
+    fallthrough = True
+    for operation in overloads:
+      tests = []
+      for position in range(0, len(info.param_infos)):

[Anton Muhin, 2012/05/18 11:33:40]

nit: for (position, param) in enumerate(info.param_infos): ?

[sra1, 2012/05/23 00:30:39]

Done.

+        param = info.param_infos[position]
+        if position < len(operation.arguments):
+          arg = operation.arguments[position]
+          type = DartType(arg.type.id)

[Anton Muhin, 2012/05/18 11:33:40]

nit: dart_type instead of type?

[sra1, 2012/05/23 00:30:39]

Done.

+          if type == param.dart_type:

[Anton Muhin, 2012/05/18 11:33:40]

this check looks somewhat hacky, maybe it should be something like type !=
'Dynamic'?

[sra1, 2012/05/23 00:30:39]

Done.

+            test = None
+          else:
+            test = TypeCheck(param.name, type)
+            if IsNullable(type) or arg.is_optional:
+              test = '(%s || %s)' % (NullCheck(param.name), test)
+        else:
+          test = NullCheck(param.name)
+        if test:
+          tests.append(test)
+      if tests:
+        cond = ' && '.join(tests)
+        if len(cond) + len(indent) + 7 > 80:

[Anton Muhin, 2012/05/18 11:33:40]

:)

+          cond = (' &&\n' + indent + '    ').join(tests)
+        call = emitter.Emit(
+            '$(INDENT)if ($COND) {\n'
+            '$!CALL'
+            '$(INDENT)}\n',
+            COND=cond,
+            INDENT=indent)
+        self.GenerateSingleOperation(call, info, indent + '  ', operation)
       else:
-        negative.append(overload)
-
-    if positive and negative:
-      (true_code, false_code) = emitter.Emit(
-          '$(INDENT)if ($COND) {\n'
-          '$!TRUE'
-          '$(INDENT)} else {\n'
-          '$!FALSE'
-          '$(INDENT)}\n',
-          COND=test, INDENT=indent)
-      fallthrough1 = self.GenerateDispatch(
-          true_code, info, indent + '  ', position + 1, positive)
-      fallthrough2 = self.GenerateDispatch(
-          false_code, info, indent + '  ', position, negative)
-      return fallthrough1 or fallthrough2
-
-    if negative:
-      raise Exception('Internal error, must be all positive')
-
-    # All overloads require the same test.  Do we bother?
-
-    # If the test is the same as the method's formal parameter then checked mode
-    # will have done the test already. (It could be null too but we ignore that
-    # case since all the overload behave the same and we don't know which types
-    # in the IDL are not nullable.)
-    if type == param.dart_type:
-      return self.GenerateDispatch(
-          emitter, info, indent, position + 1, positive)
-
-    # Otherwise the overloads have the same type but the type is a subtype of
-    # the method's synthesized formal parameter. e.g we have overloads f(X) and
-    # f(Y), implemented by the synthesized method f(Z) where X<Z and Y<Z. The
-    # dispatch has removed f(X), leaving only f(Y), but there is no guarantee
-    # that Y = Z-X, so we need to check for Y.
-    true_code = emitter.Emit(
-        '$(INDENT)if ($COND) {\n'
-        '$!TRUE'
-        '$(INDENT)}\n',
-        COND=test, INDENT=indent)
-    self.GenerateDispatch(
-        true_code, info, indent + '  ', position + 1, positive)
-    return True
+        self.GenerateSingleOperation(emitter, info, indent, operation)
+        fallthrough = False
+    return fallthrough
 
   def AddOperation(self, info):
     self._AddOperation(info)