Supporting more APIs with json_schema_compiler

tools/json_schema_compiler/cc_generator.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 from model import PropertyType
 import code
 import cpp_util
+import util_cc_helper
 
 class CCGenerator(object):
   """A .cc generator for a namespace.
   """
   def __init__(self, namespace, cpp_type_generator):
     self._cpp_type_generator = cpp_type_generator
     self._namespace = namespace
     self._target_namespace = (
         self._cpp_type_generator.GetCppNamespaceName(self._namespace))
+    self._util_cc_helper = (
+        util_cc_helper.UtilCCHelper(self._cpp_type_generator))
 
   def Generate(self):
     """Generates a code.Code object with the .cc for a single namespace.
     """
     c = code.Code()
     (c.Append(cpp_util.CHROMIUM_LICENSE)
       .Append()
       .Append(cpp_util.GENERATED_FILE_MESSAGE % self._namespace.source_file)
       .Append()
-      .Append('#include "tools/json_schema_compiler/util.h"')
+      .Append(self._util_cc_helper.GetIncludePath())
       .Append('#include "%s/%s.h"' %
-          (self._namespace.source_file_dir, self._target_namespace))
-      .Append()
-      .Concat(self._cpp_type_generator.GetCppNamespaceStart())
+          (self._namespace.source_file_dir, self._namespace.name))
+    )
+    includes = self._cpp_type_generator.GenerateIncludes()
+    if not includes.IsEmpty():
+      (c.Concat(includes)
+        .Append()
+      )
+
+    (c.Append()
+      .Concat(self._cpp_type_generator.GetRootNamespaceStart())
+      .Concat(self._cpp_type_generator.GetNamespaceStart())
       .Append()
       .Append('//')
       .Append('// Types')
       .Append('//')
       .Append()
     )
     for type_ in self._namespace.types.values():
