[Xcb] [PATCH v2] c_client.py: avoid field name clash with C++ 'explicit' keyword
Ran Benita
ran234 at gmail.com
Mon Dec 28 12:09:22 PST 2015
On Sun, Dec 27, 2015 at 10:06:27PM +0100, Klemens Baum wrote:
> The generated code was causing compilation errors when xcb/xkb.h
> is included in a C++ code base.
>
> To avoid breaking the API for C users, the unprefixed field names
> are provided as alternative names via an anonymous union.
I'm not sure if XCB is willing to use C11 already. Though I think gcc
supports anonymous unions for a long time.
> For existing C++ users, the relevant header file would not have
> compiled in the first place, so breaking the API is justified.
> If they were abusing the preprocessor to modify the header,
> it's their own fault for not resolving the issue upstream.
Note that qt already works around the issue:
https://github.com/qtproject/qtbase/blob/e979f8721731bc5f4cd0d65830548f9e70da2da5/src/plugins/platforms/xcb/qxcbconnection.h#L51
Ran
> This resolves bug #74080.
> ---
> src/c_client.py | 74 ++++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 50 insertions(+), 24 deletions(-)
>
> diff --git a/src/c_client.py b/src/c_client.py
> index c38b434..343e9d6 100644
> --- a/src/c_client.py
> +++ b/src/c_client.py
> @@ -17,9 +17,10 @@ _cname_special_cases = {'DECnet':'decnet'}
>
> _extension_special_cases = ['XPrint', 'XCMisc', 'BigRequests']
>
> -_cplusplus_annoyances = {'class' : '_class',
> - 'new' : '_new',
> - 'delete': '_delete'}
> +_cplusplus_annoyances = {'class' : '_class',
> + 'new' : '_new',
> + 'delete' : '_delete',
> + 'explicit': '_explicit'}
> _c_keywords = {'default' : '_default'}
>
> _hlines = []
> @@ -192,16 +193,21 @@ def _n_item(str):
> name_parts = [match.group(0) for match in split]
> return '_'.join(name_parts)
>
> -def _cpp(str):
> +def _c_sanitize(str):
> + '''
> + Checks for certain C reserved words and fixes them.
> + '''
> + if str in _c_keywords:
> + return _c_keywords[str]
> + return str
> +
> +def _cpp_sanitize(str):
> '''
> Checks for certain C++ reserved words and fixes them.
> '''
> if str in _cplusplus_annoyances:
> return _cplusplus_annoyances[str]
> - elif str in _c_keywords:
> - return _c_keywords[str]
> - else:
> - return str
> + return _c_sanitize(str)
>
> def _ext(str):
> '''
> @@ -297,6 +303,14 @@ def c_open(self):
> _h('')
> _h('#ifdef __cplusplus')
> _h('extern "C" {')
> + _h('#define XCB_ALTERNATIVE_NAMES(type, c_name, cpp_name) \\')
> + _h(' type cpp_name')
> + _h('#else')
> + _h('#define XCB_ALTERNATIVE_NAMES(type, c_name, cpp_name) \\')
> + _h(' union { \\')
> + _h(' type c_name; \\')
> + _h(' type cpp_name; \\')
> + _h(' }')
> _h('#endif')
>
> if _ns.is_ext:
> @@ -324,6 +338,9 @@ def c_close(self):
> _h('#endif')
>
> _h('')
> + _h('#undef XCB_ALTERNATIVE_NAMES')
> +
> + _h('')
> _h('#endif')
> _h('')
> _h('/**')
> @@ -425,7 +442,8 @@ def _c_type_setup(self, name, postfix):
> self.c_need_serialize = True
> self.c_container = 'struct'
> for bitcase in self.bitcases:
> - bitcase.c_field_name = _cpp(bitcase.field_name)
> + bitcase.c_field_name = _cpp_sanitize(bitcase.field_name)
> + bitcase.c_legacy_field_name = _c_sanitize(bitcase.field_name)
> bitcase_name = bitcase.field_type if bitcase.type.has_name else name
> _c_type_setup(bitcase.type, bitcase_name, ())
>
> @@ -439,7 +457,8 @@ def _c_type_setup(self, name, postfix):
> for field in self.fields:
> field.c_field_type = _t(field.field_type)
> field.c_field_const_type = ('' if field.type.nmemb == 1 else 'const ') + field.c_field_type
> - field.c_field_name = _cpp(field.field_name)
> + field.c_field_name = _cpp_sanitize(field.field_name)
> + field.c_legacy_field_name = _c_sanitize(field.field_name)
> field.c_subscript = '[%d]' % field.type.nmemb if (field.type.nmemb and field.type.nmemb > 1) else ''
> field.c_pointer = ' ' if field.type.nmemb == 1 else '*'
>
> @@ -580,13 +599,13 @@ def _c_helper_fieldaccess_expr(prefix, field=None):
> # (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)
> + prefix_str += last_sep + _cpp_sanitize(field.field_name)
> 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)
> + prefix_str += last_sep + _cpp_sanitize(field.field_name)
>
> return prefix_str
>
> @@ -612,7 +631,7 @@ def _c_helper_field_mapping(complex_type, prefix, flat=False):
> if f.field_name in all_fields:
> raise Exception("field name %s has been registered before" % f.field_name)
>
> - all_fields[f.field_name] = (fname, f)
> + all_fields[f.c_field_name] = (fname, f)
> if f.type.is_container and not flat:
> if f.type.is_case_or_bitcase and not f.type.has_name:
> new_prefix = prefix
> @@ -1073,9 +1092,9 @@ def _c_serialize_helper_fields_fixed_size(context, self, field,
> raise Exception("type for field '%s' (expression '%s') unkown" %
> (field.field_name, _c_accessor_get_expr(field.type.expr)))
>
> - temp_vars.append(' %s xcb_expr_%s = %s;' % (field.type.c_type, _cpp(field.field_name),
> + temp_vars.append(' %s xcb_expr_%s = %s;' % (field.type.c_type, _cpp_sanitize(field.field_name),
> _c_accessor_get_expr(field.type.expr, prefix)))
> - value += "&xcb_expr_%s;" % _cpp(field.field_name)
> + value += "&xcb_expr_%s;" % _cpp_sanitize(field.field_name)
>
> elif field.type.is_pad:
> if field.type.nmemb == 1:
> @@ -2054,6 +2073,12 @@ def _c_complex(self, force_packed = False):
> struct_fields = []
> maxtypelen = 0
>
> + def _declare_as_pointer(self, field):
> + return not ((field.type.fixed_size() and not self.is_union) or
> + # in case of switch with switch children, don't make the field a pointer
> + # necessary for unserialize to work
> + (self.is_switch and field.type.is_switch))
> +
> for field in self.fields:
> if field.wire and (field.type.fixed_size() or self.is_switch or self.is_union):
> struct_fields.append(field)
> @@ -2061,20 +2086,21 @@ def _c_complex(self, force_packed = False):
> for field in struct_fields:
> length = len(field.c_field_type)
> # account for '*' pointer_spec
> - if not field.type.fixed_size() and not self.is_union:
> + if _declare_as_pointer(self, field):
> length += 1
> maxtypelen = max(maxtypelen, length)
>
> def _c_complex_field(self, field, space=''):
> - if (field.type.fixed_size() or self.is_union or
> - # in case of switch with switch children, don't make the field a pointer
> - # necessary for unserialize to work
> - (self.is_switch and field.type.is_switch)):
> - spacing = ' ' * (maxtypelen - len(field.c_field_type))
> - _h('%s %s%s %s%s;', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
> + if field.c_field_name != field.c_legacy_field_name:
> + pointer = ' *' if _declare_as_pointer(self, field) else ''
> + _h('%s XCB_ALTERNATIVE_NAMES(%s%s, %s, %s);', space, field.c_field_type,
> + pointer, field.c_legacy_field_name, field.c_field_name)
> else:
> - spacing = ' ' * (maxtypelen - (len(field.c_field_type) + 1))
> - _h('%s %s%s *%s%s;', space, field.c_field_type, spacing, field.c_field_name, field.c_subscript)
> + spacing = ' ' * (maxtypelen - len(field.c_field_type))
> + pointer = '*' if _declare_as_pointer(self, field) else ' '
> + _h('%s %s%s%s%s%s;', space, field.c_field_type, spacing, pointer,
> + field.c_field_name, field.c_subscript)
> +
>
> if not self.is_switch:
> for field in struct_fields:
> --
> 2.6.4
>
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
More information about the Xcb
mailing list