bindings: Use Visitor to collect union types

Source/bindings/scripts/compute_interfaces_info_individual.py

Index: Source/bindings/scripts/compute_interfaces_info_individual.py
diff --git a/Source/bindings/scripts/compute_interfaces_info_individual.py b/Source/bindings/scripts/compute_interfaces_info_individual.py
index 3756879f920a0c5b4df727d04b7453bf20b7cf6a..888f410d9e11876ecd8a38845127db4caff4ce4e 100755
--- a/Source/bindings/scripts/compute_interfaces_info_individual.py
+++ b/Source/bindings/scripts/compute_interfaces_info_individual.py
@@ -47,6 +47,7 @@ import os
 import posixpath
 import sys
 
+from idl_definitions import Visitor
 from idl_reader import IdlReader
 from utilities import get_file_contents, read_file_to_list, idl_filename_to_interface_name, idl_filename_to_component, write_pickle_file, get_interface_extended_attributes_from_idl, is_callback_interface_from_idl
 
@@ -129,34 +130,27 @@ def get_put_forward_interfaces_from_definition(definition):
                       if 'PutForwards' in attribute.extended_attributes))
 
 
-def collect_union_types_from_definitions(definitions):
+class UnionTypeCollector(Visitor):

[Jens Widell, 2015/04/08 09:35:19]

I guess we could have kept a collect_union_types_from_defitions() function, and
just declared the visitor sub-class inside it. Would make the call-site a little
more natural; it now declares an essentially stateless temporary object just to
call a method on it.

[bashi, 2015/04/10 00:05:14]

Done.

     """Traverse definitions and collect all union types."""
 
-    def union_types_from(things):
-        return (thing.idl_type for thing in things
-                if thing.idl_type.is_union_type)
-
-    this_union_types = set()
-    for interface in definitions.interfaces.itervalues():
-        this_union_types.update(union_types_from(interface.attributes))
-        for operation in interface.operations:
-            this_union_types.update(union_types_from(operation.arguments))
-            if operation.idl_type.is_union_type:
-                this_union_types.add(operation.idl_type)
-        for constructor in interface.constructors:
-            this_union_types.update(union_types_from(constructor.arguments))
-        for constructor in interface.custom_constructors:
-            this_union_types.update(union_types_from(constructor.arguments))
-    for callback_function in definitions.callback_functions.itervalues():
-        this_union_types.update(union_types_from(callback_function.arguments))
-        if callback_function.idl_type.is_union_type:
-            this_union_types.add(callback_function.idl_type)
-    for dictionary in definitions.dictionaries.itervalues():
-        this_union_types.update(union_types_from(dictionary.members))
-    for typedef in definitions.typedefs.itervalues():
-        if typedef.idl_type.is_union_type:
-            this_union_types.add(typedef.idl_type)
-    return this_union_types
+    def collect(self, definitions):
+        self._union_types = set()
+        definitions.accept(self)
+        return self._union_types
+
+    def visit_typed_object(self, typed_object):
+        try:
+            idl_type = getattr(typed_object, 'idl_type')

[Jens Widell, 2015/04/08 09:35:19]

It would be better to use typed_object.idl_type_attributes, which is a list of
attributes whose values are types (and thus need to be considered in a context
such as this.)

This matters for IdlIterable, IdlMaplike and IdlSetlike, where the attributes
are named value_type and/or key_type rather than idl_type.

Also, with

  idl_type = getattr(typed_object, name, None)

you could skip the try/except and just check idl_type (as you do already.)

[bashi, 2015/04/10 00:05:14]

Done.

+        except AttributeError:
+            return
+        if not idl_type:
+            return
+        if idl_type.is_union_type:
+            self._union_types.add(idl_type)
+        if idl_type.element_type and idl_type.element_type.is_union_type:

[Jens Widell, 2015/04/08 09:35:19]

I think it would be better to have a process_type() function that handles union
types, and recurses for nested types like sequences and nullable.

This implementation mistreats sequence<union_type?> and sequence<union_type>?, I
think?

[bashi, 2015/04/10 00:05:14]

Good suggestion. I added idl_types() to IdlTypeBase and its sub classes, which
returns a list of IdlType to be processed.

+            self._union_types.add(idl_type.element_type)
+        if idl_type.inner_type and idl_type.inner_type.is_union_type:
+            self._union_types.add(idl_type.inner_type)
 
 
 class InterfaceInfoCollector(object):
@@ -183,7 +177,7 @@ class InterfaceInfoCollector(object):
         binding code generation."""
         definitions = self.reader.read_idl_file(idl_filename)
 
-        this_union_types = collect_union_types_from_definitions(definitions)
+        this_union_types = UnionTypeCollector().collect(definitions)
         self.union_types.update(this_union_types)
         self.typedefs.update(definitions.typedefs)
         # Check enum duplication.