-      (c.Concat(self._GenerateType(type_))
+      (c.Concat(self._GenerateType(type_.name, type_))
         .Append()
       )
     (c.Append('//')
       .Append('// Functions')
       .Append('//')
       .Append()
     )
     for function in self._namespace.functions.values():
       (c.Concat(self._GenerateFunction(function))
         .Append()
       )
-    (c.Concat(self._cpp_type_generator.GetCppNamespaceEnd())
+    (c.Concat(self._cpp_type_generator.GetNamespaceEnd())
+      .Concat(self._cpp_type_generator.GetRootNamespaceEnd())
       .Append()
     )
     # TODO(calamity): Events
     return c
 
-  def _GenerateType(self, type_):
+  def _GenerateType(self, cpp_namespace, type_, serializable=True):
     """Generates the function definitions for a type.
     """
+    classname = cpp_util.Classname(type_.name)
     c = code.Code()
 
-    (c.Append('%(classname)s::%(classname)s() {}')
-      .Append('%(classname)s::~%(classname)s() {}')
+    (c.Append('%(namespace)s::%(classname)s() {}')
+      .Append('%(namespace)s::~%(classname)s() {}')
       .Append()
     )
-    c.Substitute({'classname': type_.name})
+    c.Substitute({'classname': classname, 'namespace': cpp_namespace})
 
-    c.Concat(self._GenerateTypePopulate(type_))
+    c.Concat(self._GenerateTypePopulate(cpp_namespace, type_))
     c.Append()
-    # TODO(calamity): deal with non-serializable
-    c.Concat(self._GenerateTypeTovalue(type_))
+    if serializable:
+      c.Concat(self._GenerateTypeToValue(cpp_namespace, type_))
     c.Append()
 
     return c
 
-  def _GenerateTypePopulate(self, type_):
+  def _GenerateTypePopulate(self, cpp_namespace, type_):
     """Generates the function for populating a type given a pointer to it.
     """
+    classname = cpp_util.Classname(type_.name)
     c = code.Code()
     (c.Append('// static')
-      .Sblock('bool %(name)s::Populate(const Value& value, %(name)s* out) {')
+      .Sblock('bool %(namespace)s::Populate'
+              '(const Value& value, %(name)s* out) {')
         .Append('if (!value.IsType(Value::TYPE_DICTIONARY))')
         .Append('  return false;')
         .Append('const DictionaryValue* dict = '
                 'static_cast<const DictionaryValue*>(&value);')
         .Append()
     )
-    c.Substitute({'name': type_.name})
+    c.Substitute({'namespace': cpp_namespace, 'name': classname})
 
     # TODO(calamity): this handle single PropertyType.REF properties.
     # add ALL the types
     for prop in type_.properties.values():
-      sub = {'name': prop.name}
-      if prop.type_ == PropertyType.ARRAY:
-        if prop.item_type.type_ == PropertyType.REF:
-          if prop.optional:
-            (c.Append('if (!json_schema_compiler::util::'
-                      'GetOptionalTypes<%(type)s>(*dict,')
-              .Append('    "%(name)s", &out->%(name)s))')
-              .Append('  return false;')
-            )
-          else:
-            (c.Append('if (!json_schema_compiler::util::'
-                      'GetTypes<%(type)s>(*dict,')
-              .Append('    "%(name)s", &out->%(name)s))')
-              .Append('  return false;')
-            )
-          sub['type'] = self._cpp_type_generator.GetType(prop.item_type,
-              pad_for_generics=True)
-        elif prop.item_type.type_ == PropertyType.STRING:
-          if prop.optional:
-            (c.Append('if (!json_schema_compiler::util::GetOptionalStrings'
-                      '(*dict, "%(name)s", &out->%(name)s))')
-              .Append('  return false;')
-            )
-          else:
-            (c.Append('if (!json_schema_compiler::util::GetStrings'
-                      '(*dict, "%(name)s", &out->%(name)s))')
-              .Append('  return false;')
-            )
-        else:
-          raise NotImplementedError(prop.item_type.type_)
-      elif prop.type_.is_fundamental:
-        c.Append('if (!dict->%s)' %
-            cpp_util.GetFundamentalValue(prop, '&out->%s' % prop.name))
-        c.Append('  return false;')
-      else:
-        raise NotImplementedError(prop.type_)
-      c.Substitute(sub)
+      c.Concat(self._GenerateTypePopulateProperty(prop, 'dict', 'out'))
     (c.Append('return true;')
     .Eblock('}')

[not at google - send to devlin, 2012/02/08 05:02:08]

indent 2 spaces. I can see why you might have done it this way (so that it lines
up with the C++ indentation) but it looks weird.

[calamity, 2012/02/08 07:01:18]

Done.

     )
     return c
 
-  def _GenerateTypeTovalue(self, type_):
+  def _GenerateTypePopulateProperty(self, prop, src, dst):

[not at google - send to devlin, 2012/02/08 05:02:08]

As discussed, most of this code will be replaced with the choices implementation
in GeneratePopulateChoices; you'll just need to factor it a bit differently to
deal with a Value*. Then,
 - this method will get the value from src[prop.name]
 - GeneratePopulateChoices will get it from src[index]
or something like that.

[calamity, 2012/02/08 07:01:18]

Done.

+    """Generate the code to populate a single property.
+
+    src: DictionaryValue*
+    dst: Type*
+    """
+    c = code.Code()
+    dst_member = dst + '->' + prop.unix_name
+    if prop.type_ == PropertyType.ARRAY:
+      (c.Append('if (!%s)' %
+          self._util_cc_helper.GetArray(prop, src, prop.name, dst_member))
+        .Append('  return false;')
+      )
+    elif prop.type_ == PropertyType.CHOICES:
+      if prop.optional:
+        c.Append('%(dst)s_type = %(enum)s::NONE')
+      for choice in self._cpp_type_generator.ExpandedChoicesInParams([prop]):
+        dst_member = dst + '->' + choice.unix_name
+        if choice.type_ == PropertyType.ARRAY:
+          (c.Append('if (!%s)' %
+              self._util_cc_helper.GetArray(
+                  choice, src, choice.name,dst_member))
+            .Append('  return false;')
+          )
+        else:
+          (c.Sblock('{')
+              .Append('%(type)s %(name)s_temp;')
+              .Append('if (%s) {' % self._GeneratePopulatePropertyFunctionCall(
+                  choice, src, '&%s_temp' % choice.unix_name))
+              .Append('  out->%(name)s.reset(new %(type)s(%(name)s_temp));')
+              .Append('  %(dst)s = %(enum)s;')
+              .Append('}')
+            .Eblock('}')
+          )
+        c.Substitute({
+            'src': src,
+            'dst': dst + '->%s_type' % prop.unix_name,
+            'name': choice.unix_name,
+            'enum': self._cpp_type_generator.GetChoiceEnum(prop, choice.type_),
+            'type': self._cpp_type_generator.GetType(choice)
+        })
+    else:
+      if prop.optional:
+        if prop.type_.is_fundamental:
+          (c.Sblock('{')
+              .Append('%(type)s %(name)s_temp;')
+              .Append('if (%s)' % self._GeneratePopulatePropertyFunctionCall(
+                  prop, src, '&%s_temp' % prop.unix_name))
+              .Append('  out->%(name)s.reset(new %(type)s(%(name)s_temp));')
+            .Eblock('}')
+          )
+        else:
+          raise NotImplementedError('Optional %s not implemented' % prop.type_)
+      else:
+        (c.Append('if (!%s)' %
+            self._GeneratePopulatePropertyFunctionCall(
+                prop, src, '&' + dst_member))
+          .Append('  return false;')
+        )
+      c.Substitute({
+              'src': src, 'dst': dst_member, 'name': prop.unix_name,
+              'type': self._cpp_type_generator.GetType(prop)

[not at google - send to devlin, 2012/02/08 05:02:08]

too much indentation, and spread out each member on its own line. hard to read
this way.

[calamity, 2012/02/08 07:01:18]

Done.

+      })
+    return c

[not at google - send to devlin, 2012/02/08 05:02:08]

Can delay substituting src until the end?

c.Substitute({
  'src': src
})

then remove it above.

[calamity, 2012/02/08 07:01:18]

Done.

[not at google - send to devlin, 2012/02/09 02:49:36]

Not done?

Also you have the same thing above now, substituting "src" in.

+
+  def _GeneratePopulatePropertyFunctionCall(self, prop, src, dst):
+    """Generates the function call that populates the given property.
+
+    src: DictionaryValue*
+    dst: Property* or scoped_ptr<Property>
+    """
+    if prop.type_.is_fundamental:
+      populate_line = cpp_util.GetFundamentalValue(
+          prop, src, prop.name, dst)
+    elif prop.type_ in (PropertyType.REF, PropertyType.OBJECT):
+      populate_line = '%(type)s::Populate(*%(src)s, %(dst)s)'
+    else:
+      raise NotImplementedError('%s populate is not implemented' %
+          prop.type_)
+    return populate_line
+
+
+  def _GenerateTypeToValue(self, cpp_namespace, type_):
     """Generates a function that serializes the type into a |DictionaryValue|.
     """
     c = code.Code()
-    (c.Sblock('DictionaryValue* %s::ToValue() const {' % type_.name)
-      .Append('DictionaryValue* value = new DictionaryValue();')
-      .Append()
+    (c.Sblock('DictionaryValue* %s::ToValue() const {' % cpp_namespace)
+        .Append('DictionaryValue* value = new DictionaryValue();')
+        .Append()
     )
-    name = type_.name.lower()
     for prop in type_.properties.values():
-      prop_name = name + '_' + prop.name if name else prop.name
-      this_var =  prop.name
-      c.Concat(self._CreateValueFromProperty(prop_name, prop, this_var))
+      c.Concat(self._CreateValueFromProperty(prop, prop.unix_name, 'value'))
     (c.Append()
       .Append('return value;')
       .Eblock('}')
     )
     return c
 
-  # TODO(calamity): object and choices prop types
-  def _CreateValueFromProperty(self, name, prop, var):
+  def _CreateValueFromProperty(self, prop, var, dst):
     """Generates code to serialize a single property in a type.
+
+    prop: Property to create from
+    var: variable with value to create from
     """
     c = code.Code()
-    if prop.type_.is_fundamental:
-      c.Append('Value* %s_value = %s;' %
-          (name, cpp_util.CreateFundamentalValue(prop, var)))
-    elif prop.type_ == PropertyType.ARRAY:
-      if prop.item_type.type_ == PropertyType.STRING:
-        if prop.optional:
-          c.Append('json_schema_compiler::util::'
-                   'SetOptionalStrings(%s, "%s", value);' % (var, prop.name))
-        else:
-          c.Append('json_schema_compiler::util::'
-                   'SetStrings(%s, "%s", value);' % (var, prop.name))
-      else:
-        item_name = name + '_single'
-        (c.Append('ListValue* %(name)s_value = new ListValue();')
-          .Append('for (%(it_type)s::iterator it = %(var)s->begin();')
-          .Sblock('    it != %(var)s->end(); ++it) {')
-          .Concat(self._CreateValueFromProperty(item_name, prop.item_type,
-                  '*it'))
-          .Append('%(name)s_value->Append(%(prop_val)s_value);')
-          .Eblock('}')
-        )
-        c.Substitute(
-            {'it_type': self._cpp_type_generator.GetType(prop),
-             'name': name, 'var': var, 'prop_val': item_name})
-    elif prop.type_ == PropertyType.REF:
-      c.Append('Value* %s_value = %s.ToValue();' % (name, var))
+    if prop.optional:
+      c.Sblock('if (%s.get())' % var)
+    if prop.type_ == PropertyType.ARRAY:
+      c.Append('%s;' % self._util_cc_helper.SetArray(prop, var, prop.name, dst))
     else:
-      raise NotImplementedError
+      c.Append('%s->SetWithoutPathExpansion("%s", %s);' %
+          (dst, prop.name, cpp_util.CreateValueFromSingleProperty(prop, var)))
     return c
 
   def _GenerateFunction(self, function):
     """Generates the definitions for function structs.
     """
-    classname = cpp_util.CppName(function.name)
+    classname = cpp_util.Classname(function.name)
     c = code.Code()
 
     # Params::Populate function
     if function.params:
+      for param in function.params:
+        if param.type_ == PropertyType.OBJECT:
+          param_namespace = '%s::Params::%s' % (classname,
+              cpp_util.Classname(param.name))
+          c.Concat(
+              self._GenerateType(param_namespace, param, serializable=False))
+          c.Append()
       (c.Append('%(name)s::Params::Params() {}')
         .Append('%(name)s::Params::~Params() {}')
         .Append()
         .Concat(self._GenerateFunctionParamsCreate(function))
         .Append()
       )
 
     # Result::Create function
     c.Concat(self._GenerateFunctionResultCreate(function))
 
     c.Substitute({'name': classname})
 
     return c
 
+  def _GenerateParamsCheck(self, function, var):
+    """Generates a check for the correct number of arguments when creating Params.
+    """
+    c = code.Code()
+    num_optional = 0
+    num_required = 0
+    for param in function.params:
+      if param.optional:
+        num_optional += 1
+      else:
+        num_required += 1
+    if num_optional:
+      (c.Append('if (%(var)s.GetSize() > %(total)d)')
+        .Append('  return scoped_ptr<Params>();')
+      )
+    if num_required:
+      (c.Append('if (%(var)s.GetSize() < %(req)d)')
+        .Append('  return scoped_ptr<Params>();')
+      )
+    c.Substitute({
+        'var': var,
+        'req': num_required,
+        'total': num_optional + num_required,
+    })
+    return c

[not at google - send to devlin, 2012/02/08 05:02:08]

y u no write it like I wrote?

I can see why you did the num_optional thing, but those checks aren't necessary
(the "if num_{optional,required}:") because they cause a weakening of the
checks. Looking at permissions.cc for example, it used to be

if (args.GetSize() != 1)
  return NULL;

but now it's

if (args.GetSize() < 1)
  return NULL;

which isn't as strong. Instead, we want like (generated C++)

if (args.GetSize() < 1)
  return NULL;
if (args.GetSize() > 1)
  return NULL;

or just

if (args.GetSize() < 1 || args.GetSize() > 1)
  return NULL;

which is the same as != 1.  In fact, you could have like (Python)

if num_required == params.length:
  c.Append('if (%(var)s.GetSize() != %(total)d')
else:
  c.Append('if (%(var)s.GetSize() < %(required)d'
           ' || %(var)s.GetSize() > %(total)d')
c.Append('  return scoped_ptr<Params>();')

[calamity, 2012/02/08 07:01:18]

Wups, my bad. Note that you can't do args.GetSize() < 0 because C++ herps a derp
about it always being true.

[not at google - send to devlin, 2012/02/09 02:49:36]

Hah.

+
   def _GenerateFunctionParamsCreate(self, function):
     """Generate function to create an instance of Params given a pointer.
     """
-    classname = cpp_util.CppName(function.name)
+    classname = cpp_util.Classname(function.name)
     c = code.Code()
     (c.Append('// static')
       .Sblock('scoped_ptr<%(classname)s::Params> %(classname)s::Params::Create'
                '(const ListValue& args) {')
-      .Append('if (args.GetSize() != %d)' % len(function.params))
-      .Append('  return scoped_ptr<Params>();')
-      .Append()
+      .Concat(self._GenerateParamsCheck(function, 'args'))
       .Append('scoped_ptr<Params> params(new Params());')
     )
     c.Substitute({'classname': classname})
 
-    # TODO(calamity): generalize, needs to move to function to do populates for
-    # wider variety of args
     for i, param in enumerate(function.params):
-      sub = {'name': param.name, 'pos': i}
+      dst = 'params->' + param.unix_name
+      if param.optional:
+        return_line = '  return params.Pass();'
+      else:
+        return_line = '  return scoped_ptr<Params>();'
       c.Append()
-      # TODO(calamity): Make valid for not just objects
-      c.Append('DictionaryValue* %(name)s_param = NULL;')
-      c.Append('if (!args.GetDictionary(%(pos)d, &%(name)s_param))')
-      c.Append('  return scoped_ptr<Params>();')
-      if param.type_ == PropertyType.REF:
-        c.Append('if (!%(ctype)s::Populate(*%(name)s_param, '
-                 '&params->%(name)s))')
-        c.Append('  return scoped_ptr<Params>();')
-        sub['ctype'] = self._cpp_type_generator.GetType(param)
-      elif param.type_.is_fundamental:
-        raise NotImplementedError('Fundamental types are unimplemented')
-      elif param.type_ == PropertyType.OBJECT:
-        c.Append('if (!%(ctype)s::Populate(*%(name)s_param, '
-                 '&params->%(name)s))')
-        c.Append('  return scoped_ptr<Params>();')
-        sub['ctype'] = self._cpp_type_generator.GetType(param)
+      param_var = param.unix_name + '_param'
+      if param.type_ == PropertyType.ARRAY:
+        (c.Append('ListValue* %(var)s = NULL;')
+          .Append('if (!args.GetList(%(index)d, &%(var)s))')
+          .Append(return_line)
+          .Append('if (!%s)' % self._util_cc_helper.GetArrayFromList(
+              param, param_var, dst))
+          .Append(return_line)
+        )
+        c.Substitute({'var': param_var, 'index': i})
       elif param.type_ == PropertyType.CHOICES:
-        raise NotImplementedError('Choices is unimplemented')
+        c.Concat(self._GeneratePopulateChoices(param, 'args', i, return_line))
       else:
-        raise NotImplementedError(param.type_)
-      c.Substitute(sub)
-    c.Append()
-    c.Append('return params.Pass();')
+        if param.optional:
+          dst = dst + '.get()'
+        else:
+          dst = '&' + dst
+        if param.type_ in (PropertyType.REF, PropertyType.OBJECT):
+          (c.Append('DictionaryValue* %s = NULL;' % param_var)
+            .Append('if (!args.GetDictionary(%d, &%s))' % (i, param_var))
+            .Append(return_line)
+          )
+          if param.optional:
+            c.Append('params->%s.reset(new %s());' %
+                (param.unix_name, cpp_util.Classname(param.name)))
+          (c.Append('if (!%(ctype)s::Populate(*%(var)s, %(dst)s))' % {
+              'var': param_var, 'dst': dst,
+              'ctype': self._cpp_type_generator.GetType(param)
+            })
+            .Append(return_line)
+          )
+        elif param.type_.is_fundamental:
+          (c.Append(
+              'if (!%s)' % cpp_util.GetValueFromList(param, 'args', i, dst))
+            .Append(return_line)
+          )
+    (c.Append()
+      .Append('return params.Pass();')
+    .Eblock('}')
+    )
+
+    return c
+
+  def _GeneratePopulateChoices(self, param, src, index, return_line):

[not at google - send to devlin, 2012/02/08 05:02:08]

confused by the use of the word "populate" here but it's actually used as part
of the Params::Create() method, not Populate?

[calamity, 2012/02/08 07:01:18]

Now it populates. Woo.

+    """Generates the code to populate a PropertyType.CHOICES parameter.
+
+    src: ListValue*
+    index: index of the ListValue the parameter is in
+    return_line: return line to be used if the choices parameter is not present
+    """

[not at google - send to devlin, 2012/02/08 05:02:08]

as discussed, refactoring to do here.

[calamity, 2012/02/08 07:01:18]

Done.

+    c = code.Code()
+    type_var = 'params->%s_type' % param.unix_name
+    if param.optional:
+      type_var = '*' + type_var
+    c.Append('%s = %s;' %
+        (type_var, self._cpp_type_generator.GetChoiceEnum(param, None)))
+    value_var = param.unix_name + '_value'

[not at google - send to devlin, 2012/02/08 05:02:08]

Refactoring being making this method accept the name of value_var rather than a
ListValue and its index.

[calamity, 2012/02/08 07:01:18]

Done.

+    c.Append('Value* %(value)s = NULL;')
+    c.Append('if (!%(src)s.Get(%(index)s, &%(value)s))')
+    c.Append(return_line)

[not at google - send to devlin, 2012/02/08 05:02:08]

This means that optional parameters can only appear as the last parameter in
function calls. You should check that the JSON behaves that way, and if it does:
assert this in the model, and add a comment here.

If it doesn't, then you'll need to deal with it by not returning from the
function, but putting everything in an "if" block (you can figure it out I'm
sure).

Actually, if it's not too hard, you can save yourself and other people the
cognitive effort of reading the comment(s) and reason for the assertion(s) and
just implement it in the more general way to start off with.

[calamity, 2012/02/08 07:01:18]

Couldn't figure a case where requires args happen after optional args. Can't
assert because there are 'optional' first args but they need to be specified as
null/undefined.

+    c.Sblock('switch (%(value)s->GetType()) {')
+    c.Substitute({'src': src, 'index': index, 'value': value_var})

[not at google - send to devlin, 2012/02/08 05:02:08]

IMO try to put substitutions at the end of these code blocks. When you
substitute early like this, I wonder if it's because the values change later
on... and here they don't.

[calamity, 2012/02/08 07:01:18]

Done. I'm wary of someone else changing a value in the middle of a huge code
block and breaking things for the substitute at the bottom.

+    for choice in self._cpp_type_generator.ExpandedChoicesInParams([param]):
+      if choice.type_.is_fundamental:
+        c.Sblock('case %s: {' % {
+            PropertyType.STRING: 'Value::TYPE_STRING',
+            PropertyType.INTEGER: 'Value::TYPE_INTEGER',
+            PropertyType.BOOLEAN: 'Value::TYPE_BOOLEAN',
+            PropertyType.DOUBLE: 'Value::TYPE_DOUBLE'
+        }[choice.type_])
+
+        dst = 'params->%s.get()' % choice.unix_name
+        (c.Append('  %(type)s = %(enum)s;')

[not at google - send to devlin, 2012/02/08 05:02:08]

why the extra spaces before %(type)s?

[calamity, 2012/02/08 07:01:18]

Done.

+          .Append('%s;' %
+              cpp_util.GetAsFundamentalValue(choice, value_var, dst))
+          .Append('break;')
+        .Eblock('}')
+        )
+      elif choice.type_ == PropertyType.ARRAY:
+        dst = 'params->%s' % choice.unix_name
+        (c.Sblock('case Value::TYPE_LIST: {')
+            .Append('%(type)s = %(enum)s;')
+            .Append('if (!%s)' % self._util_cc_helper.GetArrayFromList(
+                choice, 'static_cast<ListValue*>(%s)' % value_var, dst))
+            .Append('  return scoped_ptr<Params>();')
+            .Append('break;')
+          .Eblock('}')
+        )
+      else:
+        raise NotImplementedError(choice.type_)
+      c.Substitute({'type': type_var,
+          'enum': self._cpp_type_generator.GetChoiceEnum(param, choice.type_)})

[not at google - send to devlin, 2012/02/08 05:02:08]

I don't know whether enum refers to the type or the value by reading the code. I
think it's value, so call enum_value.

[calamity, 2012/02/08 07:01:18]

Done.

+    if not param.optional:
+      (c.Append('default:')
+        .Append('  return scoped_ptr<Params>();')
+      )
     c.Eblock('}')
-
     return c
 
   def _GenerateFunctionResultCreate(self, function):
     """Generate function to create a Result given the return value.
     """
-    classname = cpp_util.CppName(function.name)
+    classname = cpp_util.Classname(function.name)
     c = code.Code()
     c.Append('// static')
-    param = function.callback.param
-    arg = ''
-    if param:
-      if param.type_ == PropertyType.REF:
-        arg = 'const %(type)s& %(name)s'
-      else:
-        arg = 'const %(type)s %(name)s'
-      arg = arg % {
-          'type': self._cpp_type_generator.GetType(param),
-          'name': param.name
-      }
-    c.Sblock('Value* %(classname)s::Result::Create(%(arg)s) {')
-    sub = {'classname': classname, 'arg': arg}
-    # TODO(calamity): Choices
-    if not param:
-      c.Append('return Value::CreateNullValue();')
+    params = function.callback.params
+
+    if not params:
+      (c.Append('Value* %s::Result::Create() {' % classname)
+        .Append('  return Value::CreateNullValue();')
+        .Append('}')
+      )
     else:
-      sub['argname'] = param.name
-      if param.type_.is_fundamental:
-        c.Append('return %s;' %
-            cpp_util.CreateFundamentalValue(param, param.name))
-      elif param.type_ == PropertyType.REF:
-        c.Append('return %(argname)s.ToValue();')
-      elif param.type_ == PropertyType.OBJECT:
-        raise NotImplementedError('Objects not implemented')
-      elif param.type_ == PropertyType.ARRAY:
-        raise NotImplementedError('Arrays not implemented')
-      else:
-        raise NotImplementedError(param.type_)
-    c.Substitute(sub)
-    c.Eblock('}')
+      # If there is a single parameter, this is straightforward. However, if
+      # the callback parameter is of 'choices', this generates a Create method
+      # for each choice. This works because only 1 choice can be returned at a
+      # time.
+      for param in self._cpp_type_generator.ExpandedChoicesInParams(params):
+        if param.type_ == PropertyType.REF:
+          arg = 'const %(type)s& %(name)s'
+        else:
+          arg = 'const %(type)s %(name)s'
+        arg %= {
+          'type': self._cpp_type_generator.GetType(param, wrap_optional=True),
+          'name': param.unix_name,
+        }
+        if param and param.description:
+          c.Comment(param.description)
+        c.Sblock('Value* %(classname)s::Result::Create(%(arg)s) {')
+        if param.type_ == PropertyType.ARRAY:
+          (c.Append('ListValue* value = new ListValue();')
+            .Append('%s;' % self._util_cc_helper.SetArrayToList(param,
+                param.unix_name, 'value'))
+            .Append('return value;')
+          )
+        else:
+          c.Append('return %s;' % cpp_util.CreateValueFromSingleProperty(param,
+              param.unix_name))
+        c.Substitute({'classname': classname, 'arg': arg})
+        c.Eblock('}')
+
     return c