qapi: replace if condition list with dict {'all': [...]}

Replace the simple list sugar form with a recursive structure that will
accept other operators in the following commits (all, any or not).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210804083105.97531-7-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Accidental code motion undone.  Degenerate :forms: comment dropped.
Helper _check_if() moved.  Error messages tweaked.  ui.json updated.
Accidental changes to qapi-schema-test.json dropped.]
Signed-off-by: Markus Armbruster <armbru@redhat.com>

qapi/ui.json

@@ -1136,7 +1136,8 @@
     { 'name': 'gtk', 'if': 'defined(CONFIG_GTK)' },
     { 'name': 'sdl', 'if': 'defined(CONFIG_SDL)' },
     { 'name': 'egl-headless',
-              'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' },
+              'if': { 'all': [ 'defined(CONFIG_OPENGL)',
+                               'defined(CONFIG_GBM)' ] } },
     { 'name': 'curses', 'if': 'defined(CONFIG_CURSES)' },
     { 'name': 'cocoa', 'if': 'defined(CONFIG_COCOA)' },
     { 'name': 'spice-app', 'if': 'defined(CONFIG_SPICE)'} ] }
@@ -1167,7 +1168,8 @@
       'gtk': { 'type': 'DisplayGTK', 'if': 'defined(CONFIG_GTK)' },
       'curses': { 'type': 'DisplayCurses', 'if': 'defined(CONFIG_CURSES)' },
       'egl-headless': { 'type': 'DisplayEGLHeadless',
-                        'if': 'defined(CONFIG_OPENGL) && defined(CONFIG_GBM)' }
+                        'if': { 'all': [ 'defined(CONFIG_OPENGL)',
+                                         'defined(CONFIG_GBM)' ] } }
   }
 }
 

scripts/qapi/common.py

@@ -13,7 +13,8 @@
 
 import re
 from typing import (
-    List,
+    Any,
+    Dict,
     Match,
     Optional,
     Union,
@@ -199,17 +200,29 @@ def guardend(name: str) -> str:
                  name=c_fname(name).upper())
 
 
-def cgen_ifcond(ifcond: Union[str, List[str]]) -> str:
+def cgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
     if not ifcond:
         return ''
-    return '(' + ') && ('.join(ifcond) + ')'
+    if isinstance(ifcond, str):
+        return ifcond
+
+    oper, operands = next(iter(ifcond.items()))
+    oper = {'all': '&&'}[oper]
+    operands = [cgen_ifcond(o) for o in operands]
+    return '(' + (') ' + oper + ' (').join(operands) + ')'
 
 
-def docgen_ifcond(ifcond: Union[str, List[str]]) -> str:
+def docgen_ifcond(ifcond: Union[str, Dict[str, Any]]) -> str:
     # TODO Doc generated for conditions needs polish
     if not ifcond:
         return ''
-    return ' and '.join(ifcond)
+    if isinstance(ifcond, str):
+        return ifcond
+
+    oper, operands = next(iter(ifcond.items()))
+    oper = {'all': ' and '}[oper]
+    operands = [docgen_ifcond(o) for o in operands]
+    return '(' + oper.join(operands) + ')'
 
 
 def gen_if(cond: str) -> str:

scripts/qapi/expr.py

@@ -259,14 +259,9 @@ def check_flags(expr: _JSONObject, info: QAPISourceInfo) -> None:
 
 def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
     """
-    Normalize and validate the ``if`` member of an object.
+    Validate the ``if`` member of an object.
 
-    The ``if`` member may be either a ``str`` or a ``List[str]``.
+    The ``if`` member may be either a ``str`` or a dict.
-    A ``str`` value will be normalized to ``List[str]``.
-
-    :forms:
-      :sugared: ``Union[str, List[str]]``
-      :canonical: ``List[str]``
 
     :param expr: The expression containing the ``if`` member to validate.
     :param info: QAPI schema source file information.
@@ -275,31 +270,46 @@ def check_if(expr: _JSONObject, info: QAPISourceInfo, source: str) -> None:
     :raise QAPISemError:
         When the "if" member fails validation, or when there are no
         non-empty conditions.
-    :return: None, ``expr`` is normalized in-place as needed.
+    :return: None
     """
+
+    def _check_if(cond: Union[str, object]) -> None:
+        if isinstance(cond, str):
+            if not cond.strip():
+                raise QAPISemError(
+                    info,
+                    "'if' condition '%s' of %s makes no sense"
+                    % (cond, source))
+            return
+
+        if not isinstance(cond, dict):
+            raise QAPISemError(
+                info,
+                "'if' condition of %s must be a string or an object" % source)
+        if len(cond) != 1:
+            raise QAPISemError(
+                info,
+                "'if' condition dict of %s must have one key: "
+                "'all'" % source)
+        check_keys(cond, info, "'if' condition", [],
+                   ["all"])
+
+        oper, operands = next(iter(cond.items()))
+        if not operands:
+            raise QAPISemError(
+                info, "'if' condition [] of %s is useless" % source)
+
+        if oper in ("all") and not isinstance(operands, list):
+            raise QAPISemError(
+                info, "'%s' condition of %s must be an array" % (oper, source))
+        for operand in operands:
+            _check_if(operand)
+
     ifcond = expr.get('if')
     if ifcond is None:
         return
 
