Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict

[Blue Swirl]

On Tue, Jul 31, 2012 at 6:55 PM, Michael Roth <address@hidden> wrote:
> On Tue, Jul 31, 2012 at 04:56:50PM +0000, Blue Swirl wrote:
>> On Tue, Jul 31, 2012 at 12:58 PM, Luiz Capitulino
>> <address@hidden> wrote:
>> > On Tue, 31 Jul 2012 09:28:43 +0200
>> > Paolo Bonzini <address@hidden> wrote:
>> >
>> >> Il 30/07/2012 18:04, address@hidden ha scritto:
>> >> > From: Blue Swirl <address@hidden>
>> >> >
>> >> > Clang compiler complained about use of reserved word 'restrict' in SLIRP
>> >> > and QAPI.
>> >> >
>> >> > Rename 'restrict' to 'restricted' which also matches other SLIRP code.
>> >>
>> >> Can't do it, this changes the command-line option.
>> >>
>> >> Luiz, Michael, any ideas?
>> >
>> > I'm not sure how complicated it would be to implement this, but we could 
>> > add
>> > a 'bind' keyword to the type dict to control mapping between protocol names
>> > and generated variable names. Like this:
>> >
>> > { 'type': 'NetdevUserOptions',
>> >   'data': {
>> >     '*hostname':  'str',
>> >     '*restrict':  'bool',
>> >     ...
>> >     '*hostfwd':   ['String'],
>> >     '*guestfwd':  ['String'] },
>> >
>> >   'bind': { 'restrict': 'restricted' } }
>>
>> How about prefixing all json-generated field names with for example
>> 'json_'? Should be a simple mechanical change.
>
> It's a whole lot of churn though, and clobbers the history for most QMP
> functions. It also seems like a strange thing for clang to complain about...

Not really, it's just C99:
http://en.wikipedia.org/wiki/Restrict

Prefixing would solve also future problems: 'if', 'auto', maybe
'static' can make sense for network options (as compared to DHCP) one
day etc.

>
> I think special casing is probably the way to go...
>
> Luiz's bind approach seems reasonable, though "alias" might be the more
> familiar name.
>
> As an alternative I'll throw out there, the QIDL series introduces the
> notion of "annotated" fields, which allow us to pass additional
> information to the code generators (instead of just the typename)
> regarding how to handle that field internally. So we could do something like:
>
> { 'type': 'NetdevUserOptions',
>   'data': {
>     '*hostname':  'str',
>     '*restrict': {
>       '<annotated>': 'true',
>       'type': 'bool',
>       'native_name': 'restrict' },
>     ...
>     '*hostfwd':   ['String'],
>     '*guestfwd':  ['String'] } }
>
> It's pretty flexible, but I'm hesitant to expose in documented schemas.

This could work too.

We could also introduce new names and deprecate old ones, one day we
could remove the old versions. This is bit drastic change to be done
just because a new user invisible implementation makes command line
names clash with C keywords. I'd rather prefix.

>
>>
>> In addition to 'restrict', there may also be problems with 'if'
>> (-drive, HMP drive_add) and maybe also 'auto' as value (several
>> command line options, HMP pci_add) in the future.
>>