[Xcb] [PATCH libxcb 4/6] generator: sumof: support any type, generate explicit code

[Christian Linhart]

Hi Ran,

Below are my answers.
The revised patch will be in the new thread.

On 11/01/14 11:40, Ran Benita wrote:
[...]
> 
>>          c_length_func = _c_accessor_get_expr(field.type.expr, field_mapping)
>> -        return 'xcb_sumof(%s, %s)' % (list_name, c_length_func)
>> +        #create explicit code for computing the sum.
>> +        #This works for all C-types which can be added to int64_t with +=
> 
> I don't mind the int64_t approach, I'd probably do the same. But I'm
> guessing the real type is too hard to come by?
As far as I can tell, there is no definition anywhere 
where the real type can be derived from.
It is definitely not the type of the components of the sum, as we
have cases where 8-bit values are summed and the result may be greater than 256.

The alternative to int64_t would probably be to specify the type
as an xml-attribute to the <sumof>-tag.
This would make the generator more complicated however.
For now int64_t is sufficient.
If we need other types, then we can always add
an optional type-attribute to the <sumof>-tag later.

> 
>> +        _c_pre.start()
>> +        lengthvar = _c_pre.get_tempvarname()
>> +        loopvar = _c_pre.get_tempvarname()
>> +        sumvar = _c_pre.get_tempvarname()
>> +        listvar = _c_pre.get_tempvarname()
>> +        _c_pre.tempvar( "int %s; /* sumof length */", lengthvar )
>> +        _c_pre.tempvar( "int %s; /* sumof loop counter */", loopvar )
>> +        _c_pre.tempvar( "int64_t %s; /* sumof sum */", sumvar )
> 
> Have you considered, instead of random fresh var + comments,
> incorporating a descriptive name to the tempvarname?
Yes I have thought about that after having written and tested the code.
It can rather easily be changed later by modifying get_tempvarname
to accept a string that will be incorporated into the varname.

> The generated code as-is is readable enough though.
OK, fine.

> 
>> +        _c_pre.tempvar(
>> +             "const %s* %s; /* sumof list ptr */", field.c_field_type, listvar )
> 
> No need to line break here!
What's the max allowed line length?

Chris