-    if isinstance(ifcond, list):
+    _check_if(ifcond)
-        if not ifcond:
-            raise QAPISemError(
-                info, "'if' condition [] of %s is useless" % source)
-    else:
-        # Normalize to a list
-        ifcond = expr['if'] = [ifcond]
-
-    for elt in ifcond:
-        if not isinstance(elt, str):
-            raise QAPISemError(
-                info,
-                "'if' condition of %s must be a string or a list of strings"
-                % source)
-        if not elt.strip():
-            raise QAPISemError(
-                info,
-                "'if' condition '%s' of %s makes no sense"
-                % (elt, source))
 
 
 def normalize_members(members: object) -> None:

scripts/qapi/schema.py

@@ -32,7 +32,7 @@ from .parser import QAPISchemaParser
 
 class QAPISchemaIfCond:
     def __init__(self, ifcond=None):
-        self.ifcond = ifcond or []
+        self.ifcond = ifcond or {}
 
     def cgen(self):
         return cgen_ifcond(self.ifcond)

tests/qapi-schema/bad-if-all.err

@@ -0,0 +1,2 @@
+bad-if-all.json: In struct 'TestIfStruct':
+bad-if-all.json:2: 'all' condition of struct must be an array

tests/qapi-schema/bad-if-all.json

@@ -0,0 +1,3 @@
+# check 'if all' is not a list
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': { 'all': 'ALL' } }

tests/qapi-schema/bad-if-empty-list.json

@@ -1,3 +1,3 @@
 # check empty 'if' list
 { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
-  'if': [] }
+  'if': { 'all': [] } }

tests/qapi-schema/bad-if-key.err

@@ -0,0 +1,3 @@
+bad-if-key.json: In struct 'TestIfStruct':
+bad-if-key.json:2: 'if' condition has unknown key 'value'
+Valid keys are 'all'.

tests/qapi-schema/bad-if-key.json

@@ -0,0 +1,3 @@
+# check unknown 'if' dict key
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': { 'value': 'defined(TEST_IF_STRUCT)' } }

tests/qapi-schema/bad-if-keys.err

@@ -0,0 +1,2 @@
+bad-if-keys.json: In struct 'TestIfStruct':
+bad-if-keys.json:2: 'if' condition dict of struct must have one key: 'all'

tests/qapi-schema/bad-if-keys.json

@@ -0,0 +1,3 @@
+# check multiple 'if' keys
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': { 'any': ['ANY'], 'all': ['ALL'] } }

tests/qapi-schema/bad-if-list.json

@@ -1,3 +1,3 @@
 # check invalid 'if' content
 { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
-  'if': ['foo', ' '] }
+  'if': { 'all': ['foo', ' '] } }

tests/qapi-schema/bad-if.err

@@ -1,2 +1,2 @@
 bad-if.json: In struct 'TestIfStruct':
-bad-if.json:2: 'if' condition of struct must be a string or a list of strings
+bad-if.json:2: 'if' condition of struct must be a string or an object

tests/qapi-schema/bad-if.json

@@ -1,3 +1,3 @@
 # check invalid 'if' type
 { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
-  'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
+  'if': ['defined(TEST_IF_STRUCT)'] }

tests/qapi-schema/doc-good.json

@@ -70,7 +70,8 @@
 # @base1:
 # the first member
 ##
-{ 'struct': 'Base', 'data': { 'base1': 'Enum' } }
+{ 'struct': 'Base', 'data': { 'base1': 'Enum' },
+  'if': { 'all': ['IFALL1', 'IFALL2'] } }
 
 ##
 # @Variant1:

tests/qapi-schema/doc-good.out

@@ -12,15 +12,16 @@ enum QType
 module doc-good.json
 enum Enum
     member one
-        if ['defined(IFONE)']
+        if defined(IFONE)
     member two
-    if ['defined(IFCOND)']
+    if defined(IFCOND)
     feature enum-feat
 object Base
     member base1: Enum optional=False
+    if OrderedDict([('all', ['IFALL1', 'IFALL2'])])
 object Variant1
     member var1: str optional=False
-        if ['defined(IFSTR)']
+        if defined(IFSTR)
         feature member-feat
     feature variant1-feat
 object Variant2
@@ -29,7 +30,7 @@ object Object
     tag base1
     case one: Variant1
     case two: Variant2
