Add constants and primitive type readonly attributes to Python IDL compiler

Source/bindings/scripts/code_generator_v8.py

 # 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.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 17 matching lines @@
 
 """Generate Blink V8 bindings (.h and .cpp files).
 
 Input: An object of class IdlDefinitions, containing an IDL interface X
 Output: V8X.h and V8X.cpp
 """
 
 import os
 import posixpath
 import re
+import struct
 import sys
 
 # jinja2 is in chromium's third_party directory.
 module_path, module_name = os.path.split(__file__)
 third_party = os.path.join(module_path, os.pardir, os.pardir, os.pardir, os.pardir)
 sys.path.append(third_party)
 import jinja2
 
 
+ACRONYMS = ['CSS', 'HTML', 'IME', 'JS', 'SVG', 'URL', 'XML', 'XSLT']
 CALLBACK_INTERFACE_CPP_INCLUDES = set([
     'core/dom/ScriptExecutionContext.h',
     'bindings/v8/V8Binding.h',
     'bindings/v8/V8Callback.h',
     'wtf/Assertions.h',
 ])
-
-
 CALLBACK_INTERFACE_H_INCLUDES = set([
     'bindings/v8/ActiveDOMCallback.h',
     'bindings/v8/DOMWrapperWorld.h',
     'bindings/v8/ScopedPersistent.h',
 ])
-
-
 INTERFACE_CPP_INCLUDES = set([
     'RuntimeEnabledFeatures.h',
     'bindings/v8/ScriptController.h',
     'bindings/v8/V8Binding.h',
+    'bindings/v8/V8DOMWrapper.h',
+    'bindings/v8/V8DOMConfiguration.h',
     'core/dom/ContextFeatures.h',
     'core/dom/Document.h',
     'core/page/Frame.h',
     'core/platform/chromium/TraceEvent.h',
     'wtf/UnusedParam.h',
 ])
-
-
 INTERFACE_H_INCLUDES = set([
     'bindings/v8/V8Binding.h',
+    'bindings/v8/V8DOMWrapper.h',
+    'bindings/v8/WrapperTypeInfo.h',
 ])
-
-
 CPP_TYPE_SPECIAL_CONVERSION_RULES = {
     'float': 'float',
     'double': 'double',
     'long long': 'long long',
     'unsigned long long': 'unsigned long long',
     'long': 'int',
     'short': 'int',
     'byte': 'int',
     'boolean': 'bool',
     'DOMString': 'const String&',
 }
-
-
+CPP_UNSIGNED_TYPES = set([
+    'octet',
+    'unsigned int',
+    'unsigned long',
+    'unsigned short',
+])
 PRIMITIVE_TYPES = set([
     'boolean',
     'void',
     'Date',
     'byte',
     'octet',
     'short',
     'long',
     'long long',
     'unsigned short',
     'unsigned long',
     'unsigned long long',
     'float',
     'double',
 ])
+CPP_VALUE_TO_JS_VALUE_RETURN_DICT = {

[haraken, 2013/08/22 07:31:33]

JS_VALUE => V8_VALUE

We're mixing "js" and "v8".

[Nils Barth (inactive), 2013/08/22 09:08:13]

Got it; changed throughout.

+    'unsigned': 'v8SetReturnValueUnsigned({callback_info}, {cpp_value});',
+}
 
 
 def apply_template(path_to_template, contents):
     dirname, basename = os.path.split(path_to_template)
     jinja_env = jinja2.Environment(trim_blocks=True, loader=jinja2.FileSystemLoader([dirname]))
     template = jinja_env.get_template(basename)
     return template.render(contents)
 
 
 def cpp_value_to_js_value(data_type, cpp_value, isolate, creation_context=''):
     """Returns a expression that represent JS value corresponding to a C++ value."""
     if data_type == 'boolean':
         return 'v8Boolean(%s, %s)' % (cpp_value, isolate)
     if data_type in ['long long', 'unsigned long long', 'DOMTimeStamp']:
         # long long and unsigned long long are not representable in ECMAScript.
         return 'v8::Number::New(static_cast<double>(%s))' % cpp_value
     if primitive_type(data_type):
         if data_type not in ['float', 'double']:
             raise Exception('unexpected data_type %s' % data_type)
         return 'v8::Number::New(%s)' % cpp_value
     if data_type == 'DOMString':
         return 'v8String(%s, %s)' % (cpp_value, isolate)
     if array_or_sequence_type(data_type):
         return 'v8Array(%s, %s)' % (cpp_value, isolate)
     return 'toV8(%s, %s, %s)' % (cpp_value, creation_context, isolate)
 
 
