Re: [Qemu-devel] [PATCH RFC v2 08/47] qapi-visit: Fix generated code when schema has forward refs

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 07/01/2015 02:21 PM, Markus Armbruster wrote:
>> The visit_type_implicit_FOO() are generated on demand, right before
>> their first use.  Used by visit_type_STRUCT_fields() when STRUCT has
>> base FOO, and by visit_type_UNION() when flat UNION has member a FOO.
>> 
>> If the schema defines FOO after its first use as struct base or flat
>> union member, visit_type_implicit_FOO() calls
>> visit_type_implicit_FOO() before its definition, which doesn't
>> compile.
>
> None of our public qapi .json files currently do this, so no difference
> to the generated code used in qemu proper.

Yes, we've intentionally avoided forward references.

Nevertheless, the generators either have to catch and reject them, or
make them work.  Making them work is easy, so I did that.

>> Rearrange qapi-schema-test.json to demonstrate the bug.
>
> Indeed, without testsuite exposure, nothing was relying on this fix.
>
>> 
>> Fix by generating the necessary forward declaration.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  scripts/qapi-visit.py                   | 15 ++++++++++++++-
>>  tests/qapi-schema/qapi-schema-test.json | 30 +++++++++++++++++-------------
>>  tests/qapi-schema/qapi-schema-test.out  | 10 +++++-----
>>  3 files changed, 36 insertions(+), 19 deletions(-)
>> 
>
>> +++ b/scripts/qapi-visit.py
>> @@ -17,13 +17,23 @@ from qapi import *
>>  import re
>>  
>>  implicit_structs = []
>> +struct_fields_seen = set()
>>  
>>  def generate_visit_implicit_struct(type):
>>      global implicit_structs
>>      if type in implicit_structs:
>>          return ''
>>      implicit_structs.append(type)
>> -    return mcgen('''
> ...
>> +    ret += mcgen('''
>
> Oddly enough, the ''' is at the same indentation,...
>
>>  
>>  static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, 
>> Error **errp)
>>  {
>> @@ -38,8 +48,11 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, 
>> %(c_type)s **obj, Error *
>>  }
>>  ''',
>>                   c_type=type_name(type))
>> +    return ret
>
> so nothing needed to be reindented here :)
>
> Reviewed-by: Eric Blake <address@hidden>

Thanks!