Eric Blake <ebl...@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().
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> scripts/qapi.py | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index b9822c6..a1dfc85 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -769,6 +769,9 @@ def c_enum_const(type_name, const_name):
>
> c_name_trans = string.maketrans('.-', '__')
>
> +# This function is used for converting the name portion of 'name':'type'
> +# into a valid C name, for use as a struct member or substring of a
> +# function name.
Do we use it only for "the name portion of 'name':'type'"?
I'd prefer a more conventional function comment, like
# Map @name to a valid C identifier.
# If @protect, avoid returning certain ticklish identifiers like C
# keywords by prepending "q_".
# <Explanation of intended use goes here>
> def c_name(name, protect=True):
> # ANSI X3J11/88-090, 3.1.1
> c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
> @@ -800,13 +803,18 @@ def c_name(name, protect=True):
> return "q_" + name
> return name.translate(c_name_trans)
>
> +# This function is used for computing the C type of a 'member':['name']
> array.
Likewise:
# Map type @name to the C typedef name for the list of this type.
> def c_list_type(name):
> - return '%sList' % name
> + return type_name(name) + 'List'
>
> +# 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.
> 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?
>
> def add_name(name, info, meta, implicit = False):
> global all_names
> @@ -864,6 +872,7 @@ def is_enum(name):
>
> eatspace = '\033EATSPACE.'
>
> +# This function is used for computing the full C type of 'member':'name'.
> # A special suffix is added in c_type() for pointer types, and it's
> # stripped in mcgen(). So please notice this when you check the return
> # value of c_type() outside mcgen().
Likewise:
# 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.
> 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.
>
> 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?