-        if ['IFTWO']
+        if IFTWO
     feature union-feat1
 object q_obj_Variant1-wrapper
     member data: Variant1 optional=False
@@ -38,13 +39,13 @@ object q_obj_Variant2-wrapper
 enum SugaredUnionKind
     member one
     member two
-        if ['IFTWO']
+        if IFTWO
 object SugaredUnion
     member type: SugaredUnionKind optional=False
     tag type
     case one: q_obj_Variant1-wrapper
     case two: q_obj_Variant2-wrapper
-        if ['IFTWO']
+        if IFTWO
     feature union-feat2
 alternate Alternate
     tag type

tests/qapi-schema/doc-good.txt

@@ -76,6 +76,12 @@ Members
    the first member
 
 
+If
+~~
+
+"(IFALL1 and IFALL2)"
+
+
 "Variant1" (Object)
 -------------------
 

tests/qapi-schema/enum-if-invalid.err

@@ -1,2 +1,3 @@
 enum-if-invalid.json: In enum 'TestIfEnum':
-enum-if-invalid.json:2: 'if' condition of 'data' member 'bar' must be a string or a list of strings
+enum-if-invalid.json:2: 'if' condition has unknown key 'val'
+Valid keys are 'all'.

tests/qapi-schema/features-if-invalid.err

@@ -1,2 +1,2 @@
 features-if-invalid.json: In struct 'Stru':
-features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or a list of strings
+features-if-invalid.json:2: 'if' condition of 'features' member 'f' must be a string or an object

tests/qapi-schema/meson.build

