Re: [Qemu-devel] [PATCH v3 08/14] qapi: Make c_type() consistently convert qapi names

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4421 bytes --]

On 05/07/2015 01:39 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Continuing the string of cleanups for supporting downstream names
>> containing '.', this patch focuses on ensuring c_type() can
>> handle a downstream name.  This patch alone does not fix the
>> places where generator output should be calling this function
>> but was open-coding things instead, but it gets us a step closer.
>>
>> Note that we generate a List type for our builtins; the code has
>> to make sure that ['int'] maps to 'intList' (and not 'q_intList'),
>> and that a member with type 'int' still maps to the C type 'int';
>> on the other hand, a member with a name of 'int' will still map
>> to 'q_int' when going through c_name().  This patch had to teach
> 
> has to teach
> 
>> type_name() to special-case builtins, since it is used by
>> c_list_type() which in turn feeds c_type().
>>

>>
>> +# This function is used for converting the type of 'member':'name' into a
>> +# substring for use in C pointer types or function names.
> 
> Likewise:
> 
> # Map type @name to its C typedef name.
> # <Explanation of intended use goes here>
> 
> Consider rename parameter @name, because it can either be a name string,
> or a list containing a name string.  Same for the other functions.
> Perhaps in a separate patch for easier review.

Sure, will split out a second patch, and post v4 shortly.  c_name() only
operates on strings, but type_name() and c_type() do indeed operate on
more than just strings.

> 
>>  def type_name(name):
>>      if type(name) == list:
>>          return c_list_type(name[0])
>> -    return name
>> +    if name in builtin_types.keys():
>> +        return name
>> +    return c_name(name)
> 
> Together with the change to c_list_type(), this changes type_name() as
> follows:
> 
> * Name FOO becomes c_name(FOO) instead of FOO, except when FOO is the
>   name of a built-in type.  Bug fix when FOO contains '.' or '-' or is
>   a ticklish identifier other than a built-in type.
> 
> * List of FOO becomes c_name(FOO) + "List" instead of FOOList.  Bug fix
>   when FOO contains '.' or '-'.  Not a bug fix when ticklish FOO becomes
>   q_FOO, but improves consistency with the element type's C name then.
> 
> Correct?

Yes. 'unix'->"q_unix" but ['unix']->"unixList" was inconsistent, so it
is now "q_unixList".  But ['int'] must map to "intList" and not
"q_intList", because we have implementations for intList but do not need
generated code for the builtin type int.


> 
> # Map type @name to its C type expression.
> # If @is_param, const-qualify the string type.
> # <Explanation of intended use goes here>
> # A special suffix...
> 
>> @@ -888,13 +897,13 @@ def c_type(name, is_param=False):
>>      elif type(name) == list:
>>          return '%s *%s' % (c_list_type(name[0]), eatspace)
>>      elif is_enum(name):
>> -        return name
>> +        return c_name(name)
>>      elif name == None or len(name) == 0:
>>          return 'void'
> 
> Aside: len(name) == 0 is a lame way to test name == "".
> Aside^2: I wonder whether we ever pass that.

I'll find out :)

> 
>>      elif name in events:
>>          return '%sEvent *%s' % (camel_case(name), eatspace)
>>      else:
>> -        return '%s *%s' % (name, eatspace)
>> +        return '%s *%s' % (c_name(name), eatspace)
> 
> I figure the else is for complex types.  If that's correct, we should
> perhaps add a comment.

Yes, at this point, all that is left is complex types.

> 
>>
>>  def is_c_ptr(name):
>>      suffix = "*" + eatspace
> 
> Together with the change to c_list_type(), this changes c_type() as
> follows:
> 
> * Enum FOO becomes c_name(FOO) instead of FOO.  Bug fix when FOO
>   contains '.' or '-' or is a ticklish identifier.
> 
> * Complex type FOO becomes c_name(FOO) + "*" instead of FOO *.  Bug fix
>   when FOO contains '.' or '-' or is a ticklish identifier.
> 
> * List of FOO becomes c_name(FOO) + "List *" instead of FOOList *.  Bug
>   fix when FOO contains '.' or '-'.  Not a bug fix when ticklish FOO
>   becomes q_FOO, but improves consistency with the element type's C name
>   then.
> 
> Correct?

Yes. I can try and fold that into the commit message.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]