Chromium Code Reviews| 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): |