Message ID | 20170911110623.24981-29-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | Hi, | expand |
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > scripts/qapi.py | 9 ++++++++- > tests/qapi-schema/qapi-schema-test.json | 6 +++++- > tests/qapi-schema/qapi-schema-test.out | 8 +++++++- > 3 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 2f14edfa1f..19e48bd4d2 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -849,6 +849,9 @@ def check_alternate(expr, info): for (key, value) in members.items(): check_name(info, "Member of alternate '%s'" % name, key) # Ensure alternates have no type conflicts. > check_type(info, "Member '%s' of alternate '%s'" % (key, name), > value, > allow_metas=['built-in', 'union', 'struct', 'enum']) > + if isinstance(value, dict): > + check_unknown_keys(info, value, {'type', 'if'}) > + value = value['type'] > qtype = find_alternate_member_qtype(value) > if not qtype: > raise QAPISemError(info, "Alternate '%s' member '%s' cannot use " I stared at this some because I couldn't see the check_if(). Looks like it's done in check_type(). I guess I'm hitting the limits of what I can do in *patch* review, and to do better, I'd have to go through the resulting code with a fine comb. You generalize the right hand side from "TYPE" to {"type": "TYPE", "if: "IFCOND"}, where "if" is optional. The old "TYPE" becomes syntactic sugar for {"type": "TYPE"}. Two remarks: * Since the right hand side is more than just a type, the name check_type() is now inappropriate. Since PATCH 23. * The sane way to do syntactic sugar is to desugar at the first opportunity. You do the opposite: you handle the new aspect of the general form first, and then rewrite it to the sugared form for further processing, most probably to reduce churn. Reducing churn is good, but I find the resulting code hard to follow, because @value changes meaning halfway through. May well apply to the other similar patches, too. > @@ -1671,7 +1674,11 @@ class QAPISchema(object): > None)) > > def _make_variant(self, case, typ): > - return QAPISchemaObjectTypeVariant(case, typ) > + ifcond = None > + if isinstance(typ, dict): > + ifcond = typ.get('if') > + typ = typ['type'] > + return QAPISchemaObjectTypeVariant(case, typ, ifcond) > > def _make_simple_variant(self, case, typ, info): > ifcond = None > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json > index 895e80a978..f7a62b24d1 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -208,9 +208,13 @@ > { 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' }, > 'if': 'defined(TEST_IF_UNION)' } > > -{ 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' }, > +{ 'alternate': 'TestIfAlternate', 'data': > + { 'foo': 'int', 'alt_bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} }, > 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' } > > +{ 'command': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 'TestIfAlternate' }, > + 'if': 'defined(TEST_IF_ALT)' } > + > { 'command': 'TestIfCmd', 'data': > { 'foo': 'TestIfStruct', > 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } }, > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out > index ee009c5626..6887e49c9b 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -67,8 +67,11 @@ enum QType > alternate TestIfAlternate > tag type > case foo: int > - case bar: TestStruct > + case alt_bar: TestStruct if=defined(TEST_IF_ALT_BAR) > if defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT) > +command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None > + gen=True success_response=True boxed=False > + if defined(TEST_IF_ALT) > command TestIfCmd q_obj_TestIfCmd-arg -> None > gen=True success_response=True boxed=False > if defined(TEST_IF_CMD) && defined(TEST_IF_STRUCT) > @@ -232,6 +235,9 @@ object q_obj_EVENT_D-arg > member b: str optional=False > member c: str optional=True > member enum3: EnumOne optional=True > +object q_obj_TestIfAlternateCmd-arg > + member alt_cmd_arg: TestIfAlternate optional=False > + if defined(TEST_IF_ALT) > object q_obj_TestIfCmd-arg > member foo: TestIfStruct optional=False > member bar: TestIfEnum optional=False if=defined(TEST_IF_CMD_BAR)
diff --git a/scripts/qapi.py b/scripts/qapi.py index 2f14edfa1f..19e48bd4d2 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -849,6 +849,9 @@ def check_alternate(expr, info): check_type(info, "Member '%s' of alternate '%s'" % (key, name), value, allow_metas=['built-in', 'union', 'struct', 'enum']) + if isinstance(value, dict): + check_unknown_keys(info, value, {'type', 'if'}) + value = value['type'] qtype = find_alternate_member_qtype(value) if not qtype: raise QAPISemError(info, "Alternate '%s' member '%s' cannot use " @@ -1671,7 +1674,11 @@ class QAPISchema(object): None)) def _make_variant(self, case, typ): - return QAPISchemaObjectTypeVariant(case, typ) + ifcond = None + if isinstance(typ, dict): + ifcond = typ.get('if') + typ = typ['type'] + return QAPISchemaObjectTypeVariant(case, typ, ifcond) def _make_simple_variant(self, case, typ, info): ifcond = None diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 895e80a978..f7a62b24d1 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -208,9 +208,13 @@ { 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' }, 'if': 'defined(TEST_IF_UNION)' } -{ 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' }, +{ 'alternate': 'TestIfAlternate', 'data': + { 'foo': 'int', 'alt_bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} }, 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' } +{ 'command': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 'TestIfAlternate' }, + 'if': 'defined(TEST_IF_ALT)' } + { 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct', 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } }, diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index ee009c5626..6887e49c9b 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -67,8 +67,11 @@ enum QType alternate TestIfAlternate tag type case foo: int - case bar: TestStruct + case alt_bar: TestStruct if=defined(TEST_IF_ALT_BAR) if defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT) +command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None + gen=True success_response=True boxed=False + if defined(TEST_IF_ALT) command TestIfCmd q_obj_TestIfCmd-arg -> None gen=True success_response=True boxed=False if defined(TEST_IF_CMD) && defined(TEST_IF_STRUCT) @@ -232,6 +235,9 @@ object q_obj_EVENT_D-arg member b: str optional=False member c: str optional=True member enum3: EnumOne optional=True +object q_obj_TestIfAlternateCmd-arg + member alt_cmd_arg: TestIfAlternate optional=False + if defined(TEST_IF_ALT) object q_obj_TestIfCmd-arg member foo: TestIfStruct optional=False member bar: TestIfEnum optional=False if=defined(TEST_IF_CMD_BAR)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/qapi.py | 9 ++++++++- tests/qapi-schema/qapi-schema-test.json | 6 +++++- tests/qapi-schema/qapi-schema-test.out | 8 +++++++- 3 files changed, 20 insertions(+), 3 deletions(-)