[Xcb] [PATCH libxcb 2/6] generator: fix absname for fields with only iterator function

[Christian Linhart]

On 10/12/14 20:24, Ran Benita wrote:
> On Wed, Sep 03, 2014 at 01:22:37PM +0200, Christian Linhart wrote:
>> Fix _c_helper_absolute_name for fields which can only be
>> accessed with an iterator function.
>> These are lists with var-sized structs as members.
> 
> This doesn't sound right. Does the caller know how to handle each
> iterator? 
Yes, but the caller has to find that out by itself, 
which is not ideal because it could lead to inconsistencies.
Example is in patch "libxcb 6/6" of this patchset.

I have thought about that when implementing it.
An alternative would be to return meta-info in addition to the C-expression-string.
That meta-info can tell the caller that this is an iterator.

But that would change the caller interface of that function
and required a lot of changes.

Now I have a new idea which is cleaner:
We could use two functions.
1. One function which returns meta-info, and that function will be used by the sumof code.

2. A compatibility-wrapper function which calls the first function 
   but only returns the expression-string
   and throws an exception when this is an iterator, so it prevents
   that an iterator-access is inserted somewhere where it shouldn't.

> Where it is used? I see that the code is reached, but this
> it doesn't seem to reach the generated output anywhere (with this
> libxcb+proto patchset), as opposed to the accessors in the `else` branch.
It is used in sumof over lists with varsized members.

As far as I can remember we do not have such lists in the xml with all of my changes.
Therefore, it does not reach the generated output as you have observed.

But I accidentally assumed for some time that we have such lists, so I implemented support for it, and also tested it.
I thought that this feature might be useful some time in the future, so I have included it in that patchset.

But I am OK with skipping it.

If we skip this patch, we also have to skip patch "libxcb 6/6" of this patchset.

What do you think:
Skip it or improve it as outlined above?

Chris



> 
> So can we skip this patch? If not, why is it needed?
> 
> Ran
> 
>> ---
>>  src/c_client.py | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/c_client.py b/src/c_client.py
>> index 488f0e6..35c975f 100644
>> --- a/src/c_client.py
>> +++ b/src/c_client.py
>> @@ -464,15 +464,20 @@ def _c_helper_absolute_name(prefix, field=None):
>>                  ( field.type.is_list and not field.type.fixed_size() )
>>                  or
>>                  ( field.prev_varsized_field is not None
>>                    or not field.type.fixed_size()
>>                  )
>>          )
>>      ):
>> -        prefix_str = field.c_accessor_name + "(" + prefix_str_without_lastsep + ")";
>> +        if field.type.is_list and not field.type.member.fixed_size():
>> +            #only accessible by iterator
>> +            prefix_str = field.c_iterator_name + "(" + prefix_str_without_lastsep + ")";
>> +        else:
>> +            #only accessible by access function
>> +            prefix_str = field.c_accessor_name + "(" + prefix_str_without_lastsep + ")";
>>  
>>      return prefix_str
>>  # _c_absolute_name
>>  
>>  def _c_helper_field_mapping(complex_type, prefix, flat=False):
>>      """
>>      generate absolute names, based on prefix, for all fields starting from complex_type
>> -- 
>> 2.0.1
>>
>> _______________________________________________
>> Xcb mailing list
>> Xcb at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/xcb
>