[Xcb] [PATCH libxcb 1/6 V3] generator: fix absname for fields with only accessor function

[Christian Linhart]

Hi Ran,

Below are the answers to your questions.
I'll post the revised patch in the new thread.

Chris

On 11/01/14 11:06, Ran Benita wrote:
> On Thu, Oct 30, 2014 at 01:56:33PM +0100, Christian Linhart wrote:
[...]
>> +        last_sep = sep
>> +
>> +    if field is None:
>> +        # add separator for access to a yet unknown field
>> +        prefix_str += last_sep
>> +    else:
>> +        if _c_field_needs_accessor(field):
>> +            if _c_field_is_member_of_case_or_bitcase(field):
> 
> So, just to check if I get it right: case members *do* need accessors
> generated for them in general. So we cannot just change
> `_c_field_needs_accessor()` to return false for them, as that would
> mean the accessors won't be generated at all (per hunk below this one).
Yes, exactly.

> But using the accessor here is a headache and not strictly needed, so
> we access as struct member. OK.
Yes.

> Question: do you know the original reason for generating accessors for
> case members?
I guess they are needed for generating the deserialized struct.
I.e., the struct which we are accessing here directly.

> 
>> +                # case members are available in the deserialized struct,
>> +                # so there is no need to use the accessor function
>> +                # (also, their accessor function needs a different arglist
>> +                # so this would require special treatment here)
>> +                # Therefore: Access as struct member
>> +                prefix_str += last_sep + _cpp(field.field_name)
> 
> Maybe fold the `case_or_bitcase` condition into the one above it, such
> that it falls to the `else` and this line isn't repeated?
I had this in a previous internal version of the code,
but it did not show the logical decision process as nicely as this one.
So it'd be hard to comment it correctly, and harder to understand.

> 
>> +            else:
>> +                # Access with the accessor function
>> +                prefix_str = field.c_accessor_name + "(" + prefix_str + ")"
>> +        else:
>> +            # Access as struct member
>> +            prefix_str += last_sep + _cpp(field.field_name)
>> +
>>      return prefix_str
>>  # _c_absolute_name
>>  
[...]