Re: [PATCH v3 1/8] qapi: golang: Generate enum type

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jan 14, 2025 at 09:52:23AM +0100, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
> 
> > This patch handles QAPI enum types and generates its equivalent in Go.
> > We sort the output based on enum's type name.
> 
> Any particular reason for sorting?
> 
> The existing backends generate output it source order, on the (bold?)
> assumption that developers care to pick an order that makes sense.

Even if that assumption is valid (and I question whether it really is),
what makes sense to a QEMU maintainer is not the same as what makes
sense to a consumer of QEMU.

Our approach to file splitting is fairly arbitrary

First, we've got a split across sub-systems, except where we don't.

Sometimes one subsystem splits into many files. eg block.json vs
block-core.json vs block-export.json

Sometimes we just pull something into a standalone file even when it
is not really a subsystem just common code. eg sockets.json

Sometimes we have a split because its convenient for QEMU linux-user
vs system compilation & ELF linker needs. eg misc.json vs misc-target.json 

At best we can say that the split of files is intentionally done for the
convenience of the QEMU maintainers. This is irrevelant (and harmful)
for anyone who isn't a QEMU maintainer, which covers any consumer of the
Go code.

IMHO it is also a bug that this file split leaks out into our existing
QAPI docs.


Then when adding to files, sometimes people append to the end of a
file, sometimes it is injected in the middle. I can't say that is an
intentional/concious ordering decision.

The overall result is that wearing my hat of a consumer of QEMU, when
looking at the QAPI schema there is no discernable ordering.


Given the source ordering is not useful, there are two choices

 * Sort to have declarations before use as primary key, and then
   alphabetical as a secondary sort key

 * Sort alphabetically as the primary key

Fortunately the Go compiler has no requirement for forward declarations.
Sorting declarations before use also doesn't really have much effect on
most declarations, so the first option ends up 90% alphabetical sorted
anyway. Might as well just go the whole way and do pure alphabetical
sorting.

Comparing the enums code in "source order":

  https://gitlab.com/victortoso/qapi-go/-/blob/qapi-golang-v1-by-tags/pkg/qapi/enums.go

vs alphabetical order:

  https://gitlab.com/victortoso/qapi-go/-/blob/qapi-golang-v3-by-tags/pkg/qapi/enums.go

the improvement is massive IMHO.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|