+def cpp_value_to_js_value_return(data_type, cpp_value, callback_info=''):

[haraken, 2013/08/22 07:31:33]

Why don't you simply call this cpp_value_v8_value ? Does 'return' matter?

[Nils Barth (inactive), 2013/08/22 09:08:13]

cpp_value_v8_value and cpp_value_v8_value_return are quite different; one
returns a conversion *expression* (which can be used many places), while the
other returns a setReturn... *statement* (which is used before a return;
statement).
I've changed the name to cpp_value_v8_value_set_return and distinguished the
comments.

You can see what the functions look like at the WIP:
https://codereview.chromium.org/17572008/diff/181001/Source/bindings/scripts/...

The expressions are of the form
'v8String({cpp_value}, {isolate})'
...while the set return statements are of the from
'v8SetReturnValueUnsigned({callback_info}, {cpp_value});'

[haraken, 2013/08/22 11:05:28]

Makes sense. Then set_v8_return_value ?

[Nils Barth (inactive), 2013/08/23 01:50:05]

Good point; actually, v8_set_return_value seems even clearer, no?

+    """Return an expression converting a C++ value to a JS value, as a return value."""
+    this_cpp_type = cpp_type(data_type)
+    if this_cpp_type in CPP_VALUE_TO_JS_VALUE_RETURN_DICT:
+        expression_format_string = CPP_VALUE_TO_JS_VALUE_RETURN_DICT[this_cpp_type]
+    else:
+        raise Exception('unexpected data_type %s' % data_type)
+    return expression_format_string.format(callback_info=callback_info, cpp_value=cpp_value)
+
+
 def generate_conditional_string(interface_or_attribute_or_operation):
     if 'Conditional' not in interface_or_attribute_or_operation.extended_attributes:
         return ''
     conditional = interface_or_attribute_or_operation.extended_attributes['Conditional']
     for operator in ['&', '|']:
         if operator in conditional:
             conditions = set(conditional.split(operator))
             operator_separator = ' %s%s ' % (operator, operator)
             return operator_separator.join(['ENABLE(%s)' % expression for expression in sorted(conditions)])
     return 'ENABLE(%s)' % conditional
 
 
+def generate_constants(interface):
+    return [generate_constant(constant) for constant in interface.constants]
+
+
+def generate_constant(constant):
+    # Extended Attributes: Conditional, DeprecateAs, EnabledAtRuntime, Reflect
+    # FIXME: Conditional and DeprecateAs are only used in tests, so remove
+    name = constant.extended_attributes.get('Reflect', constant.name)

[haraken, 2013/08/22 07:31:33]

name => reflected_name

[Nils Barth (inactive), 2013/08/22 09:08:13]

Done.

+    value_raw = constant.value
+    # If the value we're dealing with is a hex literal, statically cast it to a
+    # signed integer here, rather than dynamically casting via
+    # static_cast<signed int> in the generated code.
+    # FIXME: why are we casting to signed?
+    # NodeFilter has unsigned 0xFFFFFFFF, which is converted to -1.
+    # FIXME: is this necessary? Hex literals are valid C.
+    #        (only semantic difference is casting to signed)
+    # FIXME: what about octal? decimal?
+    # FIXME: what if we have a negative literal?
+    # FIXME: BUG: Perl only checks '0x', not '0X',
+    #             but we have a test case using 0X.
+    if value_raw.startswith('0x'):
+        value = struct.unpack('i', struct.pack('I', int(value_raw, 16)))[0]
+    else:
+        value = value_raw

