Re: [Qemu-devel] [PATCH v10 07/25] qapi-visit: Split off visit_type_FOO_fields forward decl

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]

On 10/23/2015 12:05 PM, Markus Armbruster wrote:

>>>>  def gen_visit_struct_fields(name, base, members):
>>>> -    struct_fields_seen.add(name)
>>>> -
>>>>      ret = ''
>>>>
>>>>      if base:
>>>>          ret += gen_visit_implicit_struct(base)
>>>>
>>>> +    struct_fields_seen.add(name)
>>>>      ret += mcgen('''
>>>>
>>>
>>> Minor cleanup not mentioned in commit message.  Okay.
>>
>> Not minor, and I probably should mention it explicitly in the message. I
>> moved it to make sure that gen_visit_implicit_struct() properly emits a
>> forward declaration when necessary; we must not modify
>> struct_fields_seen any sooner than when the next thing in the output
>> stream is either the forward declaration or the implementation.
> 
> The proper place for the .add() is right next to where the thing it
> tracks gets done, no argument.
> 
> I believe your cleanup makes an actual difference only when
> gen_visit_implicit_struct(base) can call gen_visit_struct_fields(name).
> Obviously impossible now, which is why I called it "minor".

Oh, I was probably confusing that the implicit struct is for a different
type (base) than what we are adding to struct_fields_seen (name).

> 
> PATCH 10 will drop the code between the old spot and the new spot.
> Unless something between this patch and PATCH 10 depends on this
> cleanup, I'd simply leave it dirty until then.

Indeed, dropping this hunk and letting subsequent cleanup do it right
makes no difference.  Will add it to my fixup queue.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]