Simplify Go's Mojom Type Generator

mojo/public/tools/bindings/generators/mojom_go_generator.py

Index: mojo/public/tools/bindings/generators/mojom_go_generator.py
diff --git a/mojo/public/tools/bindings/generators/mojom_go_generator.py b/mojo/public/tools/bindings/generators/mojom_go_generator.py
index d0687784ddd7f78f81254cf733253c6e0c4e84ff..cde3891dc17f8a665fd398aefe7565c38f3d724d 100644
--- a/mojo/public/tools/bindings/generators/mojom_go_generator.py
+++ b/mojo/public/tools/bindings/generators/mojom_go_generator.py
@@ -14,6 +14,8 @@ import mojom.generate.generator as generator
 import mojom.generate.module as mojom
 import mojom.generate.pack as pack
 
+GENERATOR_PREFIX = 'go'
+
 class KindInfo(object):
   def __init__(self, go_type, encode_suffix, decode_suffix, bit_size):
     self.go_type = go_type
@@ -307,36 +309,20 @@ def AddImport(imports, mojom_imports, module, element):
   imports[path] = name
   mojom_imports[path] = name
 
-# The identifier cache is used by the Type generator to determine if a type has
-# already been generated or not. This prevents over-generation of the same type
-# when it is referred to in multiple ways.
-identifier_cache = {}
 def GetIdentifier(kind):
-  # Use the kind's module to determine the package name.
+  # Use the kind's module to determine the package and name.
+  # Note: InterfaceRequestKind's should use the Interface inside them.

[azani, 2015/12/10 23:16:18]

The type is just InterfaceRequest.

[alexfandrianto, 2015/12/11 01:26:57]

Done.

   if hasattr(kind, 'module'):
     package = GetPackageName(kind.module)
+    name = kind.name
   elif mojom.IsInterfaceRequestKind(kind):
     package = GetPackageName(kind.kind.module)
+    name = kind.kind.name
   else:
+    # This is for a type that should not be stored. Return an empty identifier.
     return ''
 
-  # Most kinds have a name, but those that don't should rely on their spec.
-  # Since spec can have : and ? characters, these must be replaced. Since ? is
-  # replaced with '', the caller must keep track of optionality on its own.
-  name_or_spec = (kind.name if hasattr(kind, 'name') else kind.spec)
-  package_unique = name_or_spec.replace(':', '_').replace('?', '')
-  return '%s_%s' % (package, package_unique)
-
-def StoreIdentifier(identifier, cache_name):
-  if not cache_name in identifier_cache:
-    identifier_cache[cache_name] = {}
-  identifier_cache[cache_name][identifier] = True
-  return ''
-
-def CheckIdentifier(identifier, cache_name):
-  if not cache_name in identifier_cache:
-    identifier_cache[cache_name] = {}
-  return identifier in identifier_cache[cache_name]
+  return '%s_%s' % (package, name)
 
 # Get the mojom type's identifier suffix.
 def GetMojomTypeIdentifier(kind):
@@ -353,8 +339,6 @@ class Generator(generator.Generator):
     'expression_to_text': ExpressionToText,
     'has_response': lambda method: method.response_parameters is not None,
     'identifier': GetIdentifier,
-    'identifier_check': CheckIdentifier,
-    'identifier_store': StoreIdentifier,
     'is_array': mojom.IsArrayKind,
     'is_enum': mojom.IsEnumKind,
     'is_handle': mojom.IsAnyHandleKind,
@@ -377,9 +361,8 @@ class Generator(generator.Generator):
     'tab_indent': lambda s, size = 1: ('\n' + '\t' * size).join(s.splitlines())
   }
 
-  # TODO: This value should be settable via arguments. If False, then mojom type
-  # information will not be generated.
-  should_gen_mojom_types = True
+  # If set to True, then mojom type information will be generated.
+  should_gen_mojom_types = False
 
   def GetParameters(self):
     package = GetPackageName(self.module)
@@ -403,6 +386,9 @@ class Generator(generator.Generator):
     return self.GetParameters()
 
   def GenerateFiles(self, args):
+    if "--go_gen_types" in args:

[azani, 2015/12/10 23:16:18]

Let's have a single flag control type generation in all languages for which it
is implemented.

[alexfandrianto, 2015/12/11 01:26:57]

I very much agree, but I am having issues with lines 141 to 144 of the
mojom_bindings_generator_v1.py, which is why it is called "--go_gen_types"
instead of "--gen_types".

- only remaining_args are allowed to be sent over to the generator.
- of those, the args must begin with the generator's prefix
- if there is no prefix, then no args are sent over

In other words, if we leave that code there as-is, we cannot have a single
common argument sent to each language's generator. I've found a workaround and
can create an issue for this, but what would you recommend?

[rudominer, 2015/12/14 21:29:57]

Let's just leave it as you have it for now. We can fix it after we get rid of
v1.

[azani, 2015/12/14 22:20:01]

I just chatted with Mitch and we came up with a solution. What you can do is
after line 206 in mojom_bindings_generator_v1.py add an argument
generate-type-info. Then, on line 144, check args.generate_type_info and if it
is present, add it to filtered_args.

Then, do the same thing in run_code_generators.py. Add the arg at line 41 and
check it at line 165.

[alexfandrianto, 2015/12/15 20:53:40]

Thanks, that was very helpful. I added --generate_type_info and
--no_generate_type_info with default to true (for compilation purposes.

+        self.should_gen_mojom_types = True
+
     self.Write(self.GenerateSource(), os.path.join("go", "src",
         GetPackagePath(self.module), "%s.go" % self.module.name))