[PULL 00/13] QAPI patches patches for 2021-09-03

[PULL 00/13] QAPI patches patches for 2021-09-03

[Markus Armbruster]

The following changes since commit 8880cc4362fde4ecdac0b2092318893118206fcf:

  Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20210902' into staging (2021-09-03 08:27:38 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-09-03

for you to fetch changes up to 34f7b25e575a93182b7c0a3558caac34e26227cf:

  qapi: Tweak error messages for unknown / conflicting 'if' keys (2021-09-03 17:09:10 +0200)

----------------------------------------------------------------
QAPI patches patches for 2021-09-03

----------------------------------------------------------------
Guoyi Tu (1):
      qapi: Set boolean value correctly in examples

Markus Armbruster (12):
      qapi: Simplify QAPISchemaIfCond's interface for generating C
      qapi: Simplify how QAPISchemaIfCond represents "no condition"
      tests/qapi-schema: Correct two 'if' conditionals
      tests/qapi-schema: Demonstrate broken C code for 'if'
      qapi: Fix C code generation for 'if'
      qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
      qapi: Avoid redundant parens in code generated for conditionals
      qapi: Use "not COND" instead of "!COND" for generated documentation
      qapi: Use re.fullmatch() where appropriate
      tests/qapi-schema: Hide OrderedDict in test output
      qapi: Tweak error messages for missing / conflicting meta-type
      qapi: Tweak error messages for unknown / conflicting 'if' keys

 qapi/trace.json                         |  2 +-
 scripts/qapi/common.py                  | 51 +++++++++++++++++++--------------
 scripts/qapi/expr.py                    | 32 +++++++++------------
 scripts/qapi/gen.py                     |  6 ++--
 scripts/qapi/introspect.py              | 11 ++-----
 scripts/qapi/schema.py                  | 12 ++++++--
 scripts/qapi/types.py                   | 28 +++++++-----------
 scripts/qapi/visit.py                   | 14 ++++-----
 tests/qapi-schema/bad-if-key.err        |  2 +-
 tests/qapi-schema/bad-if-keys.err       |  2 +-
 tests/qapi-schema/doc-good.json         |  2 +-
 tests/qapi-schema/doc-good.out          |  6 ++--
 tests/qapi-schema/doc-good.txt          |  8 +++---
 tests/qapi-schema/double-type.err       |  4 +--
 tests/qapi-schema/enum-if-invalid.err   |  2 +-
 tests/qapi-schema/missing-type.err      |  2 +-
 tests/qapi-schema/qapi-schema-test.json |  9 ++++--
 tests/qapi-schema/qapi-schema-test.out  | 31 +++++++++++---------
 tests/qapi-schema/test-qapi.py          | 11 ++++++-
 19 files changed, 121 insertions(+), 114 deletions(-)

-- 
2.31.1

[PULL 01/13] qapi: Set boolean value correctly in examples

[Markus Armbruster]

From: Guoyi Tu <tugy@chinatelecom.cn>

Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
Message-Id: <a21a2b61-2653-a2c9-4478-715e5fb19120@chinatelecom.cn>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/trace.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/trace.json b/qapi/trace.json
index 47c68f04da..eedfded512 100644
--- a/qapi/trace.json
+++ b/qapi/trace.json
@@ -99,7 +99,7 @@
 # Example:
 #
 # -> { "execute": "trace-event-set-state",
-#      "arguments": { "name": "qemu_memalign", "enable": "true" } }
+#      "arguments": { "name": "qemu_memalign", "enable": true } }
 # <- { "return": {} }
 #
 ##
-- 
2.31.1

[PULL 02/13] qapi: Simplify QAPISchemaIfCond's interface for generating C

[Markus Armbruster]

QAPISchemaIfCond.cgen() is only ever used like

    gen_if(ifcond.cgen())

and

    gen_endif(ifcond.cgen())

Simplify to

    ifcond.gen_if()

and

    ifcond.gen_endif()

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-2-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[Import statements tidied up with isort]
---
 scripts/qapi/gen.py        |  6 ++----
 scripts/qapi/introspect.py | 11 +++--------
 scripts/qapi/schema.py     | 10 +++++++++-
 scripts/qapi/types.py      | 28 +++++++++++-----------------
 scripts/qapi/visit.py      | 14 ++++++--------
 5 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 51a597a025..ab26d5c937 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -24,8 +24,6 @@
 from .common import (
     c_fname,
     c_name,
-    gen_endif,
-    gen_if,
     guardend,
     guardstart,
     mcgen,
@@ -95,9 +93,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str:
     if added[0] == '\n':
         out += '\n'
         added = added[1:]
-    out += gen_if(ifcond.cgen())
+    out += ifcond.gen_if()
     out += added
-    out += gen_endif(ifcond.cgen())
+    out += ifcond.gen_endif()
     return out
 
 
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index bd4233ecee..4c079ee627 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -22,12 +22,7 @@
     Union,
 )
 
