Start GenerateDispatch refactoring.

lib/dom/scripts/generator.py

Index: lib/dom/scripts/generator.py
diff --git a/lib/dom/scripts/generator.py b/lib/dom/scripts/generator.py
index 8d72a6c024459201352f46a8d717b078d2417b26..25cfd43f28129ec0d6039b99b58dafb54af963dc 100644
--- a/lib/dom/scripts/generator.py
+++ b/lib/dom/scripts/generator.py
@@ -721,3 +721,121 @@ def GetIDLTypeInfo(idl_type_name):
   if match:
     return SequenceIDLTypeInfo(idl_type_name, GetIDLTypeInfo(match.group(1)))
   return _idl_type_registry.get(idl_type_name, IDLTypeInfo(idl_type_name))
+
+def GenerateDispatch(generator, emitter, info, indent, position, overloads):

[podivilov, 2012/05/05 16:44:12]

I don't think it's a good idea to move this method here.

[sra1, 2012/05/05 18:02:06]

Pavel, could you say why?

My concern is that native-specific tests are in this general code.
Perhaps we should have a DispatchGenerator class with overridable methods for
ShouldGenerateSingleOperation and for passing the current optional argument (to
fix the openCursor issue)

Anton: perhaps you could say what you plan to do next?

[podivilov, 2012/05/10 10:51:43]

We currently use this method in two systems - native and htmldartium. Since we
are deprecating dart:dom really soon, there will be just a single system that
uses it. I think moving this code in generator.py is wrong, because it will be
used by the native system only.

[Anton Muhin, 2012/05/10 11:30:13]

I am not sure we should be that picky about how many systems use this
functionality or if it's only native/wrapping system.  After all this
functionality looks generic enough for any other system so ideally I'd like to
move it out of systems altogether, esp. given that it depends on a single
operation provided by a system: GenerateSingleOperation.

And we can always reconsider this solution later, so I'd ask not to block this
CL on this regard.

On 2012/05/10 10:51:43, podivilov wrote:

[Anton Muhin, 2012/05/10 11:30:13]

Stephen, this CL just gets rid of code duplication we have.

Next move would be to simplify interface to GenerateDispatch---we have this code
which generates overloads, position etc. all over the call sites.

Ultimate goal is to rewrite dispatcher to match one for v8 bindings at least for
native system.

The problem I am addessing is as openCursore which in v8 case translates to:

if ((arg0 === null || arg0 is IDBKeyRange) and (arg2 == null || arg2 is int)) {
  // Invoke IDBRange overload
} else (<fake check as IDBKey can be anything you wish>) {
  // Invoke IDBKet variant.
}
throw 'Dispatch failed.'
}  

On 2012/05/05 18:02:06, sra1 wrote:

[podivilov, 2012/05/10 15:57:13]

Why is it needed? We are going to get rid of duplication anyway by merging the
systems together.

[Anton Muhin, 2012/05/10 16:11:55]

What do you suggest?  Wait until we get rid of it?  Or change only wrapping
part?

On 2012/05/10 15:57:13, podivilov wrote:

[podivilov, 2012/05/10 16:20:36]

Personally, I prefer that you wait until migration and cleanup are over to avoid
conflicts. But I'm also ok with changing wrapping part now and merging later.

[Anton Muhin, 2012/05/10 16:24:08]

What's ETA?  If it's a day or two, I'd better wait.  Otherwise I'd prefer to
proceed.

On 2012/05/10 16:20:36, podivilov wrote:

+  """Generates a dispatch to one of the overloads.
+
+  Arguments:
+    emitter: an Emitter for the body of a block of code.
+    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.
+
+  Returns True if the dispatch can fall through on failure, False if the code
+  always dispatches.
+  """
+
+  def NullCheck(name):
+    return '%s === null' % name
+
+  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
+
+    # 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')
+        return True
+
+    return False
+
+  if ShouldGenerateSingleOperation():
+    generator.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)
+    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 = GenerateDispatch(generator,
+        true_code, info, indent + '  ', position + 1, positive)
+    fallthrough2 = GenerateDispatch(generator,
+        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 GenerateDispatch(generator,
+        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)
+  GenerateDispatch(generator,
+      true_code, info, indent + '  ', position + 1, positive)
+  return True