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
signature.asc
Description: OpenPGP digital signature
