Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Unified Diff: Source/bindings/scripts/code_generator_v8.py

Issue 19607011: Generate binding code for VoidCallback.idl with code generator in python (Closed) Base URL: https://chromium.googlesource.com/chromium/blink@master
Patch Set: fix double -> single quote etc Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/bindings/scripts/code_generator_v8.py
diff --git a/Source/bindings/scripts/code_generator_v8.py b/Source/bindings/scripts/code_generator_v8.py
index d2f933044a5a171b5f1109ef2dc94fa95f660842..9af909040b2c1935f4c4a0625a09cc42f64713e2 100644
--- a/Source/bindings/scripts/code_generator_v8.py
+++ b/Source/bindings/scripts/code_generator_v8.py
@@ -26,16 +26,313 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-"""Generate Blink V8 bindings (.h and .cpp files).
+"""
+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
-
-FIXME: Currently a stub, as part of landing the parser and code generator
-incrementally. Only implements generation of dummy .cpp and .h files.
"""
-import os.path
+import os
+import posixpath
+import sys
+import re
haraken 2013/07/26 02:03:30 Nit: Alphabetical order please.
+
+import idl_definitions
+
+# 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
+
+
+def apply_template(path_to_template, params):
haraken 2013/07/26 02:03:30 params => parameters Probably 'contents' might be
+ 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(params)
+
+
+def get_v8_class_name(interface):
haraken 2013/07/26 02:03:30 get_v8_class_name => v8_class_name Omitting "get"
+ return 'V8' + interface.name
+
+
+def get_impl_name(interface_or_function):
Nils Barth (inactive) 2013/07/25 11:58:20 This could just be: return interface_or_function.e
haraken 2013/07/26 02:03:30 Good point! You're a Python expert :) Other comme
kojih 2013/07/26 07:18:38 I agree. done.
+ implementedAs = interface_or_function.extended_attributes.get('ImplementedAs')
+ if implementedAs is not None:
+ return implementedAs
+ return interface_or_function.name
+
+
+def get_conditional_string(interface_or_attribute_or_operation):
haraken 2013/07/26 02:03:30 get_conditional_string => conditional_string
+ conditional = interface_or_attribute_or_operation.extended_attributes.get('Conditional')
+ if conditional is None:
Nils Barth (inactive) 2013/07/25 11:58:20 if not conditional: return '' ...and don't nee
+ return ''
+ else:
Nils Barth (inactive) 2013/07/25 11:58:20 No need for an else, since you've just returned.
+ operator = ''
+ if '&' in conditional:
Nils Barth (inactive) 2013/07/25 11:58:20 This seems clearer with a for loop (anyway, no nee
haraken 2013/07/26 02:03:30 Looks better!
kojih 2013/07/26 07:18:38 Thanks, done.
+ operator = '&'
+ if '|' in conditional:
+ operator = '|'
+ if operator == '':
+ return 'ENABLE(%s)' % conditional
+ else:
+ # Avoid duplicated conditions.
+ conditions = set(conditional.split(operator))
+ return (' %s%s ' % (operator, operator)).join(['ENABLE(%s)' % expression for expression in sorted(conditions)])
+
+
+def sort_and_remove_duplicate(includes):
+ return sorted(list(set(includes)))
haraken 2013/07/26 02:03:30 How about assuming that includes is a set? There w
kojih 2013/07/26 07:18:38 Good idea! done.
+
+
+class NativeTypeAndJSType:
haraken 2013/07/26 02:03:30 CPPTypeAndV8Type Actually I don't think this clas
+ def __init__(self, native_type=None, js_type=None):
+ self.native_type = native_type
+ self.js_type = js_type
+
+# IDL data_type: [element's C++ data_type, V8 data_type]
haraken 2013/07/26 02:03:30 I'd remove this comment.
+typed_arrays = {
+ 'ArrayBuffer': NativeTypeAndJSType(),
+ 'ArrayBufferView': NativeTypeAndJSType(),
haraken 2013/07/26 02:03:30 As far as I see the Perl code generator, these two
kojih 2013/07/26 07:18:38 ok.
+ 'Uint8Array': NativeTypeAndJSType('unsigned char', 'v8::kExternalUnsignedByteArray'),
+ 'Uint8ClampedArray': NativeTypeAndJSType('unsigned char', 'v8::kExternalPixelArray'),
+ 'Uint16Array': NativeTypeAndJSType('unsigned short', 'v8::kExternalUnsignedShortArray'),
+ 'Uint32Array': NativeTypeAndJSType('unsigned int', 'v8::kExternalUnsignedIntArray'),
+ 'Int8Array': NativeTypeAndJSType('signed char', 'v8::kExternalByteArray'),
+ 'Int16Array': NativeTypeAndJSType('short', 'v8::kExternalShortArray'),
+ 'Int32Array': NativeTypeAndJSType('int', 'v8::kExternalIntArray'),
+ 'Float32Array': NativeTypeAndJSType('float', 'v8::kExternalFloatArray'),
+ 'Float64Array': NativeTypeAndJSType('double', 'v8::kExternalDoubleArray'),
+}
+
+primitive_types = set([
haraken 2013/07/26 02:03:30 It looks like that methods and global variables ar
kojih 2013/07/26 07:18:38 ok, done.
+ 'boolean',
+ 'void',
+ 'Date',
+ 'byte',
+ 'octet',
+ 'short',
+ 'long',
+ 'long long',
+ 'unsigned short',
+ 'unsigned long',
+ 'unsigned long long',
+ 'float',
+ 'double',
+])
+
+
+def get_sequence_type(data_type):
haraken 2013/07/26 02:03:30 Drop this method in this CL.
+ matched = re.match(r'^sequence<([\w\s]+)>.*', data_type)
+ if matched:
+ return matched.group(1)
+ return None
+
+
+def get_array_type(data_type):
haraken 2013/07/26 02:03:30 Ditto.
+ matched = re.match(r'^([\w\s]+)\[\]', data_type)
+ if matched:
+ return matched.group(1)
+ return None
+
+
+def is_typed_array_type(data_type):
haraken 2013/07/26 02:03:30 Ditto.
+ return data_type in typed_arrays
+
+
+def is_primitive_type(data_type):
+ return data_type in primitive_types
+
+
+def is_union_type(data_type):
haraken 2013/07/26 02:03:30 Ditto.
+ return isinstance(data_type, idl_definitions.IdlUnionType)
+
+
+def get_native_type(data_type):
haraken 2013/07/26 02:03:30 get_native_type => native_type
+ """
+ Return native data_type corresponds to IDL data_type.
Nils Barth (inactive) 2013/07/25 11:58:20 Syntax: no initial linebreak, but linebreak after
haraken 2013/07/26 02:03:30 native => cpp
+ @param[in] data_type ... IDL data_type
+ @param[in] called_by_webcore
+ @param[in] used_as_parameter
+ """
+ if data_type == 'boolean':
+ return 'bool'
+ raise Exception('Not supported')
+
+
+def get_callback_parameter_declaration(operation):
haraken 2013/07/26 02:03:30 get_callback_parameter_declaration => callback_arg
kojih 2013/07/26 07:18:38 done.
+ parameters = ['%s %s' % (get_native_type(parameter.data_type), parameter.name) for parameter in operation.arguments]
+ return ', '.join(parameters)
+
+
+class CodeGeneratorV8:
+ def __init__(self, definitions, interface_name, output_directory, idl_directories, verbose=False, generate_h=True, generate_cpp=True):
haraken 2013/07/26 02:03:30 Do we need generate_h and generate_cpp ? In case
Nils Barth (inactive) 2013/07/26 03:02:40 I was wondering the same thing. If we really want
kojih 2013/07/26 07:18:38 oh this was for rewriting work. (for example, gene
+ self.idl_definitions = definitions
+ self.interface_name = interface_name
+ self.idl_directories = idl_directories
+ self.output_directory = output_directory
+ self.generate_h = generate_h
+ self.generate_cpp = generate_cpp
+ self.verbose = verbose
+ self.header = ''
+ self.implementation = ''
haraken 2013/07/26 02:03:30 implementation => cpp
+ self.cached_interfaces = {}
haraken 2013/07/26 02:03:30 Drop this in this CL.
+ self.common_template_parameters = {}
+ self.interface = None
+ self.enum_types = dict([[enum.name, enum.values] for enum in self.idl_definitions.enumerations.values()])
Nils Barth (inactive) 2013/07/25 11:58:20 This would probably be easier at the parser side.
haraken 2013/07/26 02:03:30 I'd drop this in this CL.
Nils Barth (inactive) 2013/07/26 03:02:40 Good point. Koji, no enums in these IDLs, right?
kojih 2013/07/26 07:18:38 exactly. removed.
+ self.callback_function_types = self.idl_definitions.callback_functions
haraken 2013/07/26 02:03:30 Shall we rename "function" to "method"? I think we
Nils Barth (inactive) 2013/07/26 03:02:40 This is an important general point: the parser str
kojih 2013/07/26 07:18:38 Yeah now this is unnecessary. This was primarily s
+
+ def is_enum_type(self, data_type):
haraken 2013/07/26 02:03:30 Drop this in this CL.
+ return data_type in self.enum_types
+
+ def is_callback_function_type(self, data_type):
+ return data_type in self.callback_function_types
+
+ def get_includes_for_type(self, data_type):
+ if self.skip_include_header(data_type):
+ return []
+ includes = []
haraken 2013/07/26 02:03:30 Don't you want to use a set for includes?
+ if data_type == 'EventListener':
+ includes.append('core/dom/EventListener.h')
+ elif data_type == 'SerializedScriptValue':
+ includes.append('bindings/v8/SerializedScriptValue.h')
+ elif data_type == 'any' or self.is_callback_function_type(data_type):
+ includes.append('bindings/v8/ScriptValue.h')
haraken 2013/07/26 02:03:30 Are these needed in this CL? Header includes in th
+ else:
+ includes.append('V8%s.h' % data_type)
+ return includes
+
+ def get_includes_for_parameter(self, parameter):
haraken 2013/07/26 02:03:30 parameter => argument The same comment to other p
+ includes = []
haraken 2013/07/26 02:03:30 Ditto. Can you use a set?
+ array_or_sequence_type = get_array_type(parameter.data_type) or get_sequence_type(parameter.data_type)
+ if array_or_sequence_type:
+ if self.is_ref_ptr_type(array_or_sequence_type):
haraken 2013/07/26 02:03:30 I don't understand what this branch is for. Maybe
+ includes += self.get_includes_for_type(array_or_sequence_type)
+ else:
+ includes += self.get_includes_for_type(parameter.data_type)
+ return includes
+
+ def is_ref_ptr_type(self, data_type):
+ return not(is_union_type(data_type) or
+ data_type == 'any' or data_type == 'DOMString' or
Nils Barth (inactive) 2013/07/25 11:58:20 A bit clearer to consistently have each on the sam
kojih 2013/07/26 07:18:38 I'll do that in the following patches. (this part
+ is_primitive_type(data_type) or
+ get_array_type(data_type) or
+ get_sequence_type(data_type) or
+ self.is_callback_function_type(data_type) or
+ self.is_enum_type(data_type))
+
+ def skip_include_header(self, data_type):
haraken 2013/07/26 02:03:30 skip_include_header => is_skip_include
+ return is_primitive_type(data_type) or \
+ self.is_enum_type(data_type) or \
Nils Barth (inactive) 2013/07/25 11:58:20 Style: use parentheses for implicit line continuat
kojih 2013/07/26 07:18:38 I'll do that in the following patches. (this part
+ self.is_callback_function_type(data_type) or \
+ data_type == 'DOMString'
+
+ def header_files_for_interface(self, interface_name, impl_class_name):
haraken 2013/07/26 02:03:30 get_includes_for_interface (for naming consistency
kojih 2013/07/26 07:18:38 Good point......... HeaderFilesForInterface actua
+ includes = []
haraken 2013/07/26 02:03:30 Can you use a set?
kojih 2013/07/26 07:18:38 done.
+ if is_typed_array_type(interface_name):
+ includes.append('wtf/%s.h' % interface_name)
+ elif not self.skip_include_header(interface_name):
+ # FIXME: parser will prepare posix form relative path from Source/bindings in IdlInterface.file_name
haraken 2013/07/26 02:03:30 Yeah, we should do that. nbarth: could you do tha
Nils Barth (inactive) 2013/07/26 03:02:40 Will do! It's a bit tricky to do properly, since w
+ idl_filename = self.idl_definitions.file_name
+ idl_rel_path_local = os.path.relpath(idl_filename)
+ idl_rel_path_posix = idl_rel_path_local.replace(os.path.sep, posixpath.sep)
+
+ idl_dir_posix = posixpath.join('bindings', posixpath.dirname(idl_rel_path_posix))
+ includes.append(posixpath.join(idl_dir_posix, impl_class_name + '.h'))
+ return includes
+
+ def generate_interface(self, interface):
+ self.interface = interface
+ self.common_template_parameters = {
haraken 2013/07/26 02:03:30 Nit: common_template_parameters => common_template
kojih 2013/07/26 07:18:38 done.
kojih 2013/07/26 07:18:38 done. replaced all template_parameters with templa
+ 'conditional_string': get_conditional_string(self.interface),
+ }
+ if interface.is_callback:
+ template_parameters = self.common_template_parameters.copy()
+ template_parameters.update(self.generate_callback_template_parameters())
+ if self.generate_h:
+ self.header = apply_template('templates/callback.h', template_parameters)
+ if self.generate_cpp:
+ self.implementation = apply_template('templates/callback.cpp', template_parameters)
+ else:
+ if self.generate_h:
+ self.generate_header()
+ if self.generate_cpp:
+ self.generate_implementation()
+
+ def write_interface(self):
+ if self.interface_name in self.idl_definitions.interfaces:
+ interface = self.idl_definitions.interfaces[self.interface_name]
+ self.generate_interface(interface)
+ if self.generate_h:
+ header_filename = os.path.join(self.output_directory, get_v8_class_name(self.interface) + '.h')
+ with open(header_filename, 'w') as f:
haraken 2013/07/26 02:03:30 Nit: f => file
Nils Barth (inactive) 2013/07/26 03:02:40 Even better, header_file For clarity, I've been co
kojih 2013/07/26 07:18:38 done. we can't use type, file as variable name in
+ f.write(self.header)
+ if self.generate_cpp:
+ implementation_filename = os.path.join(self.output_directory, get_v8_class_name(self.interface) + '.cpp')
haraken 2013/07/26 02:03:30 implementation_filename => cpp_filename
kojih 2013/07/26 07:18:38 done.
+ with open(implementation_filename, 'w') as f:
haraken 2013/07/26 02:03:30 Nit: f => file
Nils Barth (inactive) 2013/07/26 03:02:40 => cpp_file
+ f.write(self.implementation)
+ else:
+ raise Exception('%s not in IDL definitions' % self.interface_name)
haraken 2013/07/26 02:03:30 I'd prefer: if not self.interface_name in self.
Nils Barth (inactive) 2013/07/26 03:02:40 BTW: if key not in dict: ... ...is a bit clear
kojih 2013/07/26 07:18:38 done.
+
+ def generate_callback_template_parameters(self):
haraken 2013/07/26 02:03:30 generate_callback_template_parameters => generate_
+ impl_class_name = get_impl_name(self.interface)
+ v8_class_name = get_v8_class_name(self.interface)
+ functions = []
+ implementation_includes = [
haraken 2013/07/26 02:03:30 Can you use a set for the includes?
haraken 2013/07/26 02:03:30 implementation_includes => cpp_includes
+ 'core/dom/ScriptExecutionContext.h',
+ 'bindings/v8/V8Binding.h',
+ 'bindings/v8/V8Callback.h',
+ 'wtf/Assertions.h',
+ ]
+ header_includes = [
haraken 2013/07/26 02:03:30 Ditto.
+ 'bindings/v8/ActiveDOMCallback.h',
+ 'bindings/v8/DOMWrapperWorld.h',
+ 'bindings/v8/ScopedPersistent.h',
+ ]
+ header_includes += self.header_files_for_interface(self.interface.name, impl_class_name)
+ for operation in self.interface.operations:
+ custom = 'Custom' in operation.extended_attributes
+ function = {}
+ if not custom:
haraken 2013/07/26 02:03:30 if not 'Custom' in operation.extended_attributes:
Nils Barth (inactive) 2013/07/26 03:02:40 Even better: if 'Custom' not in operation.extended
kojih 2013/07/26 07:18:38 done.
+ implementation_includes += self.get_includes_for_type(operation.data_type)
+ for parameter in operation.arguments:
+ implementation_includes += self.get_includes_for_parameter(parameter)
+ if operation.data_type != 'boolean':
+ raise Exception('We don''t yet support callbacks that return non-boolean values.')
Nils Barth (inactive) 2013/07/25 11:58:20 BTW, it's fine to use "" for quotes if there are s
kojih 2013/07/26 07:18:38 done.
+ parameters = []
+ if len(operation.arguments) > 0:
Nils Barth (inactive) 2013/07/25 11:58:20 No need for > 0.
kojih 2013/07/26 07:18:38 done.
+ raise Exception('Not supported')
+ function = {
+ 'return_type': get_native_type(operation.data_type),
+ 'name': operation.name,
+ 'parameter_declaration': get_callback_parameter_declaration(operation),
+ 'prepare_js_parameters': '',
haraken 2013/07/26 02:03:30 Drop this in this CL.
+ 'handles': ',\n'.join(['%sHandle' % parameter.name for parameter in operation.arguments]),
haraken 2013/07/26 02:03:30 You don't need to add "\n".
kojih 2013/07/26 07:18:38 ok, but dropped in this CL.
+ 'parameters': parameters,
haraken 2013/07/26 02:03:30 Nit: parameters => arguments
kojih 2013/07/26 07:18:38 ok, but dropped in this CL.
+ 'custom': custom,
+ }
+ functions.append(function)
+ implementation_includes = sort_and_remove_duplicate(implementation_includes)
+ header_includes = sort_and_remove_duplicate(header_includes)
+ template_parameters = {
haraken 2013/07/26 02:03:30 template_parameters => template_contents
+ 'interface_name': self.interface.name,
+ 'impl_class_name': impl_class_name,
+ 'v8_class_name': v8_class_name,
+ 'implementation_includes': implementation_includes,
+ 'header_includes': header_includes,
haraken 2013/07/26 02:03:30 If header_includes is a set, you can just say: sor
+ 'functions': functions,
haraken 2013/07/26 02:03:30 functions => methods As christophe commented in t
+ }
+ return template_parameters
+
+ def generate_header(self):
+ # TODO: implement
haraken 2013/07/26 02:03:30 TODO => FIXME
+ self.header = ''
+
+ def generate_implementation(self):
haraken 2013/07/26 02:03:30 generate_implementation => generate_cpp
+ # TODO: implement
haraken 2013/07/26 02:03:30 TODO => FIXME
+ self.implementation = ''
def generate_dummy_header_and_cpp(target_interface_name, output_directory):

Powered by Google App Engine
This is Rietveld 408576698