Eric Blake <ebl...@redhat.com> writes: > On 02/17/2016 05:05 AM, Markus Armbruster wrote: >> Let's check for completeness. Calls of c_enum_const(): >> >> * QAPISchemaEnumType.c_null() and (with your patch) gen_visit_union() >> call it like >> >> c_enum_const(TYPE.name, MEMBER, TYPE.prefix) >> >> where MEMBER is a member of enumeration type TYPE. >> >> As your patch shows, the prefix is easy to forget. A safer function >> would take just TYPE and MEMBER: >> >> TYPE.c_member(MEMBER) >> > > I took a look at this email again today, and while it still sounds like > a nice action, I wasn't able to get it done quickly (certainly not 2.6 > material, at any rate). > >> * gen_event_send() calls >> >> c_enum_const(event_enum_name, name) >> >> where name is member of the enum type named event_enum_name. That's >> okay because the type is auto-generated without a prefix. Regardless, >> we could do something like >> >> schema.lookup_type(event_enum_name).c_member(name) >> >> Requires actually constructing the type, which is probably a good idea >> anyway, because it gets us the necessary collision checks. Replacing >> global event_enum_name by event_enum_type would be nice then. Out of >> scope for this patch. >> >> * gen_enum_lookup() and gen_enum() work on name, values, prefix instead >> of the type. I figure they do to support qapi-event.py. If we clean >> it up to create the type, these functions could use the type as well, >> and then c_enum_const() could be dropped. > > Well, they also do it to support qapi-types.py, where the > visit_enum_type() visitor callback has already broken things out into > name/values/prefix instead of providing a type, and where we don't have > easy access to the schema to do schema.lookup_type(name). > > As I've worked more with the visitors, I'm really wondering if > visit_enum_type(self, type, info) would be a saner callback interface > than visit_enum_type(self, type.name, info, type.values, type.prefix)
Maybe it is, but I'm not sure. The existing visit_enum_type() forces discipline: you can't just use whatever property of the type you like. Discipline is good until you overdo it and it becomes a straightjacket. Let's not worry about it now, but concentrate on keeping the existing, lengthy QAPI queue moving.
