bindings: Use Visitor to collect union types

Source/bindings/scripts/compute_interfaces_info_individual.py

 #!/usr/bin/python
 #
 # Copyright (C) 2013 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
@@ skipping 29 matching lines @@
 
 Design doc: http://www.chromium.org/developers/design-documents/idl-build
 """
 
 from collections import defaultdict
 import optparse
 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
 
 module_path = os.path.dirname(__file__)
 source_path = os.path.normpath(os.path.join(module_path, os.pardir, os.pardir))
 
 
 class IdlBadFilenameError(Exception):
     """Raised if an IDL filename disagrees with the interface name in the file."""
     pass
@@ skipping 62 matching lines @@
                 (implement.left_interface, implement.right_interface, definition_name))
     return left_interfaces, right_interfaces
 
 
 def get_put_forward_interfaces_from_definition(definition):
     return sorted(set(attribute.idl_type.base_type
                       for attribute in definition.attributes
                       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)
+    def collect(self, definitions):
+        self._union_types = set()
+        definitions.accept(self)
+        return self._union_types
 
-    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 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):
     """A class that collects interface information from idl files."""
     def __init__(self, cache_directory=None):
         self.reader = IdlReader(interfaces_info=None, outputdir=cache_directory)
         self.interfaces_info = {}
         self.partial_interface_files = defaultdict(lambda: {
             'full_paths': [],
             'include_paths': [],
         })
         self.enumerations = set()
         self.union_types = set()
         self.typedefs = {}
 
     def add_paths_to_partials_dict(self, partial_interface_name, full_path,
                                    include_paths):
         paths_dict = self.partial_interface_files[partial_interface_name]
         paths_dict['full_paths'].append(full_path)
         paths_dict['include_paths'].extend(include_paths)
 
     def collect_info(self, idl_filename):
         """Reads an idl file and collects information which is required by the
         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.
         for enum_name in definitions.enumerations.keys():
             for defined_enum in self.enumerations:
                 if defined_enum.name == enum_name:
                     raise Exception('Enumeration %s has multiple definitions' % enum_name)
         self.enumerations.update(definitions.enumerations.values())
 
         if definitions.interfaces:
@@ skipping 101 matching lines @@
 
     write_pickle_file(options.interfaces_info_file,
                       info_collector.get_info_as_dict(),
                       options.write_file_only_if_changed)
     write_pickle_file(options.component_info_file,
                       info_collector.get_component_info_as_dict(),
                       options.write_file_only_if_changed)
 
 if __name__ == '__main__':
     sys.exit(main())