Eric Blake <ebl...@redhat.com> writes:
> We already have several places that want to visit all the members
> of an implicit object within a larger context (simple union variant,
> event with anonymous data, command with anonymous arguments struct);
> and will be adding another one soon (the ability to declare an
> anonymous base for a flat union). Having a C struct declared for
> these implicit types, along with a visit_type_FOO_members() helper
> function, will make for fewer special cases in our generator.
Yes.
> We do not, however, need qapi_free_FOO() or visit_type_FOO()
> functions for implicit types, because they should not be used
> directly outside of the generated code. This is done by adding a
> conditional in visit_object_type() for both qapi-types.py and
> qapi-visit.py based on the object name. The comparison of
> "name[0] == ':'" feels a bit hacky, but beats changing the
> signature of the visit_object_type() callback to pass a new
> 'implicit' flag.
PRO: it matches what QAPISchemaObjectType.is_implicit() does.
CON: it duplicates what QAPISchemaObjectType.is_implicit() does.
Ways to adhere to DRY:
(1) Add a flag to visit_object_type() and, for consistency, to
visit_object_type_flat().
(2) Change the QAPISchemaVisitor.visit_FOO() to take a full FOO instead
of FOO.name, FOO.info and selected other members.
We've discussed (2) elsewhere. The QAPISchemaEntity.visit() shrink to a
mere double-dispatch. The QAPISchemaVisitor gain full access to the
things they visit. The interface between the generators and the
internal representation changes from a narrow, explicit and inflexible
one to a much wider "anything goes" one. Both the narrow and the wide
interface have advantages and disadvantages. Have we outgrown the
narrow one?
> Also, now that we WANT to output C code for
> implicits, we have to change the condition in the visit_needed()
> filter.
>
> We have to special-case ':empty' as something that does not get
> output: because it is a built-in type, it would be emitted in
> multiple files (the main qapi-types.h and in qga-qapi-types.h)
> causing compilation failure due to redefinition. But since it
> has no members, it's easier to just avoid an attempt to visit
> that particular type.
>
> The patch relies on the fact that all our implicit objects start
> with a leading ':', which can be transliteratated to a leading
transliterated
> single underscore, and we've already documented and enforce that
Uh, these are "always reserved for use as identifiers with file scope"
(C99 7.1.3). I'm afraid we need to use the q_ prefix.
> the user can't create QAPI names with a leading underscore, so
> exposing the types won't create any collisions. It is a bit
> unfortunate that the generated struct names don't match normal
> naming conventions, but it's not too bad, since they will only
> be used in generated code.
The problem is self-inflicted: we make up these names in
_make_implicit_object_type(). We could just as well make up names that
come out prettier.
In fact, my first versions of the code had names starting with ':' for
*all* implicit types. I abandoned that for enum and array types when I
realized I need C names for them, and the easiest way to get them making
up names so that a trivial .c_name() works. We can do the same for
object types.
> The generated code grows substantially in size; in part because
> it was easier to output every implicit type rather than adding
> generator complexity to try and only output types as needed.
I happily trade larger .c files the optimizer can reduce for a simpler
generator. I'm less sanguine about larger .h files when they get
included a lot. qapi-types.h gets included basically everywhere, and
grows from ~4000 to ~5250 lines. How much of this is avoidable?
> Signed-off-by: Eric Blake <ebl...@redhat.com>
Can't find fault with the patch itself.
