From 5be92d7c1776fe65a7d3d959c6a14a229e218c2b Mon Sep 17 00:00:00 2001
From: "mpcomplete@chromium.org"
 <mpcomplete@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 28 Feb 2014 20:59:08 +0000
Subject: Allow enums to be used as type names in mojom.

This is more complicated than I expected. A lot of complexity is to support enums inside structs/interfaces. Some noteworthy things about this patch:
- Looking up a Kind when encountering a struct field or method parameter got trickier. LookupKind handles the scope resolution rules.
- Inner enums are still not fully supported. You can't refer to them outside the struct they were defined in.
- We assume all enums take up 4 bytes. I'm not sure how to fix this.

Also fixed a small bug with struct padding.

BUG=320090,345119

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254221 0039d316-1c4b-4281-b951-d872f2087c98
---
 mojo/apps/js/bindings/sample_service_unittests.js  |  8 +-
 .../cpp_templates/interface_definition.tmpl        |  2 +
 .../cpp_templates/struct_declaration.tmpl          |  7 +-
 .../generators/cpp_templates/struct_macros.tmpl    |  8 +-
 .../cpp_templates/wrapper_class_declaration.tmpl   |  9 +-
 .../generators/js_templates/struct_definition.tmpl | 12 +--
 .../bindings/generators/mojom_cpp_generator.py     | 34 ++++++--
 .../bindings/generators/mojom_js_generator.py      | 53 +++++++-----
 mojo/public/bindings/pylib/generate/mojom.py       | 33 ++------
 mojo/public/bindings/pylib/generate/mojom_data.py  | 96 +++++++++++++++-------
 .../bindings/pylib/generate/mojom_generator.py     |  3 +
 mojo/public/bindings/pylib/generate/mojom_pack.py  |  5 ++
 mojo/public/bindings/pylib/parse/mojo_translate.py |  2 -
 mojo/public/bindings/tests/sample_import2.mojom    |  2 +-
 mojo/public/bindings/tests/sample_service.mojom    | 17 ++--
 .../bindings/tests/sample_service_unittest.cc      | 10 +--
 16 files changed, 181 insertions(+), 120 deletions(-)

(limited to 'mojo')

