Hi
On Wed, Feb 6, 2019 at 7:17 PM Markus Armbruster <arm...@redhat.com> wrote:
>
> We neglect to call .visit_module() for the special module we use for
> built-ins. Harmless, but clean it up anyway. The
> tests/qapi-schema/*.out now show the built-in module as 'module None'.
>
> Subclasses of QAPISchemaModularCVisitor need to ._add_module() this
> special module to enable code generation for built-ins. When this
> hasn't been done, QAPISchemaModularCVisitor.visit_module() does
> nothing for the special module. That looks like built-ins could
> accidentally be generated into the wrong module when a subclass
> neglects to call ._add_module(). Can't happen, because built-ins are
> all visited before any other module. But that's non-obvious. Switch
> off code generation explicitly.
>
> Split QAPISchemaModularCVisitor._add_module() into ._add_user_module()
> and ._add_system_module(), for clarity.
That's in next patch.
>
> Rename QAPISchemaModularCVisitor._begin_module() to
> ._begin_user_module().
>
> New QAPISchemaModularCVisitor._is_builtin_module(), for clarity.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> scripts/qapi/commands.py | 2 +-
> scripts/qapi/common.py | 24 ++++++++++++++++++------
> scripts/qapi/events.py | 2 +-
> scripts/qapi/types.py | 2 +-
> scripts/qapi/visit.py | 2 +-
> tests/qapi-schema/comments.out | 1 +
> tests/qapi-schema/doc-bad-section.out | 1 +
> tests/qapi-schema/doc-good.out | 1 +
> tests/qapi-schema/empty.out | 1 +
> tests/qapi-schema/event-case.out | 1 +
> tests/qapi-schema/ident-with-escape.out | 1 +
> tests/qapi-schema/include-relpath.out | 1 +
> tests/qapi-schema/include-repetition.out | 1 +
> tests/qapi-schema/include-simple.out | 1 +
> tests/qapi-schema/indented-expr.out | 1 +
> tests/qapi-schema/qapi-schema-test.out | 1 +
> 16 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 0f3c991918..ebf488953d 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -242,7 +242,7 @@ class
> QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
> self._regy = QAPIGenCCode()
> self._visited_ret_types = {}
>
> - def _begin_module(self, name):
> + def _begin_user_module(self, name):
> self._visited_ret_types[self._genc] = set()
> commands = self._module_basename('qapi-commands', name)
> types = self._module_basename('qapi-types', name)
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index c89edc0cb0..0e3ec598a4 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1868,6 +1868,7 @@ class QAPISchema(object):
> def visit(self, visitor):
> visitor.visit_begin(self)
> module = None
> + visitor.visit_module(module)
> for entity in self._entity_list:
> if visitor.visit_needed(entity):
> if entity.module != module:
> @@ -2321,9 +2322,15 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
> self._what = what
> self._blurb = blurb
> self._pydoc = pydoc
> + self._genc = None
> + self._genh = None
> self._module = {}
> self._main_module = None
>
> + @staticmethod
> + def _is_builtin_module(name):
> + return not name
> +
> def _module_basename(self, what, name):
> if name is None:
> return re.sub(r'-', '-builtin-', what)
> @@ -2334,7 +2341,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
> return basename + '-' + os.path.splitext(os.path.basename(name))[0]
>
> def _add_module(self, name, blurb):
> - if self._main_module is None and name is not None:
> + if self._main_module is None and not self._is_builtin_module(name):
> self._main_module = name
> genc = QAPIGenC(blurb, self._pydoc)
> genh = QAPIGenH(blurb, self._pydoc)
> @@ -2346,22 +2353,27 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
>
> def write(self, output_dir, opt_builtins=False):
> for name in self._module:
> - if name is None and not opt_builtins:
> + if self._is_builtin_module(name) and not opt_builtins:
> continue
> basename = self._module_basename(self._what, name)
> (genc, genh) = self._module[name]
> genc.write(output_dir, basename + '.c')
> genh.write(output_dir, basename + '.h')
>
> - def _begin_module(self, name):
> + def _begin_user_module(self, name):
> pass
>
> def visit_module(self, name):
> if name in self._module:
> self._set_module(name)
> - return
> - self._add_module(name, self._blurb)
> - self._begin_module(name)
> + elif self._is_builtin_module(name):
> + # The built-in module has not been created. No code may
> + # be generated.
> + self._genc = None
> + self._genh = None
> + else:
> + self._add_module(name, self._blurb)
> + self._begin_user_module(name)
>
> def visit_include(self, name, info):
> basename = self._module_basename(self._what, name)
> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
> index d86a2d2b3e..6f39cf8196 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -142,7 +142,7 @@ class
> QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
> self._event_enum_members = []
> self._event_emit_name = c_name(prefix + 'qapi_event_emit')
>
> - def _begin_module(self, name):
> + def _begin_user_module(self, name):
> types = self._module_basename('qapi-types', name)
> visit = self._module_basename('qapi-visit', name)
> self._genc.add(mcgen('''
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index 62d4cf9f95..9fa510f7df 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -194,7 +194,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
> #include "qapi/util.h"
> '''))
>
> - def _begin_module(self, name):
> + def _begin_user_module(self, name):
> types = self._module_basename('qapi-types', name)
> visit = self._module_basename('qapi-visit', name)
> self._genc.preamble_add(mcgen('''
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 82eab72b21..ca86009398 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -298,7 +298,7 @@ class
> QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
> ''',
> prefix=prefix))
>
> - def _begin_module(self, name):
> + def _begin_user_module(self, name):
> types = self._module_basename('qapi-types', name)
> visit = self._module_basename('qapi-visit', name)
> self._genc.preamble_add(mcgen('''
> diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
> index d1abc4b5a1..273f0f54e1 100644
> --- a/tests/qapi-schema/comments.out
> +++ b/tests/qapi-schema/comments.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/doc-bad-section.out
> b/tests/qapi-schema/doc-bad-section.out
> index db8014eed0..367e2a1c3e 100644
> --- a/tests/qapi-schema/doc-bad-section.out
> +++ b/tests/qapi-schema/doc-bad-section.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 21bf345ff0..d3bca343eb 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
> index 5483cb7bc6..5b53d00702 100644
> --- a/tests/qapi-schema/empty.out
> +++ b/tests/qapi-schema/empty.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/event-case.out
> b/tests/qapi-schema/event-case.out
> index f69d4ffe4e..ec8a1406e4 100644
> --- a/tests/qapi-schema/event-case.out
> +++ b/tests/qapi-schema/event-case.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/ident-with-escape.out
> b/tests/qapi-schema/ident-with-escape.out
> index 7f891f7e90..39754eba8c 100644
> --- a/tests/qapi-schema/ident-with-escape.out
> +++ b/tests/qapi-schema/ident-with-escape.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/include-relpath.out
> b/tests/qapi-schema/include-relpath.out
> index 783ccfc855..cf8636b78f 100644
> --- a/tests/qapi-schema/include-relpath.out
> +++ b/tests/qapi-schema/include-relpath.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/include-repetition.out
> b/tests/qapi-schema/include-repetition.out
> index d45977ee56..5423983239 100644
> --- a/tests/qapi-schema/include-repetition.out
> +++ b/tests/qapi-schema/include-repetition.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/include-simple.out
> b/tests/qapi-schema/include-simple.out
> index 1afe20802a..061f81e509 100644
> --- a/tests/qapi-schema/include-simple.out
> +++ b/tests/qapi-schema/include-simple.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/indented-expr.out
> b/tests/qapi-schema/indented-expr.out
> index c0cf3243f3..bffdf6756d 100644
> --- a/tests/qapi-schema/indented-expr.out
> +++ b/tests/qapi-schema/indented-expr.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> diff --git a/tests/qapi-schema/qapi-schema-test.out
> b/tests/qapi-schema/qapi-schema-test.out
> index 9464101d26..d8aec17115 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -1,3 +1,4 @@
> +module None
> object q_empty
> enum QType
> prefix QTYPE
> --
> 2.17.2
>