-from .common import (
-    c_name,
-    gen_endif,
-    gen_if,
-    mcgen,
-)
+from .common import c_name, mcgen
 from .gen import QAPISchemaMonolithicCVisitor
 from .schema import (
     QAPISchema,
@@ -124,10 +119,10 @@ def indent(level: int) -> str:
         if obj.comment:
             ret += indent(level) + f"/* {obj.comment} */\n"
         if obj.ifcond.is_present():
-            ret += gen_if(obj.ifcond.cgen())
+            ret += obj.ifcond.gen_if()
         ret += _tree_to_qlit(obj.value, level)
         if obj.ifcond.is_present():
-            ret += '\n' + gen_endif(obj.ifcond.cgen())
+            ret += '\n' + obj.ifcond.gen_endif()
         return ret
 
     ret = ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 229d24fce9..1451cdec81 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -24,6 +24,8 @@
     c_name,
     cgen_ifcond,
     docgen_ifcond,
+    gen_endif,
+    gen_if,
 )
 from .error import QAPIError, QAPISemError, QAPISourceError
 from .expr import check_exprs
@@ -34,9 +36,15 @@ class QAPISchemaIfCond:
     def __init__(self, ifcond=None):
         self.ifcond = ifcond or {}
 
-    def cgen(self):
+    def _cgen(self):
         return cgen_ifcond(self.ifcond)
 
+    def gen_if(self):
+        return gen_if(self._cgen())
+
+    def gen_endif(self):
+        return gen_endif(self._cgen())
+
     def docgen(self):
         return docgen_ifcond(self.ifcond)
 
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index db9ff95bd1..831294fe42 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -15,13 +15,7 @@
 
 from typing import List, Optional
 