diff --git a/mojo/apps/js/bindings/sample_service_unittests.js b/mojo/apps/js/bindings/sample_service_unittests.js
index b43bca3..f3a1986 100644
--- a/mojo/apps/js/bindings/sample_service_unittests.js
+++ b/mojo/apps/js/bindings/sample_service_unittests.js
@@ -21,13 +21,13 @@ define([
     bar.alpha = 20;
     bar.beta = 40;
     bar.gamma = 60;
-    bar.type = sample.BarType.BAR_VERTICAL;
+    bar.type = sample.Bar.Type.TYPE_VERTICAL;
 
     var extra_bars = new Array(3);
     for (var i = 0; i < extra_bars.length; ++i) {
       var base = i * 100;
       var type = i % 2 ?
-          sample.BarType.BAR_VERTICAL : sample.BarType.BAR_HORIZONTAL;
+          sample.Bar.Type.TYPE_VERTICAL : sample.Bar.Type.TYPE_HORIZONTAL;
       extra_bars[i] = new sample.Bar();
       extra_bars[i].alpha = base;
       extra_bars[i].beta = base + 20;
@@ -67,13 +67,13 @@ define([
     expect(foo.bar.alpha).toBe(20);
     expect(foo.bar.beta).toBe(40);
     expect(foo.bar.gamma).toBe(60);
-    expect(foo.bar.type).toBe(sample.BarType.BAR_VERTICAL);
+    expect(foo.bar.type).toBe(sample.Bar.Type.TYPE_VERTICAL);
 
     expect(foo.extra_bars.length).toBe(3);
     for (var i = 0; i < foo.extra_bars.length; ++i) {
       var base = i * 100;
       var type = i % 2 ?
-          sample.BarType.BAR_VERTICAL : sample.BarType.BAR_HORIZONTAL;
+          sample.Bar.Type.TYPE_VERTICAL : sample.Bar.Type.TYPE_HORIZONTAL;
       expect(foo.extra_bars[i].alpha).toBe(base);
       expect(foo.extra_bars[i].beta).toBe(base + 20);
       expect(foo.extra_bars[i].gamma).toBe(base + 40);
diff --git a/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl b/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl
index fc6c17a..f4198f1 100644
--- a/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl
+++ b/mojo/public/bindings/generators/cpp_templates/interface_definition.tmpl
@@ -66,6 +66,8 @@ void {{proxy_name}}::{{method.name}}({{params_list(method)}}) {
 mojo::internal::Wrap(params->{{param.name}}())
 {%-     elif param.kind|is_handle_kind -%}
 mojo::MakePassable(params->{{param.name}}()).Pass()
+{%-     elif param.kind|is_enum_kind -%}
+static_cast<{{param.kind|cpp_wrapper_type}}>(params->{{param.name}}())
 {%-     else -%}
 params->{{param.name}}()
 {%-     endif -%}
diff --git a/mojo/public/bindings/generators/cpp_templates/struct_declaration.tmpl b/mojo/public/bindings/generators/cpp_templates/struct_declaration.tmpl
index a4bfe21..701ad33 100644
--- a/mojo/public/bindings/generators/cpp_templates/struct_declaration.tmpl
+++ b/mojo/public/bindings/generators/cpp_templates/struct_declaration.tmpl
@@ -1,15 +1,10 @@
 {%- import "struct_macros.tmpl" as struct_macros %}
 {%- set class_name = struct.name ~ "_Data" -%}
+
 class {{class_name}} {
  public:
   typedef {{struct.name}} Wrapper;
 
-{#--- Enums #}
-{%- for enum in struct.enums %}
-{%    macro enum_def() %}{% include "enum_declaration.tmpl" %}{% endmacro %}
-  {{enum_def()|indent(2)}}
-{%- endfor %}
-
   static {{class_name}}* New(mojo::Buffer* buf, mojo::Buffer::Destructor dtor = NULL);
 
 {#--- Setters -#}
diff --git a/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl b/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl
index 9e7706d..9d09052 100644
--- a/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl
+++ b/mojo/public/bindings/generators/cpp_templates/struct_macros.tmpl
@@ -31,21 +31,21 @@
   uint8_t {{name}}_ : 1;
 {%-   elif field.kind|is_handle_kind -%}
   mutable {{type}} {{name}}_;
+{%-   elif field.kind|is_enum_kind -%}
+  int32_t {{name}}_;
 {%-   else -%}
   {{type}} {{name}}_;
 {%-   endif %}
 {%- endmacro %}
 
 {%- macro fields(struct) %}
-{%-   set pad_count = 0 %}
 {%-   for packed_field in struct.packed.packed_fields %}
   {{field_line(packed_field.field)}}
 {%-     if not loop.last %}
 {%-       set next_pf = struct.packed.packed_fields[loop.index0 + 1] %}
 {%-       set pad = next_pf.offset - (packed_field.offset + packed_field.size) %}
 {%-       if pad > 0 %}
-  uint8_t _pad{{pad_count}}_[{{pad}}];
-{%-         set pad_count = pad_count + 1 %}
+  uint8_t _pad{{loop.index0}}_[{{pad}}];
 {%-       endif %}
 {%-     endif %}
 {%-   endfor -%}
@@ -56,7 +56,7 @@
 {%-     set offset = last_field.offset + last_field.size %}
 {%-     set pad = offset|get_pad(8) -%}
 {%-     if pad > 0 %}
-  uint8_t _pad{{pad_count}}_[{{pad}}];
+  uint8_t _padfinal_[{{pad}}];
 {%-     endif %}
 {%-   endif %}
 {%- endmacro %}
diff --git a/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl b/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
index 46bdcd3..8d41dab 100644
--- a/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
+++ b/mojo/public/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
@@ -1,8 +1,11 @@
 class {{struct.name}} {
  public:
   typedef internal::{{struct.name}}_Data Data;
-{%- for enum in struct.enums %}
-  typedef internal::{{enum.name}} {{enum.name}};
+
+{#--- Enums #}
+{%- for enum in struct.enums -%}
+{%    macro enum_def() %}{% include "enum_declaration.tmpl" %}{% endmacro %}
+  {{enum_def()|indent(2)}}
 {%- endfor %}
 
   {{struct.name}}() : data_(NULL) {
@@ -41,6 +44,8 @@ class {{struct.name}} {
     #}return mojo::internal::Wrap(data_->{{name}}()); }
 {%-   elif packed_field.field.kind|is_handle_kind %}
   {{type}} {{name}}() const { return mojo::MakePassable(data_->{{name}}()); }
+{%-   elif packed_field.field.kind|is_enum_kind %}
+  {{type}} {{name}}() const { return static_cast<{{type}}>(data_->{{name}}()); }
 {%-   else %}
   {{type}} {{name}}() const { return data_->{{name}}(); }
 {%-   endif %}
diff --git a/mojo/public/bindings/generators/js_templates/struct_definition.tmpl b/mojo/public/bindings/generators/js_templates/struct_definition.tmpl
index e80e675..a778b8b 100644
--- a/mojo/public/bindings/generators/js_templates/struct_definition.tmpl
+++ b/mojo/public/bindings/generators/js_templates/struct_definition.tmpl
@@ -1,4 +1,4 @@
-{%- macro set_default(kind, value, depth) -%}
+{%- macro set_default(module, kind, value, depth) -%}
 {#--- Strings ---#}
 {%-   if kind|is_string_kind -%}
 {{caller(value|expression_to_text(module))}}
@@ -9,7 +9,7 @@
   var tmp{{depth}} = [];
 {%-     for element in value[1] %}
 {%-       filter indent(2) %}
-{%-         call(result) set_default(kind.kind, element, depth+1) %}
+{%-         call(result) set_default(module, kind.kind, element, depth+1) %}
 tmp{{depth}}[{{loop.index0}}] = {{result}};
 {%-         endcall %}
 {%-       endfilter %}
@@ -26,7 +26,7 @@ tmp{{depth}}[{{loop.index0}}] = {{result}};
 {#-     Use struct.packed_fields to order struct values by ordinal number #}
 {%-     set subfield = struct.fields[loop.index0] %}
 {%-     filter indent(2) %}
-{%-       call(result) set_default(subfield.kind, element, depth+1) %}
+{%-       call(result) set_default(module, subfield.kind, element, depth+1) %}
 tmp{{depth}}.{{subfield.name}} = {{result}};
 {%-       endcall %}
 {%-     endfilter %}
@@ -45,7 +45,7 @@ tmp{{depth}}.{{subfield.name}} = {{result}};
 {%- for packed_field in struct.packed.packed_fields %}
 {%- if packed_field.field.default %}
 {%-   filter indent(4) %}
-{%-     call(result) set_default(packed_field.field.kind, packed_field.field.default, 0) %}
+{%-     call(result) set_default(module, packed_field.field.kind, packed_field.field.default, 0) %}
 this.{{packed_field.field.name}} = {{result}};
 {%-     endcall %}
 {%-   endfilter %}
@@ -56,9 +56,9 @@ this.{{packed_field.field.name}} = {{result}};
   }
 
 {#--- Enums #}
-{%- from "enum_definition.tmpl" import enum_def -%}
+{%- from "enum_definition.tmpl" import enum_def %}
 {%  for enum in struct.enums %}
-{{  enum_def("%s.%s"|format(struct.name, enum.name), enum)}}
+  {{enum_def("%s.%s"|format(struct.name, enum.name), enum, module)}}
 {%  endfor %}
 
 {#--- Encoding and decoding #}
diff --git a/mojo/public/bindings/generators/mojom_cpp_generator.py b/mojo/public/bindings/generators/mojom_cpp_generator.py
index dcff240..1a0d8cc 100644
--- a/mojo/public/bindings/generators/mojom_cpp_generator.py
+++ b/mojo/public/bindings/generators/mojom_cpp_generator.py
@@ -30,20 +30,33 @@ _kind_to_cpp_type = {
 }
 
 
+def GetNameForKind(kind, internal = False):
+  parts = []
+  if kind.imported_from:
+    parts.append(kind.imported_from["namespace"])
+    if internal:
+      parts.append("internal")
+  if kind.parent_kind:
+    parts.append(kind.parent_kind.name)
+  parts.append(kind.name)
+  return "::".join(parts)
+
 def GetCppType(kind):
   if isinstance(kind, mojom.Struct):
-    return "%s_Data*" % kind.GetFullNameInternal("::")
+    return "%s_Data*" % GetNameForKind(kind, internal=True)
   if isinstance(kind, mojom.Array):
     return "mojo::internal::Array_Data<%s>*" % GetCppType(kind.kind)
   if isinstance(kind, mojom.Interface):
     return "mojo::Interface<%s>::Handle" % kind.name
+  if isinstance(kind, mojom.Enum):
+    return "int32_t"
   if kind.spec == 's':
     return "mojo::internal::String_Data*"
   return _kind_to_cpp_type[kind]
 
 def GetCppArrayArgWrapperType(kind):
-  if isinstance(kind, mojom.Struct):
-    return kind.GetFullName("::")
+  if isinstance(kind, (mojom.Struct, mojom.Enum)):
+    return GetNameForKind(kind)
   if isinstance(kind, mojom.Array):
     return "mojo::Array<%s >" % GetCppArrayArgWrapperType(kind.kind)
   if isinstance(kind, mojom.Interface):
@@ -53,8 +66,8 @@ def GetCppArrayArgWrapperType(kind):
   return _kind_to_cpp_type[kind]
 
 def GetCppWrapperType(kind):
-  if isinstance(kind, mojom.Struct):
-    return kind.GetFullName("::")
+  if isinstance(kind, (mojom.Struct, mojom.Enum)):
+    return GetNameForKind(kind)
   if isinstance(kind, mojom.Array):
     return "mojo::Array<%s >" % GetCppArrayArgWrapperType(kind.kind)
   if isinstance(kind, mojom.Interface):
@@ -67,11 +80,13 @@ def GetCppWrapperType(kind):
 
 def GetCppConstWrapperType(kind):
   if isinstance(kind, mojom.Struct):
-    return "const %s&" % kind.GetFullName("::")
+    return "const %s&" % GetNameForKind(kind)
   if isinstance(kind, mojom.Array):
     return "const mojo::Array<%s >&" % GetCppArrayArgWrapperType(kind.kind)
   if isinstance(kind, mojom.Interface):
     return "mojo::Interface<%s>::ScopedHandle" % kind.name
+  if isinstance(kind, mojom.Enum):
+    return GetNameForKind(kind)
   if kind.spec == 's':
     return "const mojo::String&"
   if kind.spec == 'h':
@@ -82,16 +97,20 @@ def GetCppConstWrapperType(kind):
     return "mojo::ScopedDataPipeProducerHandle"
   if kind.spec == 'h:m':
     return "mojo::ScopedMessagePipeHandle"
+  if not kind in _kind_to_cpp_type:
+    print "missing:", kind.spec
   return _kind_to_cpp_type[kind]
 
 def GetCppFieldType(kind):
   if isinstance(kind, mojom.Struct):
     return ("mojo::internal::StructPointer<%s_Data>" %
-        kind.GetFullNameInternal("::"))
+        GetNameForKind(kind, internal=True))
   if isinstance(kind, mojom.Array):
     return "mojo::internal::ArrayPointer<%s>" % GetCppType(kind.kind)
   if isinstance(kind, mojom.Interface):
     return "mojo::Interface<%s>::Handle" % kind.name
+  if isinstance(kind, mojom.Enum):
+    return GetNameForKind(kind)
   if kind.spec == 's':
     return "mojo::internal::StringPointer"
   return _kind_to_cpp_type[kind]
@@ -126,6 +145,7 @@ class Generator(mojom_generator.Generator):
     "cpp_wrapper_type": GetCppWrapperType,
     "expression_to_text": ExpressionToText,
     "get_pad": mojom_pack.GetPad,
+    "is_enum_kind": mojom_generator.IsEnumKind,
     "is_handle_kind": mojom_generator.IsHandleKind,
     "is_object_kind": mojom_generator.IsObjectKind,
     "is_string_kind": mojom_generator.IsStringKind,
diff --git a/mojo/public/bindings/generators/mojom_js_generator.py b/mojo/public/bindings/generators/mojom_js_generator.py
index 75c4fb7..e52701f 100644
--- a/mojo/public/bindings/generators/mojom_js_generator.py
+++ b/mojo/public/bindings/generators/mojom_js_generator.py
@@ -41,6 +41,8 @@ def JavaScriptDefaultValue(field):
     return "[]";
   if isinstance(field.kind, mojom.Interface):
     return _kind_to_javascript_default_value[mojom.MSGPIPE]
+  if isinstance(field.kind, mojom.Enum):
+    return "0"
 
 
 def JavaScriptPayloadSize(packed):
@@ -82,6 +84,8 @@ def GetJavaScriptType(kind):
     return "new codec.ArrayOf(%s)" % GetJavaScriptType(kind.kind)
   if isinstance(kind, mojom.Interface):
     return GetJavaScriptType(mojom.MSGPIPE)
+  if isinstance(kind, mojom.Enum):
+    return _kind_to_javascript_type[mojom.INT32]
   return kind
 
 
@@ -114,6 +118,8 @@ def JavaScriptDecodeSnippet(kind):
     return "decodeArrayPointer(%s)" % GetJavaScriptType(kind.kind);
   if isinstance(kind, mojom.Interface):
     return JavaScriptDecodeSnippet(mojom.MSGPIPE)
+  if isinstance(kind, mojom.Enum):
+    return _kind_to_javascript_decode_snippet[mojom.INT32]
 
 
 _kind_to_javascript_encode_snippet = {
@@ -145,32 +151,43 @@ def JavaScriptEncodeSnippet(kind):
     return "encodeArrayPointer(%s, " % GetJavaScriptType(kind.kind);
   if isinstance(kind, mojom.Interface):
     return JavaScriptEncodeSnippet(mojom.MSGPIPE)
-
+  if isinstance(kind, mojom.Enum):
+    return _kind_to_javascript_encode_snippet[mojom.INT32]
 
 def GetConstants(module):
   """Returns a generator that enumerates all constants that can be referenced
   from this module."""
   class Constant:
-    pass
+    def __init__(self, module, enum, field, imported_from):
+      self.namespace = \
+          imported_from["namespace"] if imported_from else module.namespace
+      self.is_current_namespace = self.namespace == module.namespace
+      self.imported_from = imported_from
+      self.name = []
+      if imported_from:
+        self.name.append(imported_from["unique_name"])
+      if enum.parent_kind:
+        self.name.append(enum.parent_kind.name)
+      self.name.extend([enum.name, field.name])
 
   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
+      yield Constant(module, enum, field, None)
+
+  for struct in module.structs:
+    for enum in struct.enums:
+      for field in enum.fields:
+        yield Constant(module, enum, field, None)
+
+  for interface in module.interfaces:
+    for enum in interface.enums:
+      for field in enum.fields:
+        yield Constant(module, enum, field, None)
 
   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
+        yield Constant(module, enum, field, each)
 
 
 def TranslateConstants(value, module):
@@ -184,12 +201,8 @@ def TranslateConstants(value, module):
   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])
+      if constant.name[-1] == identifier:
+        return ".".join(constant.name)
   return value
 
 
diff --git a/mojo/public/bindings/pylib/generate/mojom.py b/mojo/public/bindings/pylib/generate/mojom.py
index 7cb7518..04d9dd1 100644
--- a/mojo/public/bindings/pylib/generate/mojom.py
+++ b/mojo/public/bindings/pylib/generate/mojom.py
@@ -11,12 +11,11 @@
 # method = interface.AddMethod('Tat', 0)
 # method.AddParameter('baz', 0, mojom.INT32)
 #
-import copy
 
 class Kind(object):
   def __init__(self, spec = None):
     self.spec = spec
-
+    self.parent_kind = None
 
 # Initialize the set of primitive types. These can be accessed by clients.
 BOOL    = Kind('b')
@@ -77,33 +76,11 @@ class Struct(Kind):
     Kind.__init__(self, spec)
     self.fields = []
 
-  @classmethod
-  def CreateFromImport(cls, kind, imported_from):
-    """Used with 'import module' - clones the kind imported from the
-    given module's namespace."""
-    kind = copy.deepcopy(kind)
-    kind.imported_from = imported_from
-    return kind
-
   def AddField(self, name, kind, ordinal = None, default = None):
     field = Field(name, kind, ordinal, default)
     self.fields.append(field)
     return field
 
-  def GetFullName(self, separator):
-    """Returns the fully qualified type name, including namespace prefix."""
-    if self.imported_from:
-      return separator.join([self.imported_from["namespace"], self.name])
-    return self.name
-
-  def GetFullNameInternal(self, separator):
-    """Returns the fully qualified type name for an internal data structure,
-    including namespace prefix."""
-    if self.imported_from:
-      return separator.join(
-          [self.imported_from["namespace"], "internal", self.name])
-    return self.name
-
 
 class Array(Kind):
   def __init__(self, kind = None):
@@ -157,9 +134,15 @@ class EnumField(object):
     self.value = value
 
 
-class Enum(object):
+class Enum(Kind):
   def __init__(self, name = None):
     self.name = name
+    self.imported_from = None
+    if name != None:
+      spec = 'x:' + name
+    else:
+      spec = None
+    Kind.__init__(self, spec)
     self.fields = []
 
 
diff --git a/mojo/public/bindings/pylib/generate/mojom_data.py b/mojo/public/bindings/pylib/generate/mojom_data.py
index 9a575a8..9a580f8 100644
--- a/mojo/public/bindings/pylib/generate/mojom_data.py
+++ b/mojo/public/bindings/pylib/generate/mojom_data.py
@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import mojom
+import copy
 
 # mojom_data provides a mechanism to turn mojom Modules to dictionaries and
 # back again. This can be used to persist a mojom Module created progromatically
@@ -39,21 +40,52 @@ def istr(index, string):
   istr.__index__ = index
   return istr
 
+def LookupKind(kinds, spec, scope):
+  """Tries to find which Kind a spec refers to, given the scope in which its
+  referenced. Starts checking from the narrowest scope to most general. For
+  example, given a struct field like
+    Foo.Bar x;
+  Foo.Bar could refer to the type 'Bar' in the 'Foo' namespace, or an inner
+  type 'Bar' in the struct 'Foo' in the current namespace.
+
+  |scope| is a tuple that looks like (namespace, struct/interface), referring
+  to the location where the type is referenced."""
+  if spec.startswith('x:'):
+    name = spec[2:]
+    for i in [2, 1, 0]:
+      test_spec = 'x:'
+      if i > 0:
+        test_spec += '.'.join(scope[:i]) + '.'
+      test_spec += name
+      kind = kinds.get(test_spec)
+      if kind:
+        return kind
+
+  return kinds.get(spec)
+
 def KindToData(kind):
   return kind.spec
 
-def KindFromData(kinds, data):
-  if kinds.has_key(data):
-    return kinds[data]
+def KindFromData(kinds, data, scope):
+  kind = LookupKind(kinds, data, scope)
+  if kind:
+    return kind
   if data.startswith('a:'):
     kind = mojom.Array()
-    kind.kind = KindFromData(kinds, data[2:])
+    kind.kind = KindFromData(kinds, data[2:], scope)
   else:
     kind = mojom.Kind()
   kind.spec = data
   kinds[data] = kind
   return kind
 
+def KindFromImport(original_kind, imported_from):
+  """Used with 'import module' - clones the kind imported from the
+  given module's namespace. Only used with Structs and Enums."""
+  kind = copy.deepcopy(original_kind)
+  kind.imported_from = imported_from
+  return kind
+
 def ImportFromData(module, data):
   import_module = data['module']
 
@@ -64,9 +96,9 @@ def ImportFromData(module, data):
 
   # Copy the struct kinds from our imports into the current module.
   for kind in import_module.kinds.itervalues():
-    # TODO(mpcomplete): Handle enums
-    if isinstance(kind, mojom.Struct) and kind.imported_from is None:
-      kind = mojom.Struct.CreateFromImport(kind, import_item)
+    if (isinstance(kind, (mojom.Struct, mojom.Enum)) and
+        kind.imported_from is None):
+      kind = KindFromImport(kind, import_item)
       module.kinds[kind.spec] = kind
   return import_item
 
@@ -81,11 +113,10 @@ def StructFromData(module, data):
   struct.name = data['name']
   struct.spec = 'x:' + module.namespace + '.' + struct.name
   module.kinds[struct.spec] = struct
+  struct.enums = map(lambda enum:
+      EnumFromData(module, enum, struct), data['enums'])
   struct.fields = map(lambda field:
-      FieldFromData(module.kinds, field), data['fields'])
-  if data.has_key('enums'):
-    struct.enums = map(lambda enum:
-        EnumFromData(module.kinds, enum), data['enums'])
+      FieldFromData(module, field, struct), data['fields'])
   return struct
 
 def FieldToData(field):
@@ -99,10 +130,11 @@ def FieldToData(field):
     data[istr(3, 'default')] = field.default
   return data
 
-def FieldFromData(kinds, data):
+def FieldFromData(module, data, struct):
   field = mojom.Field()
   field.name = data['name']
-  field.kind = KindFromData(kinds, data['kind'])
+  field.kind = KindFromData(
+      module.kinds, data['kind'], (module.namespace, struct.name))
   field.ordinal = data.get('ordinal')
   field.default = data.get('default')
   return field
@@ -118,10 +150,11 @@ def ParameterToData(parameter):
     data[istr(3, 'default')] = parameter.default
   return data
 
-def ParameterFromData(kinds, data):
+def ParameterFromData(module, data, interface):
   parameter = mojom.Parameter()
   parameter.name = data['name']
-  parameter.kind = KindFromData(kinds, data['kind'])
+  parameter.kind = KindFromData(
+      module.kinds, data['kind'], (module.namespace, interface.name))
   parameter.ordinal = data.get('ordinal')
   parameter.default = data.get('default')
   return parameter
@@ -135,13 +168,13 @@ def MethodToData(method):
     data[istr(2, 'ordinal')] = method.ordinal
   return data
 
-def MethodFromData(kinds, data):
+def MethodFromData(module, data, interface):
   method = mojom.Method()
   method.name = data['name']
   method.ordinal = data.get('ordinal')
   method.default = data.get('default')
-  method.parameters = map(
-      lambda parameter: ParameterFromData(kinds, parameter), data['parameters'])
+  method.parameters = map(lambda parameter:
+      ParameterFromData(module, parameter, interface), data['parameters'])
   return method
 
 def InterfaceToData(interface):
@@ -157,24 +190,30 @@ def InterfaceFromData(module, data):
   interface.spec = 'x:' + module.namespace + '.' + interface.name
   interface.peer = data['peer'] if data.has_key('peer') else None
   module.kinds[interface.spec] = interface
-  interface.methods = map(
-      lambda method: MethodFromData(module.kinds, method), data['methods'])
-  if data.has_key('enums'):
-    interface.enums = map(lambda enum:
-        EnumFromData(module.kinds, enum), data['enums'])
+  interface.enums = map(lambda enum:
+      EnumFromData(module, enum, interface), data['enums'])
+  interface.methods = map(lambda method:
+      MethodFromData(module, method, interface), data['methods'])
   return interface
 
-def EnumFieldFromData(kinds, data):
+def EnumFieldFromData(module, data):
   field = mojom.EnumField()
   field.name = data['name']
   field.value = data['value']
   return field
 
-def EnumFromData(kinds, data):
+def EnumFromData(module, data, parent_kind):
   enum = mojom.Enum()
   enum.name = data['name']
+  name = enum.name
+  if parent_kind:
+    name = parent_kind.name + '.' + name
+  enum.spec = 'x:%s.%s' % (module.namespace, name)
+  enum.parent_kind = parent_kind
+
   enum.fields = map(
-      lambda field: EnumFieldFromData(kinds, field), data['fields'])
+      lambda field: EnumFieldFromData(module, field), data['fields'])
+  module.kinds[enum.spec] = enum
   return enum
 
 def ModuleToData(module):
@@ -198,14 +237,13 @@ def ModuleFromData(data):
   module.imports = map(
       lambda import_data: ImportFromData(module, import_data),
       data['imports'])
+  module.enums = map(
+      lambda enum: EnumFromData(module, enum, None), data['enums'])
   module.structs = map(
       lambda struct: StructFromData(module, struct), data['structs'])
   module.interfaces = map(
       lambda interface: InterfaceFromData(module, interface),
       data['interfaces'])
-  if data.has_key('enums'):
-    module.enums = map(
-        lambda enum: EnumFromData(module.kinds, enum), data['enums'])
   return module
 
 def OrderedModuleFromData(data):
diff --git a/mojo/public/bindings/pylib/generate/mojom_generator.py b/mojo/public/bindings/pylib/generate/mojom_generator.py
index cf3a84f..a65e95e 100644
--- a/mojo/public/bindings/pylib/generate/mojom_generator.py
+++ b/mojo/public/bindings/pylib/generate/mojom_generator.py
@@ -28,6 +28,9 @@ def GetStructInfo(exported, struct):
 def IsStringKind(kind):
   return kind.spec == 's'
 
+def IsEnumKind(kind):
+  return isinstance(kind, mojom.Enum)
+
 def IsObjectKind(kind):
   return isinstance(kind, (mojom.Struct, mojom.Array)) or IsStringKind(kind)
 
diff --git a/mojo/public/bindings/pylib/generate/mojom_pack.py b/mojo/public/bindings/pylib/generate/mojom_pack.py
index 51e4279..2a65e18 100644
--- a/mojo/public/bindings/pylib/generate/mojom_pack.py
+++ b/mojo/public/bindings/pylib/generate/mojom_pack.py
@@ -37,6 +37,11 @@ class PackedField(object):
       return 8
     if isinstance(kind, mojom.Interface):
       kind = mojom.MSGPIPE
+    if isinstance(kind, mojom.Enum):
+      # TODO(mpcomplete): what about big enums?
+      return cls.kind_to_size[mojom.INT32]
+    if not kind in cls.kind_to_size:
+      raise Exception("Invalid kind: %s" % kind.spec)
     return cls.kind_to_size[kind]
 
   def __init__(self, field, ordinal):
diff --git a/mojo/public/bindings/pylib/parse/mojo_translate.py b/mojo/public/bindings/pylib/parse/mojo_translate.py
index 4ba0d14..1fe6acd 100755
--- a/mojo/public/bindings/pylib/parse/mojo_translate.py
+++ b/mojo/public/bindings/pylib/parse/mojo_translate.py
@@ -35,8 +35,6 @@ class MojomBuilder():
       return 'a:' + self.MapKind(kind[0:len(kind)-2])
     if kind in map_to_kind:
       return map_to_kind[kind]
-    if kind.find('.') == -1:
-      return 'x:' + self.mojom['namespace'] + '.' + kind
     return 'x:' + kind
 
 
diff --git a/mojo/public/bindings/tests/sample_import2.mojom b/mojo/public/bindings/tests/sample_import2.mojom
index 32be752..e5fd72d 100644
--- a/mojo/public/bindings/tests/sample_import2.mojom
+++ b/mojo/public/bindings/tests/sample_import2.mojom
@@ -20,7 +20,7 @@ struct Size {
 };
 
 struct Thing {
-  int32 shape = SHAPE_RECTANGLE;
+  imported.Shape shape = SHAPE_RECTANGLE;
   int32 color = COLOR_BLACK;
   Point location = {0, 0};
   Size size;
diff --git a/mojo/public/bindings/tests/sample_service.mojom b/mojo/public/bindings/tests/sample_service.mojom
index 979ec39..045c696 100644
--- a/mojo/public/bindings/tests/sample_service.mojom
+++ b/mojo/public/bindings/tests/sample_service.mojom
@@ -7,18 +7,17 @@ import "sample_import2.mojom"
 
 module sample {
 
-enum BarType {
-  BAR_VERTICAL = 1 << 0,
-  BAR_HORIZONTAL = (1 << 1) + 0,
-  BAR_BOTH = BAR_VERTICAL | BAR_HORIZONTAL,
-  BAR_INVALID
-};
-
 struct Bar {
+  enum Type {
+    TYPE_VERTICAL = 1 << 0,
+    TYPE_HORIZONTAL = (1 << 1) + 0,
+    TYPE_BOTH = TYPE_VERTICAL | TYPE_HORIZONTAL,
+    TYPE_INVALID
+  };
   uint8 alpha = (0x100 - 1) @0;
   uint8 beta @1;
   uint8 gamma @2;
-  int32 type @3;
+  Type type @3;
 };
 
 [RequiredFields=7]
@@ -61,7 +60,7 @@ interface Service {
     BAZ_REGULAR = 0,
     BAZ_EXTRA
   };
-  Frobinate(Foo foo @0, int32 baz @1, Port port @2) @0;
+  Frobinate(Foo foo @0, BazOptions baz @1, Port port @2) @0;
 };
 
 [Peer=Service]
diff --git a/mojo/public/bindings/tests/sample_service_unittest.cc b/mojo/public/bindings/tests/sample_service_unittest.cc
index 877f336..a252f1f 100644
--- a/mojo/public/bindings/tests/sample_service_unittest.cc
+++ b/mojo/public/bindings/tests/sample_service_unittest.cc
@@ -42,11 +42,11 @@ Foo MakeFoo() {
   bar.set_alpha(20);
   bar.set_beta(40);
   bar.set_gamma(60);
-  bar.set_type(BAR_VERTICAL);
+  bar.set_type(Bar::TYPE_VERTICAL);
 
   mojo::Array<Bar>::Builder extra_bars(3);
   for (size_t i = 0; i < extra_bars.size(); ++i) {
-    BarType type = i % 2 == 0 ? BAR_VERTICAL : BAR_HORIZONTAL;
+    Bar::Type type = i % 2 == 0 ? Bar::TYPE_VERTICAL : Bar::TYPE_HORIZONTAL;
     Bar::Builder bar;
     uint8_t base = static_cast<uint8_t>(i * 100);
     bar.set_alpha(base);
@@ -116,12 +116,12 @@ void CheckFoo(const Foo& foo) {
   EXPECT_EQ(20, foo.bar().alpha());
   EXPECT_EQ(40, foo.bar().beta());
   EXPECT_EQ(60, foo.bar().gamma());
-  EXPECT_EQ(BAR_VERTICAL, foo.bar().type());
+  EXPECT_EQ(Bar::TYPE_VERTICAL, foo.bar().type());
 
   EXPECT_EQ(3u, foo.extra_bars().size());
   for (size_t i = 0; i < foo.extra_bars().size(); i++) {
     uint8_t base = static_cast<uint8_t>(i * 100);
-    BarType type = i % 2 == 0 ? BAR_VERTICAL : BAR_HORIZONTAL;
+    Bar::Type type = i % 2 == 0 ? Bar::TYPE_VERTICAL : Bar::TYPE_HORIZONTAL;
     EXPECT_EQ(base, foo.extra_bars()[i].alpha()) << i;
     EXPECT_EQ(base + 20, foo.extra_bars()[i].beta()) << i;
     EXPECT_EQ(base + 40, foo.extra_bars()[i].gamma()) << i;
@@ -245,7 +245,7 @@ void DumpHex(const uint8_t* bytes, uint32_t num_bytes) {
 
 class ServiceImpl : public Service {
  public:
-  virtual void Frobinate(const Foo& foo, int32_t baz, ScopedPortHandle port)
+  virtual void Frobinate(const Foo& foo, BazOptions baz, ScopedPortHandle port)
       MOJO_OVERRIDE {
     // Users code goes here to handle the incoming Frobinate message.
 
-- 
cgit v1.1