Re: [PATCH 3/3] scripts/qapi: generate high-level Rust bindings

[Zhao Liu <zhao1.liu@intel.com>]

On Thu, Jun 05, 2025 at 12:11:24PM +0200, Paolo Bonzini wrote:
> Date: Thu,  5 Jun 2025 12:11:24 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 3/3] scripts/qapi: generate high-level Rust bindings
> X-Mailer: git-send-email 2.49.0
> 
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Generate high-level idiomatic Rust code for the QAPI types, with to/from
> translations for the C FFI.
> 
> - char* is mapped to String, scalars to there corresponding Rust types
> 
> - enums are simply aliased from FFI
> 
> - has_foo/foo members are mapped to Option<T>
> 
> - lists are represented as Vec<T>
> 
> - structures have Rust versions, with To/From FFI conversions
>
> - alternate are represented as Rust enum
> 
> - unions are represented in a similar way as in C: a struct S with a "u"
>   member (since S may have extra 'base' fields). However, the discriminant
>   isn't a member of S, since Rust enum already include it.

Why not map the C union to rust union directly (in `pub enum %(rs_name)sVariant`)?

(latter comments are all about format nits)

...

> +%(cfg)s
> +impl From<&%(rs_name)sVariant> for %(tag)s {
> +    fn from(e: &%(rs_name)sVariant) -> Self {
> +        match e {
> +    ''',
> +                cfg=ifcond.rsgen(),
> +                rs_name=rs_name(name),
> +                tag=rs_type(variants.tag_member.type.c_type(), ''))
> +
> +    for var in variants.variants:
> +        type_name = var.type.name
> +        var_name = to_camel_case(rs_name(var.name))
> +        patt = '(_)'
> +        if type_name == 'q_empty':
> +            patt = ''
> +        ret += mcgen('''
> +    %(cfg)s

This introduces extra \n, which will generate a blank line if there's
no cfg.

> +    %(rs_name)sVariant::%(var_name)s%(patt)s => Self::%(var_name)s,

So, I think it should be:

    %(cfg)s    %(rs_name)sVariant::%(var_name)s%(patt)s => Self::%(var_name)s,

> +''',
> +                     cfg=var.ifcond.rsgen(),
> +                     rs_name=rs_name(name),
> +                     var_name=var_name,
> +                     patt=patt)
> +
> +    ret += mcgen('''
> +        }
> +    }
> +}
> +''')
> +    return ret
> +
> +
> +def gen_rs_variants(name: str,
> +                    ifcond: QAPISchemaIfCond,
> +                    variants: Optional[QAPISchemaVariants]) -> str:
> +    ret = mcgen('''
> +
> +%(cfg)s
> +#[derive(Clone,Debug)]
> +pub enum %(rs_name)sVariant {
> +''',
> +                cfg=ifcond.rsgen(),
> +                rs_name=rs_name(name))
> +
> +    for var in variants.variants:
> +        type_name = var.type.name
> +        var_name = to_camel_case(rs_name(var.name, False))
> +        if type_name == 'q_empty':
> +            ret += mcgen('''
> +    %(cfg)s
> +    %(var_name)s,

ditto,

    %(cfg)s    %(var_name)s,

> +''',
> +                         cfg=var.ifcond.rsgen(),
> +                         var_name=var_name)
> +        else:
> +            c_type = var.type.c_unboxed_type()
> +            if c_type.endswith('_wrapper'):
> +                c_type = c_type[6:-8]  # remove q_obj*-wrapper
> +            ret += mcgen('''
> +    %(cfg)s
> +    %(var_name)s(%(rs_type)s),

    %(cfg)s    %(var_name)s(%(rs_type)s),

> +''',
> +                         cfg=var.ifcond.rsgen(),
> +                         var_name=var_name,
> +                         rs_type=rs_type(c_type, ''))
> +
> +    ret += mcgen('''
> +}
> +''')
> +
> +    ret += gen_rs_variants_to_tag(name, ifcond, variants)
> +
> +    return ret
> +
> +
> +def gen_rs_members(members: List[QAPISchemaObjectTypeMember],
> +                   exclude: List[str] = None) -> str:
> +    exclude = exclude or []
> +    return [f"{m.ifcond.rsgen()} {to_snake_case(rs_name(m.name))}"
> +            for m in members if m.name not in exclude]
> +
> +
> +def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
> +    ret = ''
> +    for memb in members:
> +        typ = rs_type(memb.type.c_type(), '', optional=memb.optional, box=True)
> +        ret += mcgen('''
> +    %(cfg)s
> +    pub %(rs_name)s: %(rs_type)s,

    %(cfg)s    pub %(rs_name)s: %(rs_type)s,

> +''',
> +                     cfg=memb.ifcond.rsgen(),
> +                     rs_type=typ,
> +                     rs_name=to_snake_case(rs_name(memb.name)))
> +    return ret
> +
> +

...

> +%(cfg)s
> +#[repr(u32)]
> +#[derive(Copy, Clone, Debug, PartialEq, Eq, qemu_api_macros::TryInto)]
> +pub enum %(rs_name)s {
> +''',
> +                cfg=ifcond.rsgen(),
> +                rs_name=rs_name(name))
> +
> +    for member in enum_members:
> +        ret += mcgen('''
> +    %(cfg)s
> +    %(c_enum)s,

    %(cfg)s    %(c_enum)s,

> +''',
> +                     cfg=member.ifcond.rsgen(),
> +                     c_enum=to_camel_case(rs_name(member.name)))
> +    # picked the first, since that's what malloc0 does
> +    # but arguably could use _MAX instead, or a qapi annotation
> +    default = to_camel_case(rs_name(enum_members[0].name))
> +    ret += mcgen('''
> +}
> +

...

> +def gen_rs_alternate(name: str,
> +                     ifcond: QAPISchemaIfCond,
> +                     variants: Optional[QAPISchemaVariants]) -> str:
> +    if name in objects_seen:
> +        return ''
> +
> +    ret = ''
> +    objects_seen.add(name)
> +
> +    ret += mcgen('''
> +%(cfg)s
> +#[derive(Clone, Debug)]
> +pub enum %(rs_name)s {
> +''',
> +                 cfg=ifcond.rsgen(),
> +                 rs_name=rs_name(name))
> +
> +    for var in variants.variants:
> +        if var.type.name == 'q_empty':
> +            continue
> +        ret += mcgen('''
> +        %(cfg)s
> +        %(mem_name)s(%(rs_type)s),

    %(cfg)s    %(mem_name)s(%(rs_type)s),

> +''',
> +                     cfg=var.ifcond.rsgen(),
> +                     rs_type=rs_type(var.type.c_unboxed_type(), ''),
> +                     mem_name=to_camel_case(rs_name(var.name)))
> +
> +    ret += mcgen('''
> +}
> +''')
> +    return ret