[Xcb] [PATCH libxcb 3/6 V2] generator: expressions can generate pre-code
Ran Benita
ran234 at gmail.com
Sat Nov 1 03:28:28 PDT 2014
On Wed, Sep 10, 2014 at 12:57:34PM +0200, Christian Linhart wrote:
> This patch provides a mechanism for generating
> preparatory code for expressions.
>
> This is e.g. necessary when an expression needs computations
> which cannot be done in a C-Expression, like for-loops.
>
> This will be used for sumof expressions but may be useful
> elsewhere.
>
> V2: adapt to changes in previous patches
I haven't given this approach much consideration, so can't offer
detailed review. But this looks clean and understandable and the sumof
user is nicely self contained. So with some style fixes below you can
add my R-b (I won't comment on style issues already fixed in git).
> ---
> src/c_client.py | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 115 insertions(+), 4 deletions(-)
>
> diff --git a/src/c_client.py b/src/c_client.py
> index 35c975f..193e050 100644
> --- a/src/c_client.py
> +++ b/src/c_client.py
> @@ -54,14 +54,110 @@ def _c(fmt, *args):
> def _hc(fmt, *args):
> '''
> Writes the given line to both the header and source files.
> '''
> _h(fmt, *args)
> _c(fmt, *args)
>
> +def _c_wr_stringlist( indent, strlist ):
> + '''
> + Writes the given list of strings to the source file.
> + Each line is prepended by the indent string
> + '''
> + for str in strlist:
> + _c("%s%s", indent, str)
> +
> +
> +#For pre-code generated by expression generation
> +#(for example, the for-loop of a sumof)
> +#This has to account for recursiveness of the expression
> +#generation, i.e., there may be pre-code for pre-code.
> +#Therefore this is implemented as a stack of lists of lines.
> +#
Maybe put this comment as the class docstring? I.e.,
class PreCode:
"""For pre-code..."""
> +class PreCode:
Should be `class PreCode(object)`, that's recommended for python2 afaik.
> + def __init__(self):
> + self.nestingLevel = 0
Can you change this to `nesting_level`?
> + self.tempvars = []
> + self.codelines = []
> + self.redirect_code = None
> + self.redirect_tempvars = None
A small comment somewhere (maybe in the comments above) on what
redirection means here would be nice.
> + self.indent_str = ' '
> + self.indent_stack = []
> + self.tempvarNum = 0
Also here: `tempvar_num` (or `num_tempvars`).
> +
> +
> + #start and end of pre-code blocks
> + def start(self):
> + self.nestingLevel += 1
> +
> + def end(self):
> + self.nestingLevel -= 1
> + if ( self.nestingLevel == 0 ):
No need for parens here.
> + #lowest pre-code level is finished -> output to source
> + if self.redirect_tempvars == None:
`is None`.
> + _c_wr_stringlist('', self.tempvars )
> + self.tempvars = []
> + else:
> + self.redirect_tempvars.extend( self.tempvars )
> + self.tempvars = []
> + if self.redirect_code == None:
> + _c_wr_stringlist('', self.codelines )
> + self.codelines = []
> + else:
> + self.redirect_code.extend( self.codelines )
> + self.codelines = []
> +
> +
> + def output_tempvars(self):
> + if self.redirect_code == None:
> + _c_wr_stringlist('', self.tempvars )
> + self.tempvars = []
> +
> + #output to precode
> + def code( self, fmt, *args):
> + self.codelines.append( self.indent_str + fmt % args )
> +
> + def tempvar( self, fmt, *args):
> + self.tempvars.append(' ' + ( fmt % args ) )
> +
> + #get a unique name for a temporary variable
> + def get_tempvarname(self):
> + self.tempvarNum += 1
> + return "xcb_pre_tmp_%d" % self.tempvarNum
> +
> + #indentation
> +
> + def push_indent(self, indentstr):
> + self.indent_stack.append( self.indent_str )
> + self.indent_str = indentstr
> +
> + def push_addindent(self, indent_add_str):
> + self.push_indent( self.indent_str + indent_add_str )
> +
> + def indent(self):
> + self.push_addindent(' ')
> +
> + def pop_indent(self):
> + self.indent_str = self.indent_stack.pop()
> +
> + #redirection to lists
> + def redirect_start( self, redirectCode, redirectTempvars = None):
Please no camelCase here also. And no spaces around `=` in
`redirectTempvars = None`.
> + self.redirect_code = redirectCode
> + self.redirect_tempvars = redirectTempvars
> + if redirectTempvars != None:
`is not None`.
Ran
> + self.tempvarNum = 0
> +
> + def redirect_end(self):
> + self.redirect_code = None
> + self.redirect_tempvars = None
> +
> +#global PreCode handler
> +_c_pre = PreCode()
> +
> +
> # XXX See if this level thing is really necessary.
> def _h_setlevel(idx):
> '''
> Changes the array that header lines are written to.
> Supports writing different sections of the header file.
> '''
> global _hlevel
> @@ -1011,14 +1107,16 @@ def _c_serialize_helper_fields_variable_size(context, self, field,
> def _c_serialize_helper_fields(context, self,
> code_lines, temp_vars,
> space, prefix, is_case_or_bitcase):
> count = 0
> need_padding = False
> prev_field_was_variable = False
>
> + _c_pre.push_indent( space + ' ' )
> +
> for field in self.fields:
> if not field.visible:
> if not ((field.wire and not field.auto) or 'unserialize' == context):
> continue
>
> # switch/bitcase: fixed size fields must be considered explicitly
> if field.type.fixed_size():
> @@ -1099,14 +1197,16 @@ def _c_serialize_helper_fields(context, self,
> if field.c_field_type == 'void' or field.type.is_switch
> else field.c_field_type))
>
> need_padding = True
> if self.c_var_followed_by_fixed_fields:
> need_padding = False
>
> + _c_pre.pop_indent()
> +
> return count
> # _c_serialize_helper_fields()
>
> def _c_serialize_helper(context, complex_type,
> code_lines, temp_vars,
> space='', prefix=[]):
> # count tracks the number of fields to serialize
> @@ -1197,14 +1297,16 @@ def _c_serialize(context, self):
> _c("%s)" % param_str[-1].rstrip(','))
> _c('{')
>
> code_lines = []
> temp_vars = []
> prefix = []
>
> + _c_pre.redirect_start( code_lines, temp_vars )
> +
> if 'serialize' == context:
> if not self.is_switch and not self.c_var_followed_by_fixed_fields:
> _c(' %s *xcb_out = *_buffer;', self.c_type)
> _c(' unsigned int xcb_out_pad = -sizeof(%s) & 3;', self.c_type)
> _c(' unsigned int xcb_buffer_len = sizeof(%s) + xcb_out_pad;', self.c_type)
> _c(' unsigned int xcb_align_to = 0;')
> else:
> @@ -1242,20 +1344,22 @@ def _c_serialize(context, self):
> elif 'sizeof' == context:
> param_names = [p[2] for p in params]
> if self.is_switch:
> # switch: call _unpack()
> _c(' %s _aux;', self.c_type)
> _c(' return %s(%s, &_aux);', self.c_unpack_name, reduce(lambda x,y: "%s, %s" % (x, y), param_names))
> _c('}')
> + _c_pre.redirect_end()
> return
> elif self.c_var_followed_by_fixed_fields:
> # special case: call _unserialize()
> _c(' return %s(%s, NULL);', self.c_unserialize_name, reduce(lambda x,y: "%s, %s" % (x, y), param_names))
> _c('}')
> + _c_pre.redirect_end()
> return
> else:
> _c(' char *xcb_tmp = (char *)_buffer;')
> prefix = [('_aux', '->', self)]
> if self.is_switch:
> _c(' unsigned int xcb_padding_offset = 0;')
>
> count = _c_serialize_helper(context, self, code_lines, temp_vars, prefix=prefix)
> @@ -1282,14 +1383,16 @@ def _c_serialize(context, self):
> _c(' %s *_aux = malloc(sizeof(%s));', self.c_type, self.c_type)
>
> _c(' unsigned int xcb_buffer_len = 0;')
> _c(' unsigned int xcb_block_len = 0;')
> _c(' unsigned int xcb_pad = 0;')
> _c(' unsigned int xcb_align_to = 0;')
>
> + _c_pre.redirect_end()
> +
> _c('')
> for t in temp_vars:
> _c(t)
> _c('')
> for l in code_lines:
> _c(l)
>
> @@ -1671,20 +1774,19 @@ def _c_accessors_list(self, field):
> _hc('')
> _hc('int')
> if switch_obj is not None:
> _hc('%s (const %s *R /**< */,', field.c_length_name, R_obj.c_type)
> spacing = ' '*(len(field.c_length_name)+2)
> _h('%sconst %s *S /**< */);', spacing, S_obj.c_type)
> _c('%sconst %s *S /**< */)', spacing, S_obj.c_type)
> - length = _c_accessor_get_expr(field.type.expr, fields)
> else:
> _h('%s (const %s *R /**< */);', field.c_length_name, c_type)
> _c('%s (const %s *R /**< */)', field.c_length_name, c_type)
> - length = _c_accessor_get_expr(field.type.expr, fields)
> _c('{')
> + length = _c_accessor_get_expr(field.type.expr, fields)
> _c(' return %s;', length)
> _c('}')
>
> if field.type.member.is_simple:
> _hc('')
> _hc('xcb_generic_iterator_t')
> if switch_obj is not None:
> @@ -1726,33 +1828,39 @@ def _c_accessors_list(self, field):
> _c('%sconst %s *S /**< */)', spacing, S_obj.c_type)
> else:
> _h('%s (const %s *R /**< */);', field.c_iterator_name, c_type)
> _c('%s (const %s *R /**< */)', field.c_iterator_name, c_type)
> _c('{')
> _c(' %s i;', field.c_iterator_type)
>
> + _c_pre.start()
> + length_expr_str = _c_accessor_get_expr(field.type.expr, fields)
> +
> if switch_obj is not None:
> + _c_pre.end()
> _c(' i.data = %s;', fields[field.c_field_name][0])
> - _c(' i.rem = %s;', _c_accessor_get_expr(field.type.expr, fields))
> + _c(' i.rem = %s;', length_expr_str)
> elif field.prev_varsized_field == None:
> + _c_pre.end()
> _c(' i.data = (%s *) (R + 1);', field.c_field_type)
> else:
> (prev_varsized_field, align_pad) = get_align_pad(field)
>
> if align_pad is None:
> align_pad = ('XCB_TYPE_PAD(%s, prev.index)' %
> type_pad_type(field.c_field_type))
>
> _c(' xcb_generic_iterator_t prev = %s;',
> _c_iterator_get_end(prev_varsized_field, 'R'))
> + _c_pre.end()
> _c(' i.data = (%s *) ((char *) prev.data + %s);',
> field.c_field_type, align_pad)
>
> if switch_obj is None:
> - _c(' i.rem = %s;', _c_accessor_get_expr(field.type.expr, fields))
> + _c(' i.rem = %s;', length_expr_str )
> _c(' i.index = (char *) i.data - (char *) %s;', 'R' if switch_obj is None else 'S' )
> _c(' return i;')
> _c('}')
>
> def _c_accessors(self, name, base):
> '''
> Declares the accessor functions for the fields of a structure.
> -- 2.0.1 _______________________________________________ Xcb mailing list Xcb at lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xcb
>
>
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
More information about the Xcb
mailing list