[Xcb] [PATCH v2] c_client.py: avoid field name clash with C++ 'explicit' keyword
Christian Linhart
chris at DemoRecorder.com
Tue Jan 26 20:15:37 PST 2016
Hi Klemens,
Thank you for your patch.
I think that any new scheme should only be used for "explicit" and other newly added
cplusplus_annoyances, thus making the change in the generated code minimal.
Then, only xkb will be affected.
Therefore, most code will still work, even for compilers that don't support anonymous unions.
(xkb is probably rarely used because the XCB-impl of xkb is rather buggy.
So we can probably get away with breaking xkb for old C-compilers that don't support anonymous unions.
or use the suggestion in the P.S. of this post)
So this will require to split "cplusplus_annoyances" into two lists:
* leave "cplusplus_annoyances" as it is, and treat it as it is.
* a list "new_cplusplus_annoyances" which will contain "explicit", and probably all the other C++-only keywords.
Then, the python-field "c_legacy_field_name" will include the result from processing cplusplus_annoyances,
and python-function "_c_sanitize" will be renamed "_legacy_sanitize" and also process the list "cplusplus_annoyances".
The python-function "_cpp_sanitize" will process "new_cplusplus_annoyances" instead of "cplusplus_annoyances".
Probably some other changes are necessary, too.
To the implementation-details in c_client.py:
* no need to use a macro "XCB_ALTERNATIVE_NAMES".
Macros are for manually written code to enforce consistency.
We can just generate the code directly because the generator works consistently anyways.
This simplifies the changes in the generator and makes the code more readable.
(This point is probably a matter of taste and therefore others probably disagree with me.)
* the change with the new function "declare_as_pointer" has nothing to do with the c++-keyword issues.
Please post a separate patch for this, with explanation why this is necessary.
(or is this change a result of a diff with a wrong version?)
Can you please post a revised patch if you think that my suggestions are useful?
Thank you,
Chris
P.S.: If we need to support xkb for compilers without anonymous unions, we may
offer a macro that disables the use of anonymous unions when defined.
Therefore, generated code will look like:
#ifdef __cplusplus
uint8_t _explicit;
#elif defined(XCB_NO_ANON_UNIONS)
uint8_t explicit;
#else
union{
uint8_t _explicit;
uint8_t explicit;
}
#endif
On 2015-12-27 22:06, 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.
>
> 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.
>
> 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