-from .common import (
-    c_enum_const,
-    c_name,
-    gen_endif,
-    gen_if,
-    mcgen,
-)
+from .common import c_enum_const, c_name, mcgen
 from .gen import QAPISchemaModularCVisitor, ifcontext
 from .schema import (
     QAPISchema,
@@ -51,13 +45,13 @@ def gen_enum_lookup(name: str,
 ''',
                 c_name=c_name(name))
     for memb in members:
-        ret += gen_if(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_if()
         index = c_enum_const(name, memb.name, prefix)
         ret += mcgen('''
         [%(index)s] = "%(name)s",
 ''',
                      index=index, name=memb.name)
-        ret += gen_endif(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_endif()
 
     ret += mcgen('''
     },
@@ -81,12 +75,12 @@ def gen_enum(name: str,
                 c_name=c_name(name))
 
     for memb in enum_members:
-        ret += gen_if(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_if()
         ret += mcgen('''
     %(c_enum)s,
 ''',
                      c_enum=c_enum_const(name, memb.name, prefix))
-        ret += gen_endif(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_endif()
 
     ret += mcgen('''
 } %(c_name)s;
@@ -126,7 +120,7 @@ def gen_array(name: str, element_type: QAPISchemaType) -> str:
 def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
     ret = ''
     for memb in members:
-        ret += gen_if(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_if()
         if memb.optional:
             ret += mcgen('''
     bool has_%(c_name)s;
@@ -136,7 +130,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
     %(c_type)s %(c_name)s;
 ''',
                      c_type=memb.type.c_type(), c_name=c_name(memb.name))
-        ret += gen_endif(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_endif()
     return ret
 
 
@@ -159,7 +153,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond,
     ret += mcgen('''
 
 ''')
-    ret += gen_if(ifcond.cgen())
+    ret += ifcond.gen_if()
     ret += mcgen('''
 struct %(c_name)s {
 ''',
@@ -193,7 +187,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond,
     ret += mcgen('''
 };
 ''')
-    ret += gen_endif(ifcond.cgen())
+    ret += ifcond.gen_endif()
 
     return ret
 
@@ -220,13 +214,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str:
     for var in variants.variants:
         if var.type.name == 'q_empty':
             continue
-        ret += gen_if(var.ifcond.cgen())
+        ret += var.ifcond.gen_if()
         ret += mcgen('''
         %(c_type)s %(c_name)s;
 ''',
                      c_type=var.type.c_unboxed_type(),
                      c_name=c_name(var.name))
-        ret += gen_endif(var.ifcond.cgen())
+        ret += var.ifcond.gen_endif()
 
     ret += mcgen('''
     } u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 56ea516399..9d9196a143 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -18,8 +18,6 @@
 from .common import (
     c_enum_const,
     c_name,
-    gen_endif,
-    gen_if,
     indent,
     mcgen,
 )
@@ -79,7 +77,7 @@ def gen_visit_object_members(name: str,
 
     for memb in members:
         deprecated = 'deprecated' in [f.name for f in memb.features]
-        ret += gen_if(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_if()
         if memb.optional:
             ret += mcgen('''
     if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -112,7 +110,7 @@ def gen_visit_object_members(name: str,
             ret += mcgen('''
     }
 ''')
-        ret += gen_endif(memb.ifcond.cgen())
+        ret += memb.ifcond.gen_endif()
 
     if variants:
         tag_member = variants.tag_member
@@ -126,7 +124,7 @@ def gen_visit_object_members(name: str,
         for var in variants.variants:
             case_str = c_enum_const(tag_member.type.name, var.name,
                                     tag_member.type.prefix)
-            ret += gen_if(var.ifcond.cgen())
+            ret += var.ifcond.gen_if()
             if var.type.name == 'q_empty':
                 # valid variant and nothing to do
                 ret += mcgen('''
@@ -142,7 +140,7 @@ def gen_visit_object_members(name: str,
                              case=case_str,
                              c_type=var.type.c_name(), c_name=c_name(var.name))
 
-            ret += gen_endif(var.ifcond.cgen())
+            ret += var.ifcond.gen_endif()
         ret += mcgen('''
     default:
         abort();
@@ -228,7 +226,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
                 c_name=c_name(name))
 
     for var in variants.variants:
-        ret += gen_if(var.ifcond.cgen())
+        ret += var.ifcond.gen_if()
         ret += mcgen('''
     case %(case)s:
 ''',
@@ -254,7 +252,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
         ret += mcgen('''
         break;
 ''')
-        ret += gen_endif(var.ifcond.cgen())
+        ret += var.ifcond.gen_endif()
 
     ret += mcgen('''
     case QTYPE_NONE:
-- 
2.31.1

[PULL 03/13] qapi: Simplify how QAPISchemaIfCond represents "no condition"

[Markus Armbruster]

None works fine, there is no need to replace it by {} in .__init__().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-3-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py | 4 ++--
 scripts/qapi/schema.py | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1724ac32db..1c1dc87ccb 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -200,7 +200,7 @@ def guardend(name: str) -> str:
                  name=c_fname(name).upper())
 
 
-def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
+def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
     if not ifcond:
         return ''
     if isinstance(ifcond, str):
@@ -214,7 +214,7 @@ def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
     return '(' + (') ' + oper + ' (').join(operands) + ')'
 
 
-def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
+def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
     # TODO Doc generated for conditions needs polish
     if not ifcond:
         return ''
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 1451cdec81..3d72c7dfc9 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -34,7 +34,7 @@
 
 class QAPISchemaIfCond:
     def __init__(self, ifcond=None):
-        self.ifcond = ifcond or {}
+        self.ifcond = ifcond
 
     def _cgen(self):
         return cgen_ifcond(self.ifcond)
-- 
2.31.1

[PULL 04/13] tests/qapi-schema: Correct two 'if' conditionals

[Markus Armbruster]

A definition's conditional should imply the conditionals of types it
uses.  If it doesn't, some configurations won't compile.

Example (from tests/qapi-schema/qapi-schema-test.json):

    { 'union': 'TestIfUnion', 'data':
      { 'foo': 'TestStruct',
	'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
      'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }

    { 'command': 'test-if-union-cmd',
      'data': { 'union-cmd-arg': 'TestIfUnion' },
      'if': 'TEST_IF_UNION' }

generates

    #if (defined(TEST_IF_UNION)) && (defined(TEST_IF_STRUCT))
    typedef struct TestIfUnion TestIfUnion;
    #endif /* (defined(TEST_IF_UNION)) && (defined(TEST_IF_STRUCT)) */

and

    #if defined(TEST_IF_UNION)
    void qmp_test_if_union_cmd(TestIfUnion *union_cmd_arg, Error **errp);
    void qmp_marshal_test_if_union_cmd(QDict *args, QObject **ret, Error **errp);
    #endif /* defined(TEST_IF_UNION) */

which doesn't compile when !defined(TEST_IF_STRUCT).

Messed up in f8c4fdd6ae "tests/qapi: Cover commands with 'if' and
union / alternate 'data'", v4.0.0.  Harmless, as we don't actually use
this configuration.  Correct it anyway, along with another instance.

This loses coverage for 'not'.  The next commit will bring it back.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-4-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qapi-schema/qapi-schema-test.json | 5 ++---
 tests/qapi-schema/qapi-schema-test.out  | 8 ++++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index fe028145e4..e20f76d84c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -236,7 +236,7 @@
 
 { 'command': 'test-if-union-cmd',
   'data': { 'union-cmd-arg': 'TestIfUnion' },
-  'if': 'TEST_IF_UNION' }
+  'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
 
 { 'alternate': 'TestIfAlternate', 'data':
   { 'foo': 'int',
@@ -245,8 +245,7 @@
 
 { 'command': 'test-if-alternate-cmd',
   'data': { 'alt-cmd-arg': 'TestIfAlternate' },
-  'if': { 'all': ['TEST_IF_ALT',
-                  {'not': 'TEST_IF_NOT_ALT'}] } }
+  'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
 
 { 'command': 'test-if-cmd',
   'data': {
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 3d0c6a8f28..517d802636 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -321,10 +321,10 @@ object TestIfUnion
     if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
 object q_obj_test-if-union-cmd-arg
     member union-cmd-arg: TestIfUnion optional=False
-    if TEST_IF_UNION
+    if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
 command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
-    if TEST_IF_UNION
+    if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
 alternate TestIfAlternate
     tag type
     case foo: int
@@ -333,10 +333,10 @@ alternate TestIfAlternate
     if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
 object q_obj_test-if-alternate-cmd-arg
     member alt-cmd-arg: TestIfAlternate optional=False
-    if OrderedDict([('all', ['TEST_IF_ALT', OrderedDict([('not', 'TEST_IF_NOT_ALT')])])])
+    if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
 command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
-    if OrderedDict([('all', ['TEST_IF_ALT', OrderedDict([('not', 'TEST_IF_NOT_ALT')])])])
+    if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
 object q_obj_test-if-cmd-arg
     member foo: TestIfStruct optional=False
     member bar: TestIfEnum optional=False
-- 
2.31.1

[PULL 05/13] tests/qapi-schema: Demonstrate broken C code for 'if'

[Markus Armbruster]

The C code generated for 'if' conditionals is incorrectly
parenthesized.  For instance,

    'if': { 'not': { 'any': [ { 'not': 'TEST_IF_EVT' },
			      { 'not': 'TEST_IF_STRUCT' } ] } } }

generates

    #if !(!defined(TEST_IF_EVT)) || (!defined(TEST_IF_STRUCT))

This is wrong.  Correct would be:

    #if !(!defined(TEST_IF_EVT) || !defined(TEST_IF_STRUCT))

Cover the issue in qapi-schema-test.json.  This generates bad #if in
tests/test-qapi-events.h and other files.

Add a similar condition to doc-good.json.  The generated documentation
is fine.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-5-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qapi-schema/doc-good.json         | 2 +-
 tests/qapi-schema/doc-good.out          | 2 +-
 tests/qapi-schema/doc-good.txt          | 2 +-
 tests/qapi-schema/qapi-schema-test.json | 5 +++++
 tests/qapi-schema/qapi-schema-test.out  | 3 +++
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 5e30790730..e0027e4cf6 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -127,7 +127,7 @@
 { 'alternate': 'Alternate',
   'features': [ 'alt-feat' ],
   'data': { 'i': 'int', 'b': 'bool' },
-  'if': { 'not': 'IFNOT' } }
+  'if': { 'not': { 'any': [ 'IFONE', 'IFTWO' ] } } }
 
 ##
 # == Another subsection
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 26d1fa5d28..d72f3047e9 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -51,7 +51,7 @@ alternate Alternate
     tag type
     case i: int
     case b: bool
-    if OrderedDict([('not', 'IFNOT')])
+    if OrderedDict([('not', OrderedDict([('any', ['IFONE', 'IFTWO'])]))])
     feature alt-feat
 object q_obj_cmd-arg
     member arg1: int optional=False
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 5bfe06e14e..85a370831f 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -174,7 +174,7 @@ Features
 If
 ~~
 
-"!IFNOT"
+"!(IFONE or IFTWO)"
 
 
 Another subsection
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index e20f76d84c..6e37758280 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -261,6 +261,11 @@
     'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
   'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
 
+{ 'event': 'TEST_IF_EVENT2', 'data': {},
+  # FIXME C #if generated for this conditional is wrong
+  'if': { 'not': { 'any': [ { 'not': 'TEST_IF_EVT' },
+                            { 'not': 'TEST_IF_STRUCT' } ] } } }
+
 # test 'features'
 
 { 'struct': 'FeatureStruct0',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 517d802636..5d2e830ba2 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -357,6 +357,9 @@ object q_obj_TEST_IF_EVENT-arg
 event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
     boxed=False
     if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
+event TEST_IF_EVENT2 None
+    boxed=False
+    if OrderedDict([('not', OrderedDict([('any', [OrderedDict([('not', 'TEST_IF_EVT')]), OrderedDict([('not', 'TEST_IF_STRUCT')])])]))])
 object FeatureStruct0
     member foo: int optional=False
 object FeatureStruct1
-- 
2.31.1

[PULL 06/13] qapi: Fix C code generation for 'if'

[Markus Armbruster]

When commit 5d83b9a130 "qapi: replace if condition list with dict
{'all': [...]}" made cgen_ifcond() and docgen_ifcond() recursive, it
messed up parenthesises in the former, and got them right in the
latter, as the previous commit demonstrates.

To fix, adopt the latter's working code for the former.  This
generates the correct code from the previous commit's commit message.

Fixes: 5d83b9a130690f879d5f33e991beabe69cb88bc8
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-6-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py                  | 4 ++--
 tests/qapi-schema/qapi-schema-test.json | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1c1dc87ccb..f31e077d7b 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -209,9 +209,9 @@ def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
     oper, operands = next(iter(ifcond.items()))
     if oper == 'not':
         return '!' + cgen_ifcond(operands)
-    oper = {'all': '&&', 'any': '||'}[oper]
+    oper = {'all': ' && ', 'any': ' || '}[oper]
     operands = [cgen_ifcond(o) for o in operands]
-    return '(' + (') ' + oper + ' (').join(operands) + ')'
+    return '(' + oper.join(operands) + ')'
 
 
 def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6e37758280..b6c36a9eee 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -262,7 +262,6 @@
   'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
 
 { 'event': 'TEST_IF_EVENT2', 'data': {},
-  # FIXME C #if generated for this conditional is wrong
   'if': { 'not': { 'any': [ { 'not': 'TEST_IF_EVT' },
                             { 'not': 'TEST_IF_STRUCT' } ] } } }
 
-- 
2.31.1

[PULL 07/13] qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-7-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py | 45 +++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f31e077d7b..df92cff852 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -17,6 +17,7 @@
     Dict,
     Match,
     Optional,
+    Sequence,
     Union,
 )
 
@@ -200,33 +201,37 @@ def guardend(name: str) -> str:
                  name=c_fname(name).upper())
 
 
-def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
+def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
+               cond_fmt: str, not_fmt: str,
+               all_operator: str, any_operator: str) -> str:
+
+    def do_gen(ifcond: Union[str, Dict[str, Any]]):
+        if isinstance(ifcond, str):
+            return cond_fmt % ifcond
+        assert isinstance(ifcond, dict) and len(ifcond) == 1
+        if 'not' in ifcond:
+            return not_fmt % do_gen(ifcond['not'])
+        if 'all' in ifcond:
+            gen = gen_infix(all_operator, ifcond['all'])
+        else:
+            gen = gen_infix(any_operator, ifcond['any'])
+        return gen
+
+    def gen_infix(operator: str, operands: Sequence[Any]) -> str:
+        return '(' + operator.join([do_gen(o) for o in operands]) + ')'
+
     if not ifcond:
         return ''
-    if isinstance(ifcond, str):
-        return 'defined(' + ifcond + ')'
+    return do_gen(ifcond)
 
-    oper, operands = next(iter(ifcond.items()))
-    if oper == 'not':
-        return '!' + cgen_ifcond(operands)
-    oper = {'all': ' && ', 'any': ' || '}[oper]
-    operands = [cgen_ifcond(o) for o in operands]
-    return '(' + oper.join(operands) + ')'
+
+def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
+    return gen_ifcond(ifcond, 'defined(%s)', '!%s', ' && ', ' || ')
 
 
 def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
     # TODO Doc generated for conditions needs polish
-    if not ifcond:
-        return ''
-    if isinstance(ifcond, str):
-        return ifcond
-
-    oper, operands = next(iter(ifcond.items()))
-    if oper == 'not':
-        return '!' + docgen_ifcond(operands)
-    oper = {'all': ' and ', 'any': ' or '}[oper]
-    operands = [docgen_ifcond(o) for o in operands]
-    return '(' + oper.join(operands) + ')'
+    return gen_ifcond(ifcond, '%s', '!%s', ' and ', ' or ')
 
 
 def gen_if(cond: str) -> str:
-- 
2.31.1

[PULL 08/13] qapi: Avoid redundant parens in code generated for conditionals

[Markus Armbruster]

Commit 6cc2e4817f "qapi: introduce QAPISchemaIfCond.cgen()" caused a
minor regression: redundant parenthesis.  Subsequent commits
eliminated of many of them, but not all.  Get rid of the rest now.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-8-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py         | 10 ++++++----
 tests/qapi-schema/doc-good.txt |  6 +++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index df92cff852..c7ccc7cec7 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -205,24 +205,26 @@ def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
                cond_fmt: str, not_fmt: str,
                all_operator: str, any_operator: str) -> str:
 
-    def do_gen(ifcond: Union[str, Dict[str, Any]]):
+    def do_gen(ifcond: Union[str, Dict[str, Any]], need_parens: bool):
         if isinstance(ifcond, str):
             return cond_fmt % ifcond
         assert isinstance(ifcond, dict) and len(ifcond) == 1
         if 'not' in ifcond:
-            return not_fmt % do_gen(ifcond['not'])
+            return not_fmt % do_gen(ifcond['not'], True)
         if 'all' in ifcond:
             gen = gen_infix(all_operator, ifcond['all'])
         else:
             gen = gen_infix(any_operator, ifcond['any'])
+        if need_parens:
+            gen = '(' + gen + ')'
         return gen
 
     def gen_infix(operator: str, operands: Sequence[Any]) -> str:
-        return '(' + operator.join([do_gen(o) for o in operands]) + ')'
+        return operator.join([do_gen(o, True) for o in operands])
 
     if not ifcond:
         return ''
-    return do_gen(ifcond)
+    return do_gen(ifcond, False)
 
 
 def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 85a370831f..75f51a6fc1 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -79,7 +79,7 @@ Members
 If
 ~~
 
-"(IFALL1 and IFALL2)"
+"IFALL1 and IFALL2"
 
 
 "Variant1" (Object)
@@ -120,8 +120,8 @@ Members
 
 The members of "Base"
 The members of "Variant1" when "base1" is ""one""
-The members of "Variant2" when "base1" is ""two"" (**If: **"(IFONE or
-IFTWO)")
+The members of "Variant2" when "base1" is ""two"" (**If: **"IFONE or
+IFTWO")
 
 Features
 ~~~~~~~~
-- 
2.31.1

[PULL 09/13] qapi: Use "not COND" instead of "!COND" for generated documentation

[Markus Armbruster]

Generated documentation uses operators "and", "or", and "!".  Change
the latter to "not".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-9-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py         | 2 +-
 tests/qapi-schema/doc-good.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c7ccc7cec7..5f8f76e5b2 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -233,7 +233,7 @@ def cgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
 
 def docgen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]]) -> str:
     # TODO Doc generated for conditions needs polish
-    return gen_ifcond(ifcond, '%s', '!%s', ' and ', ' or ')
+    return gen_ifcond(ifcond, '%s', 'not %s', ' and ', ' or ')
 
 
 def gen_if(cond: str) -> str:
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 75f51a6fc1..0c59d75964 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -174,7 +174,7 @@ Features
 If
 ~~
 
-"!(IFONE or IFTWO)"
+"not (IFONE or IFTWO)"
 
 
 Another subsection
-- 
2.31.1

[PULL 10/13] qapi: Use re.fullmatch() where appropriate

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-10-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/expr.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 019f4c97aa..9e2aa1d43a 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -275,7 +275,7 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
 
     def _check_if(cond: Union[str, object]) -> None:
         if isinstance(cond, str):
-            if not re.match(r'^[A-Z][A-Z0-9_]*$', cond):
+            if not re.fullmatch(r'[A-Z][A-Z0-9_]*', cond):
                 raise QAPISemError(
                     info,
                     "'if' condition '%s' of %s is not a valid identifier"
-- 
2.31.1

[PULL 11/13] tests/qapi-schema: Hide OrderedDict in test output

[Markus Armbruster]

Since commit 5d83b9a130 "qapi: replace if condition list with dict
{'all': [...]}", we represent if conditionals as trees consisting of
OrderedDict, list and str.  This results in less than legible test
output.  For instance:

    if OrderedDict([('not', OrderedDict([('any', [OrderedDict([('not', 'TEST_IF_EVT')]), OrderedDict([('not', 'TEST_IF_STRUCT')])])]))])

We intend to replace OrderedDict by dict when we get Python 3.7, which
will result in more legible output:

    if {'not': {'any': [{'not': 'TEST_IF_EVT'}, {'not': 'TEST_IF_STRUCT'}]}}

Can't wait: put in a hack to get that now, with a comment to revert it
when we replace OrderedDict.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-11-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qapi-schema/doc-good.out         |  6 +++---
 tests/qapi-schema/qapi-schema-test.out | 30 +++++++++++++-------------
 tests/qapi-schema/test-qapi.py         | 11 +++++++++-
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index d72f3047e9..478fe6f82e 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -18,7 +18,7 @@ enum Enum
     feature enum-feat
 object Base
     member base1: Enum optional=False
-    if OrderedDict([('all', ['IFALL1', 'IFALL2'])])
+    if {'all': ['IFALL1', 'IFALL2']}
 object Variant1
     member var1: str optional=False
         if IFSTR
@@ -30,7 +30,7 @@ object Object
     tag base1
     case one: Variant1
     case two: Variant2
-        if OrderedDict([('any', ['IFONE', 'IFTWO'])])
+        if {'any': ['IFONE', 'IFTWO']}
     feature union-feat1
 object q_obj_Variant1-wrapper
     member data: Variant1 optional=False
@@ -51,7 +51,7 @@ alternate Alternate
     tag type
     case i: int
     case b: bool
-    if OrderedDict([('not', OrderedDict([('any', ['IFONE', 'IFTWO'])]))])
+    if {'not': {'any': ['IFONE', 'IFTWO']}}
     feature alt-feat
 object q_obj_cmd-arg
     member arg1: int optional=False
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 5d2e830ba2..d557fe2d89 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -311,40 +311,40 @@ enum TestIfUnionKind
     member foo
     member bar
         if TEST_IF_UNION_BAR
-    if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
 object TestIfUnion
     member type: TestIfUnionKind optional=False
     tag type
     case foo: q_obj_TestStruct-wrapper
     case bar: q_obj_str-wrapper
         if TEST_IF_UNION_BAR
-    if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
 object q_obj_test-if-union-cmd-arg
     member union-cmd-arg: TestIfUnion optional=False
-    if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
 command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
-    if OrderedDict([('all', ['TEST_IF_UNION', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT']}
 alternate TestIfAlternate
     tag type
     case foo: int
     case bar: TestStruct
         if TEST_IF_ALT_BAR
-    if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
 object q_obj_test-if-alternate-cmd-arg
     member alt-cmd-arg: TestIfAlternate optional=False
-    if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
 command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
-    if OrderedDict([('all', ['TEST_IF_ALT', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT']}
 object q_obj_test-if-cmd-arg
     member foo: TestIfStruct optional=False
     member bar: TestIfEnum optional=False
         if TEST_IF_CMD_BAR
-    if OrderedDict([('all', ['TEST_IF_CMD', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT']}
 command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree
     gen=True success_response=True boxed=False oob=False preconfig=False
-    if OrderedDict([('all', ['TEST_IF_CMD', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_CMD', 'TEST_IF_STRUCT']}
 command test-cmd-return-def-three None -> UserDefThree
     gen=True success_response=True boxed=False oob=False preconfig=False
 array TestIfEnumList TestIfEnum
@@ -353,13 +353,13 @@ object q_obj_TEST_IF_EVENT-arg
     member foo: TestIfStruct optional=False
     member bar: TestIfEnumList optional=False
         if TEST_IF_EVT_BAR
-    if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT']}
 event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
     boxed=False
-    if OrderedDict([('all', ['TEST_IF_EVT', 'TEST_IF_STRUCT'])])
+    if {'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT']}
 event TEST_IF_EVENT2 None
     boxed=False
-    if OrderedDict([('not', OrderedDict([('any', [OrderedDict([('not', 'TEST_IF_EVT')]), OrderedDict([('not', 'TEST_IF_STRUCT')])])]))])
+    if {'not': {'any': [{'not': 'TEST_IF_EVT'}, {'not': 'TEST_IF_STRUCT'}]}}
 object FeatureStruct0
     member foo: int optional=False
 object FeatureStruct1
@@ -392,11 +392,11 @@ object CondFeatureStruct2
 object CondFeatureStruct3
     member foo: int optional=False
     feature feature1
-        if OrderedDict([('all', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
+        if {'all': ['TEST_IF_COND_1', 'TEST_IF_COND_2']}
 object CondFeatureStruct4
     member foo: int optional=False
     feature feature1
-        if OrderedDict([('any', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
+        if {'any': ['TEST_IF_COND_1', 'TEST_IF_COND_2']}
 enum FeatureEnum1
     member eins
     member zwei
@@ -447,7 +447,7 @@ command test-command-cond-features2 None -> None
 command test-command-cond-features3 None -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
     feature feature1
-        if OrderedDict([('all', ['TEST_IF_COND_1', 'TEST_IF_COND_2'])])
+        if {'all': ['TEST_IF_COND_1', 'TEST_IF_COND_2']}
 event TEST_EVENT_FEATURES0 FeatureStruct1
     boxed=False
 event TEST_EVENT_FEATURES1 None
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index c92be2d086..73cffae2b6 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -94,8 +94,17 @@ def _print_variants(variants):
 
     @staticmethod
     def _print_if(ifcond, indent=4):
+        # TODO Drop this hack after replacing OrderedDict by plain
+        # dict (requires Python 3.7)
+        def _massage(subcond):
+            if isinstance(subcond, str):
+                return subcond
+            if isinstance(subcond, list):
+                return [_massage(val) for val in subcond]
+            return {key: _massage(val) for key, val in subcond.items()}
+
         if ifcond.is_present():
-            print('%sif %s' % (' ' * indent, ifcond.ifcond))
+            print('%sif %s' % (' ' * indent, _massage(ifcond.ifcond)))
 
     @classmethod
     def _print_features(cls, features, indent=4):
-- 
2.31.1

[PULL 12/13] qapi: Tweak error messages for missing / conflicting meta-type

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-12-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/expr.py               | 23 +++++++++--------------
 tests/qapi-schema/double-type.err  |  4 +---
 tests/qapi-schema/missing-type.err |  2 +-
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 9e2aa1d43a..ae4437ba08 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -630,20 +630,15 @@ def check_exprs(exprs: List[_JSONObject]) -> List[_JSONObject]:
         if 'include' in expr:
             continue
 
-        if 'enum' in expr:
-            meta = 'enum'
-        elif 'union' in expr:
-            meta = 'union'
-        elif 'alternate' in expr:
-            meta = 'alternate'
-        elif 'struct' in expr:
-            meta = 'struct'
-        elif 'command' in expr:
-            meta = 'command'
-        elif 'event' in expr:
-            meta = 'event'
-        else:
-            raise QAPISemError(info, "expression is missing metatype")
+        metas = expr.keys() & {'enum', 'struct', 'union', 'alternate',
+                               'command', 'event'}
+        if len(metas) != 1:
+            raise QAPISemError(
+                info,
+                "expression must have exactly one key"
+                " 'enum', 'struct', 'union', 'alternate',"
+                " 'command', 'event'")
+        meta = metas.pop()
 
         check_name_is_str(expr[meta], info, "'%s'" % meta)
         name = cast(str, expr[meta])
diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err
index 576e716197..6a1e8a5990 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -1,3 +1 @@
-double-type.json: In struct 'Bar':
-double-type.json:2: struct has unknown key 'command'
-Valid keys are 'base', 'data', 'features', 'if', 'struct'.
+double-type.json:2: expression must have exactly one key 'enum', 'struct', 'union', 'alternate', 'command', 'event'
diff --git a/tests/qapi-schema/missing-type.err b/tests/qapi-schema/missing-type.err
index 5755386a18..cb39569e49 100644
--- a/tests/qapi-schema/missing-type.err
+++ b/tests/qapi-schema/missing-type.err
@@ -1 +1 @@
-missing-type.json:2: expression is missing metatype
+missing-type.json:2: expression must have exactly one key 'enum', 'struct', 'union', 'alternate', 'command', 'event'
-- 
2.31.1

[PULL 13/13] qapi: Tweak error messages for unknown / conflicting 'if' keys

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-13-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/expr.py                  | 7 +++----
 tests/qapi-schema/bad-if-key.err      | 2 +-
 tests/qapi-schema/bad-if-keys.err     | 2 +-
 tests/qapi-schema/enum-if-invalid.err | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index ae4437ba08..b62f0a3640 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -286,13 +286,12 @@ def _check_if(cond: Union[str, object]) -> None:
             raise QAPISemError(
                 info,
                 "'if' condition of %s must be a string or an object" % source)
+        check_keys(cond, info, "'if' condition of %s" % source, [],
+                   ["all", "any", "not"])
         if len(cond) != 1:
             raise QAPISemError(
                 info,
-                "'if' condition dict of %s must have one key: "
-                "'all', 'any' or 'not'" % source)
-        check_keys(cond, info, "'if' condition", [],
-                   ["all", "any", "not"])
+                "'if' condition of %s has conflicting keys" % source)
 
         oper, operands = next(iter(cond.items()))
         if not operands:
diff --git a/tests/qapi-schema/bad-if-key.err b/tests/qapi-schema/bad-if-key.err
index a69dc9ee86..38cf44b687 100644
--- a/tests/qapi-schema/bad-if-key.err
+++ b/tests/qapi-schema/bad-if-key.err
@@ -1,3 +1,3 @@
 bad-if-key.json: In struct 'TestIfStruct':
-bad-if-key.json:2: 'if' condition has unknown key 'value'
+bad-if-key.json:2: 'if' condition of struct has unknown key 'value'
 Valid keys are 'all', 'any', 'not'.
diff --git a/tests/qapi-schema/bad-if-keys.err b/tests/qapi-schema/bad-if-keys.err
index aceb31dc6d..fe87bd30ac 100644
--- a/tests/qapi-schema/bad-if-keys.err
+++ b/tests/qapi-schema/bad-if-keys.err
@@ -1,2 +1,2 @@
 bad-if-keys.json: In struct 'TestIfStruct':
-bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all', 'any' or 'not'
+bad-if-keys.json:2: 'if' condition of struct has conflicting keys
diff --git a/tests/qapi-schema/enum-if-invalid.err b/tests/qapi-schema/enum-if-invalid.err
index 3bb84075a9..2b2bbffb65 100644
--- a/tests/qapi-schema/enum-if-invalid.err
+++ b/tests/qapi-schema/enum-if-invalid.err
@@ -1,3 +1,3 @@
 enum-if-invalid.json: In enum 'TestIfEnum':
-enum-if-invalid.json:2: 'if' condition has unknown key 'val'
+enum-if-invalid.json:2: 'if' condition of 'data' member 'bar' has unknown key 'val'
 Valid keys are 'all', 'any', 'not'.
-- 
2.31.1

Re: [PULL 00/13] QAPI patches patches for 2021-09-03

[Peter Maydell]

On Fri, 3 Sept 2021 at 20:32, Markus Armbruster <armbru@redhat.com> wrote:
>
> The following changes since commit 8880cc4362fde4ecdac0b2092318893118206fcf:
>
>   Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20210902' into staging (2021-09-03 08:27:38 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-09-03
>
> for you to fetch changes up to 34f7b25e575a93182b7c0a3558caac34e26227cf:
>
>   qapi: Tweak error messages for unknown / conflicting 'if' keys (2021-09-03 17:09:10 +0200)
>
> ----------------------------------------------------------------
> QAPI patches patches for 2021-09-03
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.2
for any user-visible changes.

-- PMM