From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44203) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwKxN-0003TB-D9 for qemu-devel@nongnu.org; Tue, 31 Jul 2012 18:30:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwKxK-0005M2-Qt for qemu-devel@nongnu.org; Tue, 31 Jul 2012 18:30:25 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:49293) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwKxK-0005Lb-KM for qemu-devel@nongnu.org; Tue, 31 Jul 2012 18:30:22 -0400 Received: by obbta14 with SMTP id ta14so10612821obb.4 for ; Tue, 31 Jul 2012 15:30:21 -0700 (PDT) Sender: fluxion Date: Tue, 31 Jul 2012 17:30:02 -0500 From: Michael Roth Message-ID: <20120731223002.GJ2880@illuin> References: <5017892B.2070609@redhat.com> <20120731095834.517c37bc@doriath.home> <20120731185534.GI2880@illuin> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Paolo Bonzini , qemu-devel@nongnu.org, Luiz Capitulino On Tue, Jul 31, 2012 at 08:38:45PM +0000, Blue Swirl wrote: > On Tue, Jul 31, 2012 at 6:55 PM, Michael Roth 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 > >> wrote: > >> > On Tue, 31 Jul 2012 09:28:43 +0200 > >> > Paolo Bonzini wrote: > >> > > >> >> Il 30/07/2012 18:04, blauwirbel@gmail.com ha scritto: > >> >> > From: Blue Swirl > >> >> > > >> >> > 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 As a struct field it didn't seem like there was a case where the usage would be ambiguous, but yah, it's still justified to complain. > > Prefixing would solve also future problems: 'if', 'auto', maybe > 'static' can make sense for network options (as compared to DHCP) one > day etc. > I don't think there's a necessary risk of this happening again. We can catch it in code review, warn against it in documentation, and even go as far as adding a list of reserved keywords that the schema parser/code generators can complain about. (I'm also somewhat biased because QIDL needs the schema names to align with the actual field names, since the schema in that case is a description of an exiting type that is not generated by QAPI. Although it wouldn't be too hard to add a command-line option to still support that...) Another option I'll put out there is to have the code generators special-case `field_name in [ ]:` to re-name the fields internally, and document the behavior for developers. This keeps our mistakes from spilling out into the QAPI schemas. > > > > 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': { > > '': '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. > >> >