On Tue, Mar 18, 2025 at 10:46:10AM +0800, Zhao Liu wrote:
> Date: Tue, 18 Mar 2025 10:46:10 +0800
> From: Zhao Liu <zhao1....@intel.com>
> Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
>  vmsd in vmstate_struct macro
> 
> On Mon, Mar 17, 2025 at 06:17:07PM +0100, Paolo Bonzini wrote:
> > Date: Mon, 17 Mar 2025 18:17:07 +0100
> > From: Paolo Bonzini <pbonz...@redhat.com>
> > Subject: Re: [PATCH 04/17] rust/vmstate: Use ident instead of expr to parse
> >  vmsd in vmstate_struct macro
> > 
> > On Mon, Mar 17, 2025 at 3:52 PM Zhao Liu <zhao1....@intel.com> wrote:
> > >
> > > When specify an array field in vmstate_struct macro, there will be an
> > > error:
> > >
> > > > local ambiguity when calling macro `vmstate_struct`: multiple parsing
> > > > options: built-in NTs expr ('vmsd') or 1 other option.
> > >
> > > This is because "expr" can't recognize the "vmsd" field correctly, so
> > > that it gets confused with the previous array field.
> > >
> > > To fix the above issue, use "ident" for "vmsd" field, and explicitly
> > > refer to it in the macro.
> > 
> > I think this is not needed if the varray case is left as is, and other
> > cases use .with_...() instead of arguments?
> > 
> 
> Yes! With a[0..num], the `vmsd` could be parsed correctly. I'll drop this
> patch as well and refresh unit tests.
> 

Additionally, at present, IMO it is not suitable to replace the vmsd argument
with .with_vmsd() method because VMS_STRUCT requires a vmsd field, and
.with_vmsd() is optional.

There is no way to ensure that vmsd is not omitted... unless VMS_STRUCT is
also set within .with_vmsd(). This would be akin to merging vmstate_struct
and vmstate_of, but I suppose that would be a long way as for now.

So I prefer vmsd argument at the moment :-)

Thanks,
Zhao



Reply via email to