Re: [RFC PATCH 15/18] qapi: Support empty modules

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> If you added an include file that doesn't contain any definitions, no
> source files would be generated for it. However, in other source files,
> you would still get an #include for the header files of the empty
> module.

Bug.

Cause: we generate #include module.h always, and the module.h when
visiting its first definition.  If there are no definitions, we don't.

> The intended behaviour is that empty source files are created for empty
> modules.

Yes.

>          This patch makes QAPISchema keep a list of all modules
> (including empty ones) and modifies visit() to first visit all modules
> in that list.

Minimally invasive fix.  Backends still initialize module output on
first visit_module(), but now all modules are visited upfront.

Separating "initialize module" from "switch to module" might be easier
to understand.  Idea, not demand.

> Some test reference outputs need to be updated due to the additional
> visitor calls.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  scripts/qapi/schema.py                   | 9 +++++++++
>  tests/qapi-schema/comments.out           | 2 ++
>  tests/qapi-schema/doc-bad-section.out    | 2 ++
>  tests/qapi-schema/doc-good.out           | 2 ++
>  tests/qapi-schema/empty.out              | 2 ++
>  tests/qapi-schema/event-case.out         | 2 ++
>  tests/qapi-schema/include-repetition.out | 4 ++++
>  tests/qapi-schema/include-simple.out     | 3 +++
>  tests/qapi-schema/indented-expr.out      | 2 ++
>  tests/qapi-schema/qapi-schema-test.out   | 4 ++++
>  10 files changed, 32 insertions(+)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 38041098bd..e1b034d67d 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -749,6 +749,7 @@ class QAPISchema(object):
>          self.docs = parser.docs
>          self._entity_list = []
>          self._entity_dict = {}
> +        self._modules = [os.path.basename(fname)]
>          self._predefining = True
>          self._def_predefineds()
>          self._predefining = False
> @@ -800,6 +801,8 @@ class QAPISchema(object):
>              main_info = main_info.parent
>          fname = os.path.relpath(include, os.path.dirname(main_info.fname))
>          self._def_entity(QAPISchemaInclude(fname, info))
> +        if fname not in self._modules:
> +            self._modules.append(fname)
>  
>      def _def_builtin_type(self, name, json_type, c_type):
>          self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
> @@ -1033,6 +1036,12 @@ class QAPISchema(object):
>          visitor.visit_begin(self)
>          module = None
>          visitor.visit_module(module)
> +
> +        # Make sure that all modules are visited, even if they contain no
> +        # entities
> +        for module in self._modules:
> +            visitor.visit_module(module)
> +

Slightly neater, I think:

           visitor.visit_begin(self)
  +
  +        # Visit all modules, to ensure @visitor sees them
  +        for module in self._modules:
  +            visitor.visit_module(module)
  +
           module = None
           visitor.visit_module(module)

This way, we keep starting with module None rather than whatever user
module comes last.  The .out diffs below then don't add a nother "module
None" line.

>          for entity in self._entity_list:
>              if visitor.visit_needed(entity):
>                  if entity.module != module:
> diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
> index 273f0f54e1..fa7e95d1cc 100644
> --- a/tests/qapi-schema/comments.out
> +++ b/tests/qapi-schema/comments.out
> @@ -1,4 +1,6 @@
>  module None
> +module comments.json
> +module None
>  object q_empty
>  enum QType
>      prefix QTYPE
[...]