Re: [Qemu-devel] [PATCH v2 15/29] qapi: Record 'include' directives in parse tree

[Markus Armbruster <armbru@redhat.com>]

Michael Roth <mdroth@linux.vnet.ibm.com> writes:

> Quoting Markus Armbruster (2018-02-11 03:35:53)
>> The parse tree is a list of expressions.  Except include expressions
>> currently get replaced by the included file's parse tree.
>> 
>> Instead of throwing away the include expression, keep it with the file
>> name expanded so you don't have to track the including file's
>> directory to make sense of it.
>> 
>> A future commit will put this include expression to use.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  scripts/qapi/common.py | 21 +++++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>> 
>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
>> index cc5a5941dd..6d49709784 100644
>> --- a/scripts/qapi/common.py
>> +++ b/scripts/qapi/common.py
>> @@ -290,8 +290,11 @@ class QAPISchemaParser(object):
>>                  if not isinstance(include, str):
>>                      raise QAPISemError(info,
>>                                         "Value of 'include' must be a string")
>> -                exprs_include = self._include(include, info,
>> -                                              os.path.dirname(self.fname),
>> +                incl_fname = os.path.join(os.path.dirname(self.fname),
>> +                                          include)
>> +                self.exprs.append({'expr': {'include': incl_fname},
>> +                                   'info': info})
>> +                exprs_include = self._include(include, info, incl_fname,
>>                                                previously_included)
>>                  if exprs_include:
>>                      self.exprs.extend(exprs_include.exprs)
>> @@ -326,8 +329,7 @@ class QAPISchemaParser(object):
>>                  "Documentation for '%s' is not followed by the definition"
>>                  % doc.symbol)
>> 
>> -    def _include(self, include, info, base_dir, previously_included):
>> -        incl_fname = os.path.join(base_dir, include)
>> +    def _include(self, include, info, incl_fname, previously_included):
>>          incl_abs_fname = os.path.abspath(incl_fname)
>>          # catch inclusion cycle
>>          inf = info
>> @@ -893,6 +895,9 @@ def check_exprs(exprs):
>>          info = expr_elem['info']
>>          doc = expr_elem.get('doc')
>> 
>> +        if 'include' in expr:
>> +            continue
>> +
>>          if not doc and doc_required:
>>              raise QAPISemError(info,
>>                                 "Expression missing documentation comment")
>> @@ -931,6 +936,9 @@ def check_exprs(exprs):
>> 
>>      # Try again for hidden UnionKind enum
>>      for expr_elem in exprs:
>> +        if 'include' in expr:
>> +            continue
>> +
>
> Wouldn't this ^
>
>>          expr = expr_elem['expr']
>
> Need to come after this?

D'oh!

>>          if 'union' in expr and not discriminator_find_enum_define(expr):
>>              name = '%sKind' % expr['union']
>> @@ -943,6 +951,9 @@ def check_exprs(exprs):
>> 
>>      # Validate that exprs make sense
>>      for expr_elem in exprs:
>> +        if 'include' in expr:
>> +            continue
>> +
>>          expr = expr_elem['expr']
>
> And here.

Right.  I wonder how this passes tests.

>>          info = expr_elem['info']
>>          doc = expr_elem.get('doc')
>> @@ -1667,6 +1678,8 @@ class QAPISchema(object):
>>                  self._def_command(expr, info, doc)
>>              elif 'event' in expr:
>>                  self._def_event(expr, info, doc)
>> +            elif 'include' in expr:
>> +                pass
>>              else:
>>                  assert False
>> 
>> -- 
>> 2.13.6
>>