Commit 671e0343 authored by mpcomplete@google.com's avatar mpcomplete@google.com

Fix bug with using enums as default values in mojom. We were previously

generating invalid JavaScript bindings. We now translate an enum value like
FOO_VALUE in a mojom file to proper_namespace.EnumName.FOO_VALUE in JS.

I also fixed enum declarations in JS so an enum value can be defined in terms
of itself, e.g.
  enum Foo { FOO_1, FOO_2 = FOO_1 * 2 };

BUG=320082
R=darin@chromium.org

Review URL: https://codereview.chromium.org/164873002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251417 0039d316-1c4b-4281-b951-d872f2087c98
parent 494492f4
...@@ -8,7 +8,8 @@ define([ ...@@ -8,7 +8,8 @@ define([
"gin/test/expect", "gin/test/expect",
"mojom/sample_service", "mojom/sample_service",
"mojom/sample_import", "mojom/sample_import",
], function(console, hexdump, expect, sample, imported) { "mojom/sample_import2",
], function(console, hexdump, expect, sample, imported, imported2) {
var global = this; var global = this;
...@@ -117,12 +118,10 @@ define([ ...@@ -117,12 +118,10 @@ define([
expect(full.point.y).toBe(15); expect(full.point.y).toBe(15);
expect(full.shape_masks.length).toBe(1); expect(full.shape_masks.length).toBe(1);
// TODO(mpcomplete): This is broken. expect(full.shape_masks[0]).toBe(1 << imported.Shape.SHAPE_RECTANGLE);
// http://crbug.com/320082
// expect(full.shape_masks[0]).toBe(1 << imported.Shape.SHAPE_RECTANGLE);
//expect(full.thing.shape).toBe(imported.Shape.SHAPE_RECTANGLE); expect(full.thing.shape).toBe(imported.Shape.SHAPE_CIRCLE);
//expect(full.thing.color).toBe(imported.Color.COLOR_RED); expect(full.thing.color).toBe(imported2.Color.COLOR_BLACK);
} }
function ServiceImpl() { function ServiceImpl() {
......
enum {{enum.name}} { enum {{enum.name}} {
{%- for field in enum.fields %} {%- for field in enum.fields %}
{%- if field.value %} {%- if field.value %}
{{field.name}} = {{field.value}}, {{field.name}} = {{field.value|expression_to_text(module)}},
{%- else %} {%- else %}
{{field.name}}, {{field.name}},
{%- endif %} {%- endif %}
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
{%- macro set_default(kind, value, depth) -%} {%- macro set_default(kind, value, depth) -%}
{#--- Strings ---#} {#--- Strings ---#}
{%- if kind|is_string_kind -%} {%- if kind|is_string_kind -%}
{{caller("mojo::String(" ~ value ~ ")")}} {{caller("mojo::String(" ~ value|expression_to_text(module) ~ ")")}}
{#--- Arrays ---#} {#--- Arrays ---#}
{%- elif kind|is_array_kind %} {%- elif kind|is_array_kind %}
{%- set _ = value|verify_token_type("ARRAY") %} {%- set _ = value|verify_token_type("ARRAY") %}
...@@ -43,7 +43,7 @@ tmp{{depth}}.set_{{subfield.name}}({{result}}); ...@@ -43,7 +43,7 @@ tmp{{depth}}.set_{{subfield.name}}({{result}});
} }
{#--- POD types ---#} {#--- POD types ---#}
{%- else -%} {%- else -%}
{{caller(value|substitute_namespace(imports))}} {{caller(value|expression_to_text(module))}}
{%- endif %} {%- endif %}
{%- endmacro %} {%- endmacro %}
......
{%- macro enum_def(enum_init, enum) %} {%- macro enum_def(enum_name, enum, module) -%}
{{enum_init}} = { {{enum_name}} = {};
{%- set next_value = 0 %}
{%- set prev_enum = 0 %}
{%- for field in enum.fields %} {%- for field in enum.fields %}
{%- if field.value %} {%- if field.value %}
{%- set next_value = field.value|int %} {{enum_name}}.{{field.name}} = {{field.value|expression_to_text(module)}};
{%- elif loop.first %}
{{enum_name}}.{{field.name}} = 0;
{%- else %}
{{enum_name}}.{{field.name}} = {{enum_name}}.{{enum.fields[loop.index0 - 1].name}} + 1;
{%- endif %} {%- endif %}
{{field.name}}: {{next_value}},
{%- set next_value = next_value + 1 %}
{%- endfor %} {%- endfor %}
};
{%- endmacro %} {%- endmacro %}
...@@ -46,8 +46,8 @@ params.{{parameter.name}}{% if not loop.last %}, {% endif %} ...@@ -46,8 +46,8 @@ params.{{parameter.name}}{% if not loop.last %}, {% endif %}
}; };
{#--- Enums #} {#--- Enums #}
{%- from "enum_definition.tmpl" import enum_def -%} {% from "enum_definition.tmpl" import enum_def -%}
{% for enum in interface.enums %} {% for enum in interface.enums %}
{{ enum_def("%sProxy.%s"|format(interface.name, enum.name), enum)}} {{enum_def("%sProxy.%s"|format(interface.name, enum.name), enum, module)}}
{{interface.name}}Stub.{{enum.name}} = {{interface.name}}Proxy.{{enum.name}}; {{interface.name}}Stub.{{enum.name}} = {{interface.name}}Proxy.{{enum.name}};
{%- endfor %} {%- endfor %}
...@@ -15,14 +15,14 @@ define([ ...@@ -15,14 +15,14 @@ define([
) { ) {
{#--- Enums #} {#--- Enums #}
{%- from "enum_definition.tmpl" import enum_def -%} {% from "enum_definition.tmpl" import enum_def -%}
{% for enum in enums %} {% for enum in enums %}
{{ enum_def("var %s"|format(enum.name), enum) }} var {{ enum_def(enum.name, enum, module) }}
{%- endfor %} {%- endfor %}
{#--- Struct definitions #} {#--- Struct definitions #}
{% for struct in structs %} {% for struct in structs %}
{% include "struct_definition.tmpl" %} {%- include "struct_definition.tmpl" %}
{%- endfor %} {%- endfor %}
{#--- Interface definitions #} {#--- Interface definitions #}
......
{%- macro set_default(kind, value, depth) -%} {%- macro set_default(kind, value, depth) -%}
{#--- Strings ---#} {#--- Strings ---#}
{%- if kind|is_string_kind -%} {%- if kind|is_string_kind -%}
{{caller(value)}} {{caller(value|expression_to_text(module))}}
{#--- Arrays ---#} {#--- Arrays ---#}
{%- elif kind|is_array_kind %} {%- elif kind|is_array_kind %}
{%- set _ = value|verify_token_type("ARRAY") %} {%- set _ = value|verify_token_type("ARRAY") %}
...@@ -35,11 +35,12 @@ tmp{{depth}}.{{subfield.name}} = {{result}}; ...@@ -35,11 +35,12 @@ tmp{{depth}}.{{subfield.name}} = {{result}};
} }
{#--- POD types ---#} {#--- POD types ---#}
{%- else -%} {%- else -%}
{{caller(value|substitute_namespace(imports))}} {{caller(value|expression_to_text(module))}}
{%- endif %} {%- endif %}
{%- endmacro %} {%- endmacro %}
{#--- Begin #}
function {{struct.name}}() { function {{struct.name}}() {
{%- for packed_field in struct.packed.packed_fields %} {%- for packed_field in struct.packed.packed_fields %}
{%- if packed_field.field.default %} {%- if packed_field.field.default %}
......
...@@ -102,11 +102,17 @@ def IsStructWithHandles(struct): ...@@ -102,11 +102,17 @@ def IsStructWithHandles(struct):
return True return True
return False return False
def SubstituteNamespace(value, imports): def SubstituteNamespace(token, module):
for import_item in imports: for import_item in module.imports:
value = value.replace(import_item['namespace'] + ".", token = token.replace(import_item["namespace"] + ".",
import_item['namespace'] + "::") import_item["namespace"] + "::")
return value return token
def ExpressionToText(value, module):
if value[0] != "EXPRESSION":
raise Exception("Expected EXPRESSION, got" + value)
return "".join(mojom_generator.ExpressionMapper(value,
lambda token: SubstituteNamespace(token, module)))
_HEADER_SIZE = 8 _HEADER_SIZE = 8
...@@ -118,6 +124,7 @@ class Generator(mojom_generator.Generator): ...@@ -118,6 +124,7 @@ class Generator(mojom_generator.Generator):
"cpp_field_type": GetCppFieldType, "cpp_field_type": GetCppFieldType,
"cpp_type": GetCppType, "cpp_type": GetCppType,
"cpp_wrapper_type": GetCppWrapperType, "cpp_wrapper_type": GetCppWrapperType,
"expression_to_text": ExpressionToText,
"get_pad": mojom_pack.GetPad, "get_pad": mojom_pack.GetPad,
"is_handle_kind": mojom_generator.IsHandleKind, "is_handle_kind": mojom_generator.IsHandleKind,
"is_object_kind": mojom_generator.IsObjectKind, "is_object_kind": mojom_generator.IsObjectKind,
...@@ -127,12 +134,12 @@ class Generator(mojom_generator.Generator): ...@@ -127,12 +134,12 @@ class Generator(mojom_generator.Generator):
"struct_size": lambda ps: ps.GetTotalSize() + _HEADER_SIZE, "struct_size": lambda ps: ps.GetTotalSize() + _HEADER_SIZE,
"struct_from_method": mojom_generator.GetStructFromMethod, "struct_from_method": mojom_generator.GetStructFromMethod,
"stylize_method": mojom_generator.StudlyCapsToCamel, "stylize_method": mojom_generator.StudlyCapsToCamel,
"substitute_namespace": SubstituteNamespace,
"verify_token_type": mojom_generator.VerifyTokenType, "verify_token_type": mojom_generator.VerifyTokenType,
} }
def GetJinjaExports(self): def GetJinjaExports(self):
return { return {
"module": self.module,
"module_name": self.module.name, "module_name": self.module.name,
"namespace": self.module.namespace, "namespace": self.module.namespace,
"imports": self.module.imports, "imports": self.module.imports,
......
...@@ -10,7 +10,6 @@ from generate import mojom_generator ...@@ -10,7 +10,6 @@ from generate import mojom_generator
from generate.template_expander import UseJinja from generate.template_expander import UseJinja
_kind_to_javascript_default_value = { _kind_to_javascript_default_value = {
mojom.BOOL: "false", mojom.BOOL: "false",
mojom.INT8: "0", mojom.INT8: "0",
...@@ -148,13 +147,59 @@ def JavaScriptEncodeSnippet(kind): ...@@ -148,13 +147,59 @@ def JavaScriptEncodeSnippet(kind):
return JavaScriptEncodeSnippet(mojom.MSGPIPE) return JavaScriptEncodeSnippet(mojom.MSGPIPE)
def SubstituteNamespace(value, imports): def GetConstants(module):
for import_item in imports: """Returns a generator that enumerates all constants that can be referenced
value = value.replace(import_item["namespace"] + ".", from this module."""
import_item["unique_name"] + ".") class Constant:
pass
for enum in module.enums:
for field in enum.fields:
constant = Constant()
constant.namespace = module.namespace
constant.is_current_namespace = True
constant.import_item = None
constant.name = (enum.name, field.name)
yield constant
for each in module.imports:
for enum in each["module"].enums:
for field in enum.fields:
constant = Constant()
constant.namespace = each["namespace"]
constant.is_current_namespace = constant.namespace == module.namespace
constant.import_item = each
constant.name = (enum.name, field.name)
yield constant
def TranslateConstants(value, module):
# We're assuming we're dealing with an identifier, but that may not be
# the case. If we're not, we just won't find any matches.
if value.find(".") != -1:
namespace, identifier = value.split(".")
else:
namespace, identifier = "", value
for constant in GetConstants(module):
if namespace == constant.namespace or (
namespace == "" and constant.is_current_namespace):
if constant.name[1] == identifier:
if constant.import_item:
return "%s.%s.%s" % (constant.import_item["unique_name"],
constant.name[0], constant.name[1])
else:
return "%s.%s" % (constant.name[0], constant.name[1])
return value return value
def ExpressionToText(value, module):
if value[0] != "EXPRESSION":
raise Exception("Expected EXPRESSION, got" + value)
return "".join(mojom_generator.ExpressionMapper(value,
lambda token: TranslateConstants(token, module)))
def JavascriptType(kind): def JavascriptType(kind):
if kind.imported_from: if kind.imported_from:
return kind.imported_from["unique_name"] + "." + kind.name return kind.imported_from["unique_name"] + "." + kind.name
...@@ -169,12 +214,12 @@ class Generator(mojom_generator.Generator): ...@@ -169,12 +214,12 @@ class Generator(mojom_generator.Generator):
"payload_size": JavaScriptPayloadSize, "payload_size": JavaScriptPayloadSize,
"decode_snippet": JavaScriptDecodeSnippet, "decode_snippet": JavaScriptDecodeSnippet,
"encode_snippet": JavaScriptEncodeSnippet, "encode_snippet": JavaScriptEncodeSnippet,
"expression_to_text": ExpressionToText,
"is_object_kind": mojom_generator.IsObjectKind, "is_object_kind": mojom_generator.IsObjectKind,
"is_string_kind": mojom_generator.IsStringKind, "is_string_kind": mojom_generator.IsStringKind,
"is_array_kind": lambda kind: isinstance(kind, mojom.Array), "is_array_kind": lambda kind: isinstance(kind, mojom.Array),
"js_type": JavascriptType, "js_type": JavascriptType,
"stylize_method": mojom_generator.StudlyCapsToCamel, "stylize_method": mojom_generator.StudlyCapsToCamel,
"substitute_namespace": SubstituteNamespace,
"verify_token_type": mojom_generator.VerifyTokenType, "verify_token_type": mojom_generator.VerifyTokenType,
} }
...@@ -184,6 +229,7 @@ class Generator(mojom_generator.Generator): ...@@ -184,6 +229,7 @@ class Generator(mojom_generator.Generator):
"imports": self.GetImports(), "imports": self.GetImports(),
"kinds": self.module.kinds, "kinds": self.module.kinds,
"enums": self.module.enums, "enums": self.module.enums,
"module": self.module,
"structs": self.GetStructs() + self.GetStructsFromMethods(), "structs": self.GetStructs() + self.GetStructsFromMethods(),
"interfaces": self.module.interfaces, "interfaces": self.module.interfaces,
} }
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
}, },
'inputs': [ 'inputs': [
'<(mojom_bindings_generator)', '<(mojom_bindings_generator)',
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/enum_declaration.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_declaration.tmpl', '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_declaration.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl', '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl', '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl',
...@@ -30,6 +31,7 @@ ...@@ -30,6 +31,7 @@
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/struct_destructor.tmpl', '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/struct_destructor.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl', '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl', '<(DEPTH)/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/js_templates/enum_definition.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/js_templates/interface_definition.tmpl', '<(DEPTH)/mojo/public/bindings/generators/js_templates/interface_definition.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/js_templates/module.js.tmpl', '<(DEPTH)/mojo/public/bindings/generators/js_templates/module.js.tmpl',
'<(DEPTH)/mojo/public/bindings/generators/js_templates/struct_definition.tmpl', '<(DEPTH)/mojo/public/bindings/generators/js_templates/struct_definition.tmpl',
......
...@@ -60,6 +60,7 @@ def ImportFromData(module, data): ...@@ -60,6 +60,7 @@ def ImportFromData(module, data):
import_item = {} import_item = {}
import_item['module_name'] = import_module.name import_item['module_name'] = import_module.name
import_item['namespace'] = import_module.namespace import_item['namespace'] = import_module.namespace
import_item['module'] = import_module
# Copy the struct kinds from our imports into the current module. # Copy the struct kinds from our imports into the current module.
for kind in import_module.kinds.itervalues(): for kind in import_module.kinds.itervalues():
......
...@@ -53,6 +53,14 @@ def VerifyTokenType(token, expected): ...@@ -53,6 +53,14 @@ def VerifyTokenType(token, expected):
raise Exception("Expected token type '%s'. Got '%s'." % raise Exception("Expected token type '%s'. Got '%s'." %
(expected, token[0])) (expected, token[0]))
def ExpressionMapper(expression, mapper):
if isinstance(expression, tuple) and expression[0] == 'EXPRESSION':
result = []
for each in expression[1]:
result.extend(ExpressionMapper(each, mapper))
return result
return [mapper(expression)]
class Generator(object): class Generator(object):
# Pass |output_dir| to emit files to disk. Omit |output_dir| to echo all # Pass |output_dir| to emit files to disk. Omit |output_dir| to echo all
# files to stdout. # files to stdout.
......
...@@ -243,7 +243,7 @@ class Parser(object): ...@@ -243,7 +243,7 @@ class Parser(object):
def p_expression(self, p): def p_expression(self, p):
"""expression : conditional_expression""" """expression : conditional_expression"""
p[0] = p[1] p[0] = ('EXPRESSION', p[1])
def p_conditional_expression(self, p): def p_conditional_expression(self, p):
"""conditional_expression : binary_expression """conditional_expression : binary_expression
...@@ -251,7 +251,7 @@ class Parser(object): ...@@ -251,7 +251,7 @@ class Parser(object):
conditional_expression""" conditional_expression"""
# Just pass the arguments through. I don't think it's possible to preserve # Just pass the arguments through. I don't think it's possible to preserve
# the spaces of the original, so just put a single space between them. # the spaces of the original, so just put a single space between them.
p[0] = ' '.join(p[1:]) p[0] = ListFromConcat(*p[1:])
# PLY lets us specify precedence of operators, but since we don't actually # PLY lets us specify precedence of operators, but since we don't actually
# evaluate them, we don't need that here. # evaluate them, we don't need that here.
...@@ -259,7 +259,7 @@ class Parser(object): ...@@ -259,7 +259,7 @@ class Parser(object):
"""binary_expression : unary_expression """binary_expression : unary_expression
| binary_expression binary_operator \ | binary_expression binary_operator \
binary_expression""" binary_expression"""
p[0] = ' '.join(p[1:]) p[0] = ListFromConcat(*p[1:])
def p_binary_operator(self, p): def p_binary_operator(self, p):
"""binary_operator : TIMES """binary_operator : TIMES
...@@ -285,7 +285,7 @@ class Parser(object): ...@@ -285,7 +285,7 @@ class Parser(object):
def p_unary_expression(self, p): def p_unary_expression(self, p):
"""unary_expression : primary_expression """unary_expression : primary_expression
| unary_operator expression""" | unary_operator expression"""
p[0] = ''.join(p[1:]) p[0] = ListFromConcat(*p[1:])
def p_unary_operator(self, p): def p_unary_operator(self, p):
"""unary_operator : PLUS """unary_operator : PLUS
...@@ -296,9 +296,13 @@ class Parser(object): ...@@ -296,9 +296,13 @@ class Parser(object):
def p_primary_expression(self, p): def p_primary_expression(self, p):
"""primary_expression : constant """primary_expression : constant
| NAME | identifier
| NAME DOT NAME
| LPAREN expression RPAREN""" | LPAREN expression RPAREN"""
p[0] = ListFromConcat(*p[1:])
def p_identifier(self, p):
"""identifier : NAME
| NAME DOT NAME"""
p[0] = ''.join(p[1:]) p[0] = ''.join(p[1:])
def p_constant(self, p): def p_constant(self, p):
...@@ -311,7 +315,7 @@ class Parser(object): ...@@ -311,7 +315,7 @@ class Parser(object):
| WCHAR_CONST | WCHAR_CONST
| STRING_LITERAL | STRING_LITERAL
| WSTRING_LITERAL""" | WSTRING_LITERAL"""
p[0] = ''.join(p[1:]) p[0] = ListFromConcat(*p[1:])
def p_error(self, e): def p_error(self, e):
print('error: %s'%e) print('error: %s'%e)
......
...@@ -20,8 +20,8 @@ struct Size { ...@@ -20,8 +20,8 @@ struct Size {
}; };
struct Thing { struct Thing {
int32 shape; int32 shape = SHAPE_RECTANGLE;
int32 color; int32 color = COLOR_BLACK;
Point location = {0, 0}; Point location = {0, 0};
Size size; Size size;
}; };
......
...@@ -48,7 +48,7 @@ struct DefaultsTest { ...@@ -48,7 +48,7 @@ struct DefaultsTest {
uint8[] data = [1, 2, 3] @2; uint8[] data = [1, 2, 3] @2;
imported.Point point = {7, 15} @3; imported.Point point = {7, 15} @3;
int32[] shape_masks = [1 << imported.SHAPE_RECTANGLE] @4; int32[] shape_masks = [1 << imported.SHAPE_RECTANGLE] @4;
imported.Thing thing = {imported.SHAPE_RECTANGLE, imported.COLOR_RED}; imported.Thing thing = {imported.SHAPE_CIRCLE, imported.COLOR_BLACK};
}; };
interface Port { interface Port {
......
...@@ -343,8 +343,8 @@ TEST(BindingsSampleTest, DefaultValues) { ...@@ -343,8 +343,8 @@ TEST(BindingsSampleTest, DefaultValues) {
EXPECT_EQ(1u, full.shape_masks().size()); EXPECT_EQ(1u, full.shape_masks().size());
EXPECT_EQ(1 << imported::SHAPE_RECTANGLE, full.shape_masks()[0]); EXPECT_EQ(1 << imported::SHAPE_RECTANGLE, full.shape_masks()[0]);
EXPECT_EQ(imported::SHAPE_RECTANGLE, full.thing().shape()); EXPECT_EQ(imported::SHAPE_CIRCLE, full.thing().shape());
EXPECT_EQ(imported::COLOR_RED, full.thing().color()); EXPECT_EQ(imported::COLOR_BLACK, full.thing().color());
} }
} // namespace sample } // namespace sample
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment