IDL union: nullable support

Source/bindings/scripts/v8_union.py

Index: Source/bindings/scripts/v8_union.py
diff --git a/Source/bindings/scripts/v8_union.py b/Source/bindings/scripts/v8_union.py
index 2c4d735d6edcf6b44cd8428b70a220ff17a7e436..d0fb4a008c2cc59034d3fc298d0b9f20d8f18aa5 100644
--- a/Source/bindings/scripts/v8_union.py
+++ b/Source/bindings/scripts/v8_union.py
@@ -16,12 +16,32 @@ header_forward_decls = set()
 
 
 def union_context(union_types, interfaces_info):
+    # For container classes we strip nullable wrappers. For example,
+    # both (A or B)? and (A? or B) will become AOrB. This should be OK
+    # because container classes can handle null and it seems that
+    # distinguishing (A or B)? and (A? or B) doesn't make sense.
+    container_cpp_types = set()
+    union_types_for_containers = set()
+    nullable_cpp_types = set()
+    for union_type in union_types:
+        cpp_type = union_type.cpp_type
+        if cpp_type not in container_cpp_types:
+            union_types_for_containers.add(union_type.as_union_type)

[haraken, 2014/11/07 18:06:31]

I'm not sure what .as_union_type is for. Can we just use union_type ?

[Jens Widell, 2014/11/07 18:15:46]

Since container_context() uses union_type.name, it makes a difference. But it
only affects (Python) exception messages, so the difference is insignificant.

[bashi, 2014/11/08 07:10:34]

Yes, it only affects exception messages, but just using union_type would be
suitable. Done.

+            container_cpp_types.add(cpp_type)
+        if union_type.includes_nullable_type:

[Jens Widell, 2014/11/07 10:58:19]

Should this also check union_type.is_nullable?

[bashi, 2014/11/08 07:10:34]

IdlNullableType.includes_nullable_type is True so it should already be covered.

+            nullable_cpp_types.add(cpp_type)
+
+    union_types_for_containers = sorted(union_types_for_containers,
+                                        key=lambda union_type: union_type.name)

[Jens Widell, 2014/11/07 10:58:19]

It doesn't really matter (stable order in the generated code is the priority)
but with (A? or B) and (A or C) you'll get the order AOrC, AorB in the generated
code, won't you?

[bashi, 2014/11/08 07:10:34]

Good catch! Then, we should use idl_type.cpp_type as the sort key. Done.

+    nullable_cpp_types = sorted(nullable_cpp_types)
+
     return {
         'containers': [container_context(union_type, interfaces_info)
-                       for union_type in union_types],
+                       for union_type in union_types_for_containers],
         'cpp_includes': sorted(cpp_includes),
         'header_forward_decls': sorted(header_forward_decls),
         'header_includes': sorted(UNION_H_INCLUDES),
+        'nullable_cpp_types': nullable_cpp_types,
     }
 
 
@@ -69,12 +89,20 @@ def container_context(union_type, interfaces_info):
         else:
             raise Exception('%s is not supported as an union member.' % member.name)
 
+    # Nullable restriction checks
+    nullable_members = union_type.number_of_nullable_member_types
+    if nullable_members > 1:
+        raise Exception('%s contains more than one nullable members' % union_type.name)
+    if dictionary_type and nullable_members == 1:
+        raise Exception('%s has a dictionary and a nullable member' % union_type.name)
+
     return {
         'array_buffer_type': array_buffer_type,
         'array_buffer_view_type': array_buffer_view_type,
         'boolean_type': boolean_type,
-        'cpp_class': union_type.name,
+        'cpp_class': union_type.cpp_type,
         'dictionary_type': dictionary_type,
+        'includes_nullable_type': union_type.includes_nullable_type,

[haraken, 2014/11/07 18:06:31]

includes_nullable_type => contains_nullable_type ?

Or simply, is_nullable ?

[bashi, 2014/11/08 07:10:34]

"includes a nullable type" is what the spec is using so I'd prefer not changing
the name.
http://heycam.github.io/webidl/#dfn-includes-a-nullable-type

         'interface_types': interface_types,
         'members': members,
         'needs_trace': any(member['is_traceable'] for member in members),
@@ -89,6 +117,8 @@ def member_context(member, interfaces_info):
     if interface_info:
         cpp_includes.update(interface_info.get('dependencies_include_paths', []))
         header_forward_decls.add(member.implemented_as)
+    if member.is_nullable:
+        member = member.inner_type
     return {
         'cpp_name': v8_utilities.uncapitalize(member.name),
         'cpp_type': member.cpp_type_args(used_in_cpp_sequence=True),