[haraken, 2013/08/22 07:31:33]

I don't fully understand why this is needed. Can we get rid of the complexity by
replacing "0X" with "0x" in the IDL files? Or if it's really needed, you can do
the conversion in the parser. (We don't want to mess up the code generator for
edge cases no one cares about.)

[Nils Barth (inactive), 2013/08/22 09:08:13]

I have no idea why this is needed either, hence all the FIXMEs.
However, it's what the Perl currently generates.
So it's quicker to land this (this doesn't change anything) and then see if we
can fix it.
I *think* we can just skip this; the only actual effect is listing 0xFFFFFFFF as
-1 instead of 4294967295.

In a followup, I'll see if we remove this from the Perl as well.
If it causes no problems, it cleans up the code quickly.
If it causes problems, we can fix it later.

[haraken, 2013/08/22 11:05:28]

OK, then let's fix it in a follow-up CL.

[Nils Barth (inactive), 2013/08/23 01:50:05]

Got it!

+
+    constant_parameter = {
+        'name': constant.name,
+        'name_reflect': name,  # FIXME: use name_reflect as correct 'name'

[haraken, 2013/08/22 07:31:33]

name_reflect => reflected_name

[Nils Barth (inactive), 2013/08/22 09:08:13]

Done.

+        'value': value,
+        'value_raw': value_raw,
+        # FIXME: remove conditional: only used in tests
+        'conditional_string': generate_conditional_string(constant),
+        'enabled_at_runtime': 'EnabledAtRuntime' in constant.extended_attributes,
+        'enable_function': runtime_enable_function_name(constant),
+    }
+    return constant_parameter
+
+
+def getter_expression(interface, attribute):

[haraken, 2013/08/22 07:31:33]

getter_expression => attribute_getter_name ?

You can concatenate 'imp->' in the caller side.

[Nils Barth (inactive), 2013/08/22 09:08:13]

attribute_getter is redundant; good point about name vs. expression, but we do
want a function returning the expression (see below).
The main function that generates these includes the 'imp->':
get_attribute_function_call_expression in v8_attributes.py:
https://codereview.chromium.org/17572008/diff/181001/Source/bindings/scripts/...

We can probably refactor getter_expression into getter_name + getter_expression
(adding the imp-> in the expression function), but for now I'm keeping it *very*
simple.

[haraken, 2013/08/22 11:05:28]

If this function is going to be used by other getters (indexed getters, named
getters etc), getter_expression sounds nice. Otherwise, attribute_getter_name
would sound nicer.

Given that it is the caller side that declares "imp", I'd put "imp" in the
caller side. Specifically, the caller side can say
"imp->"+attribute_getter_name(interface, attribute).

[Nils Barth (inactive), 2013/08/23 01:50:05]

Looking at this more, at this point there's no need for a separate function,
since it's barely doing anything.
Once we actually have complex logic, we can see how it's sensible to factor.

On 2013/08/22 11:05:28, haraken wrote:
It's just used for regular getters.
Got it, that's good to know!
In that case the function just becomes "uncapitalize", so it's unnecessary
abstraction.

For context, the function is GetterExpression in Perl,
and get_attribute_function_call_expression in the WIP:
https://codereview.chromium.org/17572008/diff/181001/Source/bindings/scripts/...
It's a pretty long function, and there's adding/stripping of imp->,
so when we actually start landing this, we'll probably want to refactor it so
it's sensible.

