| Message ID | 20200915224027.2529813-16-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
John Snow <jsnow@redhat.com> writes: > Including it in common.py creates a circular import dependency, because > schema relies on common.py. To type build_params properly, it needs to > be moved outside of the chain. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/commands.py | 2 +- > scripts/qapi/common.py | 23 ----------------------- > scripts/qapi/events.py | 2 +- > scripts/qapi/params.py | 40 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 42 insertions(+), 25 deletions(-) > create mode 100644 scripts/qapi/params.py Ugh. Would moving it gen.py work?
On 9/17/20 10:42 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> Including it in common.py creates a circular import dependency, because >> schema relies on common.py. To type build_params properly, it needs to >> be moved outside of the chain. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> scripts/qapi/commands.py | 2 +- >> scripts/qapi/common.py | 23 ----------------------- >> scripts/qapi/events.py | 2 +- >> scripts/qapi/params.py | 40 ++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 42 insertions(+), 25 deletions(-) >> create mode 100644 scripts/qapi/params.py > > Ugh. > > Would moving it gen.py work? > Actually, yes. I have an experimental patch way, way later in the series that does this: 1. Leaves common.py with *just* functions and constants used by schema.py: c_name, POINTER_SUFFIX, and transitively EATSPACE. 2. Splits gen_c.py out of gen.py, putting all of the C-specific generator classes in there. 3. Adds params.py and the C-specific bits of common.py into gen_c.py. In effect, you get gen_c.py with all of the C-specific bits in it, all of the other code-generation modules import from gen_c (marking them obviously as C code generators), and schema.py and other parsing friends import only the tiny common.py for c_name(). I'll adjust this patch to stash this in gen.py for now. It's too disruptive to shift the other refactor around in my stack. --js
On 9/17/20 10:42 AM, Markus Armbruster wrote: > Ugh. > > Would moving it gen.py work? Done.
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index 2e4b4de0fa..0c0fe854fe 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -14,10 +14,10 @@ """ from .common import ( - build_params, c_name, mcgen, ) +from .params import build_params from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 38d380f0a9..0b1af694e6 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -209,26 +209,3 @@ def gen_endif(ifcond: Sequence[str]) -> str: #endif /* %(cond)s */ ''', cond=ifc) return ret - - -def build_params(arg_type, - boxed: bool, - extra: Optional[str] = None) -> str: - ret = '' - sep = '' - if boxed: - assert arg_type - ret += '%s arg' % arg_type.c_param_type() - sep = ', ' - elif arg_type: - assert not arg_type.variants - for memb in arg_type.members: - ret += sep - sep = ', ' - if memb.optional: - ret += 'bool has_%s, ' % c_name(memb.name) - ret += '%s %s' % (memb.type.c_param_type(), - c_name(memb.name)) - if extra: - ret += sep + extra - return ret if ret else 'void' diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index 6b3afa14d7..75eda72534 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -13,11 +13,11 @@ """ from .common import ( - build_params, c_enum_const, c_name, mcgen, ) +from .params import build_params from .gen import QAPISchemaModularCVisitor, ifcontext from .schema import QAPISchemaEnumMember from .types import gen_enum, gen_enum_lookup diff --git a/scripts/qapi/params.py b/scripts/qapi/params.py new file mode 100644 index 0000000000..4d4b02f60d --- /dev/null +++ b/scripts/qapi/params.py @@ -0,0 +1,40 @@ +# +# QAPI helper library +# +# Copyright IBM, Corp. 2011 +# Copyright (c) 2013-2018 Red Hat Inc. +# +# Authors: +# Anthony Liguori <aliguori@us.ibm.com> +# Markus Armbruster <armbru@redhat.com> +# +# This work is licensed under the terms of the GNU GPL, version 2. +# See the COPYING file in the top-level directory. + +from typing import Optional + +from .common import c_name +from .schema import QAPISchemaObjectType + + +def build_params(arg_type: Optional[QAPISchemaObjectType], + boxed: bool, + extra: Optional[str] = None) -> str: + ret = '' + sep = '' + if boxed: + assert arg_type + ret += '%s arg' % arg_type.c_param_type() + sep = ', ' + elif arg_type: + assert not arg_type.variants + for memb in arg_type.members: + ret += sep + sep = ', ' + if memb.optional: + ret += 'bool has_%s, ' % c_name(memb.name) + ret += '%s %s' % (memb.type.c_param_type(), + c_name(memb.name)) + if extra: + ret += sep + extra + return ret if ret else 'void'
Including it in common.py creates a circular import dependency, because schema relies on common.py. To type build_params properly, it needs to be moved outside of the chain. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/commands.py | 2 +- scripts/qapi/common.py | 23 ----------------------- scripts/qapi/events.py | 2 +- scripts/qapi/params.py | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 25 deletions(-) create mode 100644 scripts/qapi/params.py