@@ -37,8 +37,11 @@ schemas = [
   'bad-data.json',
   'bad-ident.json',
   'bad-if.json',
+  'bad-if-all.json',
   'bad-if-empty.json',
   'bad-if-empty-list.json',
+  'bad-if-key.json',
+  'bad-if-keys.json',
   'bad-if-list.json',
   'bad-type-bool.json',
   'bad-type-dict.json',

tests/qapi-schema/qapi-schema-test.json

@@ -232,7 +232,7 @@
 { 'union': 'TestIfUnion', 'data':
   { 'foo': 'TestStruct',
     'bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
-  'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
+  'if': { 'all': ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'] } }
 
 { 'command': 'test-if-union-cmd',
   'data': { 'union-cmd-arg': 'TestIfUnion' },
@@ -241,25 +241,25 @@
 { 'alternate': 'TestIfAlternate', 'data':
   { 'foo': 'int',
     'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
-  'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
+  'if': { 'all': ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'] } }
 
 { 'command': 'test-if-alternate-cmd',
   'data': { 'alt-cmd-arg': 'TestIfAlternate' },
-  'if': 'defined(TEST_IF_ALT)' }
+  'if': { 'all': ['defined(TEST_IF_ALT)'] } }
 
 { 'command': 'test-if-cmd',
   'data': {
     'foo': 'TestIfStruct',
     'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
   'returns': 'UserDefThree',
-  'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
+  'if': { 'all': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] } }
 
 { 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
 
 { 'event': 'TEST_IF_EVENT', 'data':
   { 'foo': 'TestIfStruct',
     'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
-  'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
+  'if': { 'all': ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'] } }
 
 # test 'features'
 
@@ -288,8 +288,9 @@
                 { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
 { 'struct': 'CondFeatureStruct3',
   'data': { 'foo': 'int' },
-  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
+  'features': [ { 'name': 'feature1',
-                                              'defined(TEST_IF_COND_2)'] } ] }
+                  'if': { 'all': [ 'defined(TEST_IF_COND_1)',
+                                   'defined(TEST_IF_COND_2)'] } } ] }
 
 { 'enum': 'FeatureEnum1',
   'data': [ 'eins', 'zwei', 'drei' ],
@@ -328,8 +329,9 @@
   'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
                 { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
 { 'command': 'test-command-cond-features3',
-  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
+  'features': [ { 'name': 'feature1',
-                                              'defined(TEST_IF_COND_2)'] } ] }
+                  'if': { 'all': [ 'defined(TEST_IF_COND_1)',
+                                   'defined(TEST_IF_COND_2)'] } } ] }
 
 { 'event': 'TEST_EVENT_FEATURES0',
   'data': 'FeatureStruct1' }

tests/qapi-schema/qapi-schema-test.out

@@ -298,65 +298,65 @@ command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Unio
 object TestIfStruct
     member foo: int optional=False
     member bar: int optional=False
-        if ['defined(TEST_IF_STRUCT_BAR)']
+        if defined(TEST_IF_STRUCT_BAR)
-    if ['defined(TEST_IF_STRUCT)']
+    if defined(TEST_IF_STRUCT)
 enum TestIfEnum
     member foo
     member bar
-        if ['defined(TEST_IF_ENUM_BAR)']
+        if defined(TEST_IF_ENUM_BAR)
-    if ['defined(TEST_IF_ENUM)']
+    if defined(TEST_IF_ENUM)
 object q_obj_TestStruct-wrapper
     member data: TestStruct optional=False
 enum TestIfUnionKind
     member foo
     member bar
-        if ['defined(TEST_IF_UNION_BAR)']
+        if defined(TEST_IF_UNION_BAR)
-    if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(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 ['defined(TEST_IF_UNION_BAR)']
+        if defined(TEST_IF_UNION_BAR)
-    if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_UNION)', 'defined(TEST_IF_STRUCT)'])])
 object q_obj_test-if-union-cmd-arg
     member union-cmd-arg: TestIfUnion optional=False
-    if ['defined(TEST_IF_UNION)']
+    if defined(TEST_IF_UNION)
 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 ['defined(TEST_IF_UNION)']
+    if defined(TEST_IF_UNION)
 alternate TestIfAlternate
     tag type
     case foo: int
     case bar: TestStruct
-        if ['defined(TEST_IF_ALT_BAR)']
+        if defined(TEST_IF_ALT_BAR)
-    if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_ALT)', 'defined(TEST_IF_STRUCT)'])])
 object q_obj_test-if-alternate-cmd-arg
     member alt-cmd-arg: TestIfAlternate optional=False
-    if ['defined(TEST_IF_ALT)']
+    if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
 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 ['defined(TEST_IF_ALT)']
+    if OrderedDict([('all', ['defined(TEST_IF_ALT)'])])
 object q_obj_test-if-cmd-arg
     member foo: TestIfStruct optional=False
     member bar: TestIfEnum optional=False
-        if ['defined(TEST_IF_CMD_BAR)']
+        if defined(TEST_IF_CMD_BAR)
-    if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(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 ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_CMD)', 'defined(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
-    if ['defined(TEST_IF_ENUM)']
+    if defined(TEST_IF_ENUM)
 object q_obj_TEST_IF_EVENT-arg
     member foo: TestIfStruct optional=False
     member bar: TestIfEnumList optional=False
-        if ['defined(TEST_IF_EVT_BAR)']
+        if defined(TEST_IF_EVT_BAR)
-    if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
 event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
     boxed=False
-    if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+    if OrderedDict([('all', ['defined(TEST_IF_EVT)', 'defined(TEST_IF_STRUCT)'])])
 object FeatureStruct0
     member foo: int optional=False
 object FeatureStruct1
@@ -379,17 +379,17 @@ object FeatureStruct4
 object CondFeatureStruct1
     member foo: int optional=False
     feature feature1
-        if ['defined(TEST_IF_FEATURE_1)']
+        if defined(TEST_IF_FEATURE_1)
 object CondFeatureStruct2
     member foo: int optional=False
     feature feature1
-        if ['defined(TEST_IF_FEATURE_1)']
+        if defined(TEST_IF_FEATURE_1)
     feature feature2
-        if ['defined(TEST_IF_FEATURE_2)']
+        if defined(TEST_IF_FEATURE_2)
 object CondFeatureStruct3
     member foo: int optional=False
     feature feature1
-        if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
+        if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
 enum FeatureEnum1
     member eins
     member zwei
@@ -429,17 +429,17 @@ command test-command-features3 None -> None
 command test-command-cond-features1 None -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
     feature feature1
-        if ['defined(TEST_IF_FEATURE_1)']
+        if defined(TEST_IF_FEATURE_1)
 command test-command-cond-features2 None -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
     feature feature1
-        if ['defined(TEST_IF_FEATURE_1)']
+        if defined(TEST_IF_FEATURE_1)
     feature feature2
-        if ['defined(TEST_IF_FEATURE_2)']
+        if defined(TEST_IF_FEATURE_2)
 command test-command-cond-features3 None -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
     feature feature1
-        if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
+        if OrderedDict([('all', ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'])])
 event TEST_EVENT_FEATURES0 FeatureStruct1
     boxed=False
 event TEST_EVENT_FEATURES1 None

tests/qapi-schema/struct-member-if-invalid.err

@@ -1,2 +1,2 @@
 struct-member-if-invalid.json: In struct 'Stru':
-struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or a list of strings
+struct-member-if-invalid.json:2: 'if' condition of 'data' member 'member' must be a string or an object

tests/qapi-schema/union-branch-if-invalid.json

@@ -3,4 +3,4 @@
 { 'struct': 'Stru', 'data': { 'member': 'str' } }
 { 'union': 'Uni',
   'base': { 'tag': 'Branches' }, 'discriminator': 'tag',
-  'data': { 'branch1': { 'type': 'Stru', 'if': [''] } } }
+  'data': { 'branch1': { 'type': 'Stru', 'if': { 'all': [''] } } } }