Re: [PATCH v1 1/9] qapi: golang: Generate qapi's enum types in Go

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

On Thu, Sep 28, 2023 at 04:20:55PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Wed, Sep 27, 2023 at 01:25:36PM +0200, Victor Toso wrote:
> >> This patch handles QAPI enum types and generates its equivalent in Go.
> >> 
> >> Basically, Enums are being handled as strings in Golang.
> >> 
> >> 1. For each QAPI enum, we will define a string type in Go to be the
> >>    assigned type of this specific enum.
> >> 
> >> 2. Naming: CamelCase will be used in any identifier that we want to
> >>    export [0], which is everything.
> >> 
> >> [0] https://go.dev/ref/spec#Exported_identifiers
> >> 
> >> Example:
> >> 
> >> qapi:
> >>   | { 'enum': 'DisplayProtocol',
> >>   |   'data': [ 'vnc', 'spice' ] }
> >> 
> >> go:
> >>   | type DisplayProtocol string
> >>   |
> >>   | const (
> >>   |     DisplayProtocolVnc   DisplayProtocol = "vnc"
> >>   |     DisplayProtocolSpice DisplayProtocol = "spice"
> >>   | )
> >> 
> >> Signed-off-by: Victor Toso <victortoso@redhat.com>
> >> ---
> >>  scripts/qapi/golang.py | 140 +++++++++++++++++++++++++++++++++++++++++
> >>  scripts/qapi/main.py   |   2 +
> >>  2 files changed, 142 insertions(+)
> >>  create mode 100644 scripts/qapi/golang.py
> >> 
> >> diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> >> new file mode 100644
> >> index 0000000000..87081cdd05
> >> --- /dev/null
> >> +++ b/scripts/qapi/golang.py
> >> @@ -0,0 +1,140 @@
> >> +"""
> >> +Golang QAPI generator
> >> +"""
> >> +# Copyright (c) 2023 Red Hat Inc.
> >> +#
> >> +# Authors:
> >> +#  Victor Toso <victortoso@redhat.com>
> >> +#
> >> +# This work is licensed under the terms of the GNU GPL, version 2.
> >> +# See the COPYING file in the top-level directory.
> >> +
> >> +# due QAPISchemaVisitor interface
> >> +# pylint: disable=too-many-arguments
> >> +
> >> +# Just for type hint on self
> >> +from __future__ import annotations
> >> +
> >> +import os
> >> +from typing import List, Optional
> >> +
> >> +from .schema import (
> >> +    QAPISchema,
> >> +    QAPISchemaType,
> >> +    QAPISchemaVisitor,
> >> +    QAPISchemaEnumMember,
> >> +    QAPISchemaFeature,
> >> +    QAPISchemaIfCond,
> >> +    QAPISchemaObjectType,
> >> +    QAPISchemaObjectTypeMember,
> >> +    QAPISchemaVariants,
> >> +)
> >> +from .source import QAPISourceInfo
> >> +
> >> +TEMPLATE_ENUM = '''
> >> +type {name} string
> >> +const (
> >> +{fields}
> >> +)
> >> +'''
> >> +
> >> +
> >> +def gen_golang(schema: QAPISchema,
> >> +               output_dir: str,
> >> +               prefix: str) -> None:
> >> +    vis = QAPISchemaGenGolangVisitor(prefix)
> >> +    schema.visit(vis)
> >> +    vis.write(output_dir)
> >> +
> >> +
> >> +def qapi_to_field_name_enum(name: str) -> str:
> >> +    return name.title().replace("-", "")
> >> +
> >> +
> >> +class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
> >> +
> >> +    def __init__(self, _: str):
> >> +        super().__init__()
> >> +        types = ["enum"]
> >> +        self.target = {name: "" for name in types}
> >> +        self.schema = None
> >> +        self.golang_package_name = "qapi"
> >> +
> >> +    def visit_begin(self, schema):
> >> +        self.schema = schema
> >> +
> >> +        # Every Go file needs to reference its package name
> >> +        for target in self.target:
> >> +            self.target[target] = f"package {self.golang_package_name}\n"
> >> +
> >> +    def visit_end(self):
> >> +        self.schema = None
> >> +
> >> +    def visit_object_type(self: QAPISchemaGenGolangVisitor,
> >> +                          name: str,
> >> +                          info: Optional[QAPISourceInfo],
> >> +                          ifcond: QAPISchemaIfCond,
> >> +                          features: List[QAPISchemaFeature],
> >> +                          base: Optional[QAPISchemaObjectType],
> >> +                          members: List[QAPISchemaObjectTypeMember],
> >> +                          variants: Optional[QAPISchemaVariants]
> >> +                          ) -> None:
> >> +        pass
> >> +
> >> +    def visit_alternate_type(self: QAPISchemaGenGolangVisitor,
> >> +                             name: str,
> >> +                             info: Optional[QAPISourceInfo],
> >> +                             ifcond: QAPISchemaIfCond,
> >> +                             features: List[QAPISchemaFeature],
> >> +                             variants: QAPISchemaVariants
> >> +                             ) -> None:
> >> +        pass
> >> +
> >> +    def visit_enum_type(self: QAPISchemaGenGolangVisitor,
> >> +                        name: str,
> >> +                        info: Optional[QAPISourceInfo],
> >> +                        ifcond: QAPISchemaIfCond,
> >> +                        features: List[QAPISchemaFeature],
> >> +                        members: List[QAPISchemaEnumMember],
> >> +                        prefix: Optional[str]
> >> +                        ) -> None:
> >> +
> >> +        value = qapi_to_field_name_enum(members[0].name)
> >> +        fields = ""
> >> +        for member in members:
> >> +            value = qapi_to_field_name_enum(member.name)
> >> +            fields += f'''\t{name}{value} {name} = "{member.name}"\n'''
> >> +
> >> +        self.target["enum"] += TEMPLATE_ENUM.format(name=name, fields=fields[:-1])
> >
> > Here you are formatting the enums as you visit them, appending to
> > the output buffer. The resulting enums appear in whatever order we
> > visited them with, which is pretty arbitrary.
> 
> We visit in source order, not in arbitrary order.

I meant arbitrary in the sense that us developers just add new
QAPI types pretty much anywhere we feel like it in the .qapi
files.

> 
> > Browsing the generated Go code to understand it, I find myself
> > wishing that it was emitted in alphabetical order.
> 
> If that's easier to read in generated Go, then I suspect it would also
> be easier to read in the QAPI schema and in generated C.

Yes, although C has some ordering constraints on things being
declared, so it would be a bit harder to do this in C.


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 :|