+    # FIXME: very incomplete
+    return 'imp->%s()' % uncapitalize(attribute.name)
+
+
+def runtime_enable_function_name(definition_or_member):
+    """Return the name of the RuntimeEnabledFeatures function.
+
+    The returned function checks if a method/attribute is enabled.
+    If a parameter is given (e.g. 'EnabledAtRuntime=FeatureName'), return:
+        RuntimeEnabledFeatures::{featureName}Enabled
+    Otherwise return:
+        RuntimeEnabledFeatures::{methodName}Enabled
+    """
+    name = definition_or_member.extended_attributes.get('EnabledAtRuntime') or definition_or_member.name
+    return 'RuntimeEnabledFeatures::%sEnabled' % uncapitalize(name)
+
+
+def uncapitalize(name):
+    """Uncapitalize first letter or initial acronym (* with some exceptions).
+
+    E.g., 'SetURL' becomes 'setURL', but 'URLFoo' becomes 'urlFoo'.
+    Used in method names; exceptions differ from capitalize().
+    """
+    for acronym in ACRONYMS:
+        if name.startswith(acronym):
+            name.replace(acronym, acronym.lower())
+            return name
+    return name[0].lower() + name[1:]
+
+
 def includes_for_type(data_type):
     if primitive_type(data_type) or data_type == 'DOMString':
         return set()
     if array_or_sequence_type(data_type):
         return includes_for_type(array_or_sequence_type(data_type))
     return set(['V8%s.h' % data_type])
 
 
 def includes_for_cpp_class(class_name, relative_dir_posix):
     return set([posixpath.join('bindings', relative_dir_posix, class_name + '.h')])
@@ skipping 20 matching lines @@
 def array_type(data_type):
     matched = re.match(r'([\w\d_\s]+)\[\]', data_type)
     if not matched:
         return None
     return matched.group(1)
 
 
 def array_or_sequence_type(data_type):
     return array_type(data_type) or sequence_type(data_type)
 
-def cpp_type(data_type, pointer_type):
+
+def cpp_type(data_type, pointer_type=None):
     """Returns the C++ type corresponding to the IDL type.
 
     Args:
         pointer_type:

[kouhei (in TOK), 2013/08/22 06:14:32]

add doc for None?
None: return raw value form (e.g. Foo)

[Nils Barth (inactive), 2013/08/22 06:33:48]

Good point!
Fixed (in more detail).
I made this optional b/c it's unnecessary if this isn't a pointer type (we might
be able to split off a separate function for that), but in fact it's simpler to
just default to raw unless a type is specified.

I've also converted the strings to format strings, so they look just like
templates and are more readable, and renamed the parameter to idl_type, as
that's what it is.

             'raw': return raw pointer form (e.g. Foo*)
             'RefPtr': return RefPtr form (e.g. RefPtr<Foo>)
             'PassRefPtr': return PassRefPtr form (e.g. RefPtr<Foo>)
     """
     if data_type in CPP_TYPE_SPECIAL_CONVERSION_RULES:
         return CPP_TYPE_SPECIAL_CONVERSION_RULES[data_type]
+    if data_type in CPP_UNSIGNED_TYPES:
+        return 'unsigned'
     if array_or_sequence_type(data_type):
         return 'const Vector<%s >&' % cpp_type(array_or_sequence_type(data_type), 'RefPtr')
     if pointer_type == 'raw':
         return data_type + '*'
     if pointer_type in ['RefPtr', 'PassRefPtr']:
         return '%s<%s>' % (pointer_type, data_type)
     raise Exception('Unrecognized pointer type: "%s"' % pointer_type)
 
 
 def v8_type(data_type):
@@ skipping 74 matching lines @@
         header_filename = os.path.join(self.output_directory, header_basename)
         with open(header_filename, 'w') as header_file:
             header_file.write(header_file_text)
 
     def write_cpp_code(self, cpp_basename, cpp_file_text):
         cpp_filename = os.path.join(self.output_directory, cpp_basename)
         with open(cpp_filename, 'w') as cpp_file:
             cpp_file.write(cpp_file_text)
 
     def generate_attribute(self, attribute):
