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

[Victor Toso]

Hi,

On Thu, Sep 28, 2023 at 02:52:08PM +0100, Daniel P. Berrangé wrote:
> 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.
> 
> Browsing the generated Go code to understand it, I find myself
> wishing that it was emitted in alphabetical order.
> 
> This could be done if we worked in two phase. In the visit phase,
> we collect the bits of data we need, and then add a format phase
> then generates the formatted output, having first sorted by enum
> name.
> 
> Same thought for the other types/commands.

I cared for sorted in some places [0] but not all of them indeed.
I'll include your request/suggestion in the next version.

[0] 
https://gitlab.com/victortoso/qemu/-/blob/qapi-golang-v1/scripts/qapi/golang.py?ref_type=heads#L804

> 
> > +
> > +    def visit_array_type(self, name, info, ifcond, element_type):
> > +        pass
> > +
> > +    def visit_command(self,
> > +                      name: str,
> > +                      info: Optional[QAPISourceInfo],
> > +                      ifcond: QAPISchemaIfCond,
> > +                      features: List[QAPISchemaFeature],
> > +                      arg_type: Optional[QAPISchemaObjectType],
> > +                      ret_type: Optional[QAPISchemaType],
> > +                      gen: bool,
> > +                      success_response: bool,
> > +                      boxed: bool,
> > +                      allow_oob: bool,
> > +                      allow_preconfig: bool,
> > +                      coroutine: bool) -> None:
> > +        pass
> > +
> > +    def visit_event(self, name, info, ifcond, features, arg_type, boxed):
> > +        pass
> > +
> > +    def write(self, output_dir: str) -> None:
> > +        for module_name, content in self.target.items():
> > +            go_module = module_name + "s.go"
> > +            go_dir = "go"
> > +            pathname = os.path.join(output_dir, go_dir, go_module)
> > +            odir = os.path.dirname(pathname)
> > +            os.makedirs(odir, exist_ok=True)
> > +
> > +            with open(pathname, "w", encoding="ascii") as outfile:
> 
> IIUC, we defacto consider the .qapi json files to be UTF-8, and thus
> in theory we could have non-ascii characters in there somewhere. I'd
> suggest we using utf8 encoding when outputting to avoid surprises.

Sure thing.

Cheers,
Victor

signature.asc
Description: PGP signature