-        self.cpp_includes |= includes_for_type(attribute.data_type)
+        data_type = attribute.data_type
+        # FIXME: need to check should_keep_attribute_alive, but for now
+        # sufficient to check if primitive.
+        should_keep_attribute_alive = not primitive_type(data_type)
+        # FIXME: eliminate should_keep_attribute_alive

[haraken, 2013/08/22 07:31:33]

Remove the FIXME. We will need should_keep_attribute_alive.

[Nils Barth (inactive), 2013/08/22 09:08:13]

Got it!
(There was a FIXME asking to remove it, but I guess we need it.)

+        if should_keep_attribute_alive:
+            return_js_value_statement = None  # unused
+            self.cpp_includes |= includes_for_type(data_type)
+            self.cpp_includes.add('bindings/v8/V8HiddenPropertyName.h')
+        else:
+            cpp_value = getter_expression(self.interface, attribute)
+            return_js_value_statement = cpp_value_to_js_value_return(data_type, cpp_value, callback_info='info')
         return {
             'name': attribute.name,
             'conditional_string': generate_conditional_string(attribute),
             'cpp_method_name': cpp_method_name(attribute),
-            'cpp_type': cpp_type(attribute.data_type, pointer_type='RefPtr'),
-            'v8_type': v8_type(attribute.data_type),
+            'cpp_type': cpp_type(data_type, pointer_type='RefPtr'),
+            'should_keep_attribute_alive': should_keep_attribute_alive,
+            'return_js_value_statement': return_js_value_statement,
+            'v8_type': v8_type(data_type),
         }
 
     def generate_interface(self):
         self.header_includes = INTERFACE_H_INCLUDES
         self.header_includes |= includes_for_cpp_class(cpp_class_name(self.interface), self.relative_dir_posix)
         self.cpp_includes = INTERFACE_CPP_INCLUDES
 
         template_contents = {
             'interface_name': self.interface.name,
             'cpp_class_name': cpp_class_name(self.interface),
             'v8_class_name': v8_class_name(self.interface),
             'attributes': [self.generate_attribute(attribute) for attribute in self.interface.attributes],
             # Size 0 constant array is not allowed in VC++
-            'number_of_attributes': 'WTF_ARRAY_LENGTH(%sAttributes)' % v8_class_name(self.interface) if self.interface.attributes else '0',
-            'attribute_templates': v8_class_name(self.interface) + 'Attributes' if self.interface.attributes else '0',
+            'number_of_attributes': 'WTF_ARRAY_LENGTH(%sAttrs)' % v8_class_name(self.interface) if self.interface.attributes else '0',
+            'attribute_templates': v8_class_name(self.interface) + 'Attrs' if self.interface.attributes else '0',
         }
+        template_contents['constants'] = generate_constants(self.interface)
         # Add includes afterwards, as they are modified by generate_attribute etc.
         template_contents['header_includes'] = sorted(list(self.header_includes))
         template_contents['cpp_includes'] = sorted(list(self.cpp_includes))
         return template_contents
 
     def generate_callback_interface(self):
         self.header_includes = CALLBACK_INTERFACE_H_INCLUDES
         self.header_includes |= includes_for_cpp_class(cpp_class_name(self.interface), self.relative_dir_posix)
         self.cpp_includes = CALLBACK_INTERFACE_CPP_INCLUDES
 
@@ skipping 28 matching lines @@
 
         methods = [generate_method(operation) for operation in self.interface.operations]
         template_contents = {
             'cpp_class_name': self.interface.name,
             'v8_class_name': v8_class_name(self.interface),
             'cpp_includes': sorted(list(self.cpp_includes)),
             'header_includes': sorted(list(self.header_includes)),
             'methods': methods,
         }
         return template_contents