qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-4.0 v7 13/27] qapi: add a dictionary form fo

From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PATCH for-4.0 v7 13/27] qapi: add a dictionary form for TYPE
Date: Tue, 11 Dec 2018 16:11:00 +0400 
Hi

On Mon, Dec 10, 2018 at 9:26 PM Markus Armbruster <address@hidden> wrote:
>
> Marc-André Lureau <address@hidden> writes:
>
> > Wherever a struct/union/alternate/command/event member with NAME: TYPE
> > form is accepted, desugar it to a NAME: { 'type': TYPE } form.
> >
> > This will allow to add new member details, such as 'if' in the
> > following patch to introduce conditionals, or 'default' for default
> > values etc.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  scripts/qapi/common.py                        | 71 ++++++++++++++-----
> >  tests/Makefile.include                        |  6 ++
> >  tests/qapi-schema/alternate-invalid-dict.err  |  1 +
> >  tests/qapi-schema/alternate-invalid-dict.exit |  1 +
> >  tests/qapi-schema/alternate-invalid-dict.json |  4 ++
> >  tests/qapi-schema/alternate-invalid-dict.out  |  0
> >  .../qapi-schema/event-member-invalid-dict.err |  1 +
> >  .../event-member-invalid-dict.exit            |  1 +
> >  .../event-member-invalid-dict.json            |  2 +
> >  .../qapi-schema/event-member-invalid-dict.out |  0
> >  tests/qapi-schema/event-nest-struct.json      |  2 +-
> >  .../flat-union-inline-invalid-dict.err        |  1 +
> >  .../flat-union-inline-invalid-dict.exit       |  1 +
> >  .../flat-union-inline-invalid-dict.json       | 11 +++
> >  .../flat-union-inline-invalid-dict.out        |  0
> >  tests/qapi-schema/flat-union-inline.json      |  2 +-
> >  .../nested-struct-data-invalid-dict.err       |  1 +
> >  .../nested-struct-data-invalid-dict.exit      |  1 +
> >  .../nested-struct-data-invalid-dict.json      |  3 +
> >  .../nested-struct-data-invalid-dict.out       |  0
> >  tests/qapi-schema/nested-struct-data.json     |  2 +-
> >  tests/qapi-schema/qapi-schema-test.json       | 10 +--
> >  .../struct-member-invalid-dict.err            |  1 +
> >  .../struct-member-invalid-dict.exit           |  1 +
> >  .../struct-member-invalid-dict.json           |  3 +
> >  .../struct-member-invalid-dict.out            |  0
> >  .../qapi-schema/union-branch-invalid-dict.err |  1 +
> >  .../union-branch-invalid-dict.exit            |  1 +
> >  .../union-branch-invalid-dict.json            |  4 ++
> >  .../qapi-schema/union-branch-invalid-dict.out |  0
> >  30 files changed, 106 insertions(+), 26 deletions(-)
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/event-member-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.out
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 95e55b3f44..4b3ba53dc7 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -588,11 +588,13 @@ def discriminator_find_enum_define(expr):
> >      if not base_members:
> >          return None
> >
> > -    discriminator_type = base_members.get(discriminator)
> > -    if not discriminator_type:
> > +    discriminator_value = base_members.get(discriminator)
> > +    if not discriminator_value:
> >          return None
> >
> > -    return enum_types.get(discriminator_type)
> > +    if isinstance(discriminator_value, dict):
> > +        discriminator_value = discriminator_value['type']
> > +    return enum_types.get(discriminator_value)
>
> As in PATCH 08, this is slightly more complicated than v6 because @expr
> isn't normalized.  I can't say offhand whether normalizing in the place
> I suggested in my review of v6 would avoid this complication.
>
> >
> >
> >  # Names must be letters, numbers, -, and _.  They must start with letter,
> > @@ -660,6 +662,15 @@ def check_if(expr, info):
> >          check_if_str(ifcond, info)
> >
> >
> > +def normalize_members(expr, field):
> > +    members = expr.get(field)
> > +    if isinstance(members, OrderedDict):
> > +        for key, arg in members.items():
> > +            if isinstance(arg, dict):
> > +                continue
> > +            members[key] = {'type': arg}
> > +
> > +
> >  def check_type(info, source, value, allow_array=False,
> >                 allow_implicit=False, allow_optional=False,
> >                 allow_metas=[]):
> > @@ -704,8 +715,14 @@ def check_type(info, source, value, allow_array=False,
> >                                 % (source, key))
> >          # Todo: allow dictionaries to represent default values of
> >          # an optional argument.
> > -        check_type(info, "Member '%s' of %s" % (key, source), arg,
> > -                   allow_array=True,
> > +        if isinstance(arg, dict):
> > +            check_known_keys(info, "member '%s' of %s" % (key, source),
> > +                             arg, ['type'], [])
> > +            typ = arg['type']
> > +        else:
> > +            typ = arg
> > +        check_type(info, "Member '%s' of %s" % (key, source),
> > +                   typ, allow_array=True,
> >                     allow_metas=['built-in', 'union', 'alternate', 'struct',
> >                                  'enum'])
> >
>
> Slightly more complicated than v6, this time because members of the
> implicit struct type aren't normalized, yet.
>
> > @@ -776,13 +793,15 @@ def check_union(expr, info):
> >          # member of the base struct.
> >          check_name(info, "Discriminator of flat union '%s'" % name,
> >                     discriminator)
> > -        discriminator_type = base_members.get(discriminator)
> > -        if not discriminator_type:
> > +        discriminator_value = base_members.get(discriminator)
> > +        if not discriminator_value:
> >              raise QAPISemError(info,
> >                                 "Discriminator '%s' is not a member of base 
> > "
> >                                 "struct '%s'"
> >                                 % (discriminator, base))
> > -        enum_define = enum_types.get(discriminator_type)
> > +        if isinstance(discriminator_value, dict):
> > +            discriminator_value = discriminator_value['type']
> > +        enum_define = enum_types.get(discriminator_value)
> >          allow_metas = ['struct']
> >          # Do not allow string discriminator
> >          if not enum_define:
> > @@ -795,10 +814,16 @@ def check_union(expr, info):
> >          raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % 
> > name)
> >      for (key, value) in members.items():
> >          check_name(info, "Member of union '%s'" % name, key)
> > +        if isinstance(value, dict):
> > +            check_known_keys(info, "member '%s' of union '%s'" % (key, 
> > name),
> > +                             value, ['type'], [])
> > +            typ = value['type']
> > +        else:
> > +            typ = value
>
> Likewise.
>
> >
> >          # Each value must name a known type
> >          check_type(info, "Member '%s' of union '%s'" % (key, name),
> > -                   value, allow_array=not base, allow_metas=allow_metas)
> > +                   typ, allow_array=not base, allow_metas=allow_metas)
> >
> >          # If the discriminator names an enum type, then all members
> >          # of 'data' must also be members of the enum type.
> > @@ -822,18 +847,24 @@ def check_alternate(expr, info):
> >                             "in 'data'" % name)
> >      for (key, value) in members.items():
> >          check_name(info, "Member of alternate '%s'" % name, key)
> > +        if isinstance(value, dict):
> > +            check_known_keys(info,
> > +                             "member '%s' of alternate '%s'" % (key, name),
> > +                             value, ['type'], [])
> > +            typ = value['type']
> > +        else:
> > +            typ = value
>
> Likewise.
>
> >
> >          # Ensure alternates have no type conflicts.
> > -        check_type(info, "Member '%s' of alternate '%s'" % (key, name),
> > -                   value,
> > +        check_type(info, "Member '%s' of alternate '%s'" % (key, name), 
> > typ,
> >                     allow_metas=['built-in', 'union', 'struct', 'enum'])
> > -        qtype = find_alternate_member_qtype(value)
> > +        qtype = find_alternate_member_qtype(typ)
> >          if not qtype:
> >              raise QAPISemError(info, "Alternate '%s' member '%s' cannot 
> > use "
> > -                               "type '%s'" % (name, key, value))
> > +                               "type '%s'" % (name, key, typ))
> >          conflicting = set([qtype])
> >          if qtype == 'QTYPE_QSTRING':
> > -            enum_expr = enum_types.get(value)
> > +            enum_expr = enum_types.get(typ)
> >              if enum_expr:
> >                  for v in enum_get_names(enum_expr):
> >                      if v in ['on', 'off']:
> > @@ -1035,6 +1066,10 @@ def normalize_exprs(exprs):
> >          expr = expr_elem['expr']
> >          if 'enum' in expr:
> >              normalize_enum(expr)
> > +        elif 'union' in expr:
> > +            normalize_members(expr, 'base')
> > +        if {'union', 'alternate', 'struct', 'command', 'event'} & 
> > set(expr):
> > +            normalize_members(expr, 'data')
> >
> >      return exprs
> >
> > @@ -1738,7 +1773,7 @@ class QAPISchema(object):
> >          return QAPISchemaObjectTypeMember(name, typ, optional)
> >
> >      def _make_members(self, data, info):
> > -        return [self._make_member(key, value, info)
> > +        return [self._make_member(key, value['type'], info)
> >                  for (key, value) in data.items()]
> >
> >      def _def_struct_type(self, expr, info, doc):
> > @@ -1774,11 +1809,11 @@ class QAPISchema(object):
> >                  name, info, doc, ifcond,
> >                  'base', self._make_members(base, info))
> >          if tag_name:
> > -            variants = [self._make_variant(key, value)
> > +            variants = [self._make_variant(key, value['type'])
> >                          for (key, value) in data.items()]
> >              members = []
> >          else:
> > -            variants = [self._make_simple_variant(key, value, info)
> > +            variants = [self._make_simple_variant(key, value['type'], info)
> >                          for (key, value) in data.items()]
> >              typ = self._make_implicit_enum_type(name, info, ifcond,
> >                                                  [v.name for v in variants])
> > @@ -1794,7 +1829,7 @@ class QAPISchema(object):
> >          name = expr['alternate']
> >          data = expr['data']
> >          ifcond = expr.get('if')
> > -        variants = [self._make_variant(key, value)
> > +        variants = [self._make_variant(key, value['type'])
> >                      for (key, value) in data.items()]
> >          tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
> >          self._def_entity(
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 3c9eea27fd..ea5d1e8787 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -318,6 +318,7 @@ qapi-schema += alternate-conflict-string.json
> >  qapi-schema += alternate-conflict-bool-string.json
> >  qapi-schema += alternate-conflict-num-string.json
> >  qapi-schema += alternate-empty.json
> > +qapi-schema += alternate-invalid-dict.json
> >  qapi-schema += alternate-nested.json
> >  qapi-schema += alternate-unknown.json
> >  qapi-schema += args-alternate.json
> > @@ -394,6 +395,7 @@ qapi-schema += escape-too-big.json
> >  qapi-schema += escape-too-short.json
> >  qapi-schema += event-boxed-empty.json
> >  qapi-schema += event-case.json
> > +qapi-schema += event-member-invalid-dict.json
> >  qapi-schema += event-nest-struct.json
> >  qapi-schema += flat-union-array-branch.json
> >  qapi-schema += flat-union-bad-base.json
> > @@ -403,6 +405,7 @@ qapi-schema += flat-union-base-union.json
> >  qapi-schema += flat-union-clash-member.json
> >  qapi-schema += flat-union-empty.json
> >  qapi-schema += flat-union-inline.json
> > +qapi-schema += flat-union-inline-invalid-dict.json
> >  qapi-schema += flat-union-int-branch.json
> >  qapi-schema += flat-union-invalid-branch-key.json
> >  qapi-schema += flat-union-invalid-discriminator.json
> > @@ -430,6 +433,7 @@ qapi-schema += missing-comma-list.json
> >  qapi-schema += missing-comma-object.json
> >  qapi-schema += missing-type.json
> >  qapi-schema += nested-struct-data.json
> > +qapi-schema += nested-struct-data-invalid-dict.json
> >  qapi-schema += non-objects.json
> >  qapi-schema += oob-test.json
> >  qapi-schema += allow-preconfig-test.json
> > @@ -460,6 +464,7 @@ qapi-schema += returns-whitelist.json
> >  qapi-schema += struct-base-clash-deep.json
> >  qapi-schema += struct-base-clash.json
> >  qapi-schema += struct-data-invalid.json
> > +qapi-schema += struct-member-invalid-dict.json
> >  qapi-schema += struct-member-invalid.json
> >  qapi-schema += trailing-comma-list.json
> >  qapi-schema += trailing-comma-object.json
> > @@ -471,6 +476,7 @@ qapi-schema += unicode-str.json
> >  qapi-schema += union-base-empty.json
> >  qapi-schema += union-base-no-discriminator.json
> >  qapi-schema += union-branch-case.json
> > +qapi-schema += union-branch-invalid-dict.json
> >  qapi-schema += union-clash-branches.json
> >  qapi-schema += union-empty.json
> >  qapi-schema += union-invalid-base.json
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.err 
> > b/tests/qapi-schema/alternate-invalid-dict.err
> > new file mode 100644
> > index 0000000000..631d46628e
> > --- /dev/null
> > +++ b/tests/qapi-schema/alternate-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/alternate-invalid-dict.json:2: Key 'type' is missing 
> > from member 'two' of alternate 'Alt'
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.exit 
> > b/tests/qapi-schema/alternate-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/alternate-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.json 
> > b/tests/qapi-schema/alternate-invalid-dict.json
> > new file mode 100644
> > index 0000000000..8e0b2ac287
> > --- /dev/null
> > +++ b/tests/qapi-schema/alternate-invalid-dict.json
> > @@ -0,0 +1,4 @@
> > +# exploded member form must have a 'type'
> > +{ 'alternate': 'Alt',
> > +  'data': { 'one': 'str',
> > +            'two': { 'if': 'foo' } } }
> > diff --git a/tests/qapi-schema/alternate-invalid-dict.out 
> > b/tests/qapi-schema/alternate-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.err 
> > b/tests/qapi-schema/event-member-invalid-dict.err
> > new file mode 100644
> > index 0000000000..1a57fa29b0
> > --- /dev/null
> > +++ b/tests/qapi-schema/event-member-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/event-member-invalid-dict.json:1: Key 'type' is missing 
> > from member 'a' of 'data' for event 'EVENT_A'
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.exit 
> > b/tests/qapi-schema/event-member-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/event-member-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.json 
> > b/tests/qapi-schema/event-member-invalid-dict.json
> > new file mode 100644
> > index 0000000000..ee6f3ecb6f
> > --- /dev/null
> > +++ b/tests/qapi-schema/event-member-invalid-dict.json
> > @@ -0,0 +1,2 @@
> > +{ 'event': 'EVENT_A',
> > +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/event-member-invalid-dict.out 
> > b/tests/qapi-schema/event-member-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/event-nest-struct.json 
> > b/tests/qapi-schema/event-nest-struct.json
> > index ee6f3ecb6f..355ddaeff1 100644
> > --- a/tests/qapi-schema/event-nest-struct.json
> > +++ b/tests/qapi-schema/event-nest-struct.json
> > @@ -1,2 +1,2 @@
> >  { 'event': 'EVENT_A',
> > -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > +  'data': { 'a' : { 'type' : { 'integer': 'int' } }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.err 
> > b/tests/qapi-schema/flat-union-inline-invalid-dict.err
> > new file mode 100644
> > index 0000000000..9c4c45b7f0
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/flat-union-inline-invalid-dict.json:7: Key 'type' is 
> > missing from member 'value1' of union 'TestUnion'
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.exit 
> > b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json 
> > b/tests/qapi-schema/flat-union-inline-invalid-dict.json
> > new file mode 100644
> > index 0000000000..62c7cda617
> > --- /dev/null
> > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json
> > @@ -0,0 +1,11 @@
> > +# we require branches to be a struct name
> > +# TODO: should we allow anonymous inline branch types?
> > +{ 'enum': 'TestEnum',
> > +  'data': [ 'value1', 'value2' ] }
> > +{ 'struct': 'Base',
> > +  'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
> > +{ 'union': 'TestUnion',
> > +  'base': 'Base',
> > +  'discriminator': 'enum1',
> > +  'data': { 'value1': { 'string': 'str' },
> > +            'value2': { 'integer': 'int' } } }
> > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.out 
> > b/tests/qapi-schema/flat-union-inline-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/flat-union-inline.json 
> > b/tests/qapi-schema/flat-union-inline.json
> > index 62c7cda617..a9b3ce3f0d 100644
> > --- a/tests/qapi-schema/flat-union-inline.json
> > +++ b/tests/qapi-schema/flat-union-inline.json
> > @@ -7,5 +7,5 @@
> >  { 'union': 'TestUnion',
> >    'base': 'Base',
> >    'discriminator': 'enum1',
> > -  'data': { 'value1': { 'string': 'str' },
> > +  'data': { 'value1': { 'type': {} },
> >              'value2': { 'integer': 'int' } } }
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err 
> > b/tests/qapi-schema/nested-struct-data-invalid-dict.err
> > new file mode 100644
> > index 0000000000..5bd364e8d9
> > --- /dev/null
> > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/nested-struct-data-invalid-dict.json:2: Key 'type' is 
> > missing from member 'a' of 'data' for command 'foo'
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.exit 
> > b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json 
> > b/tests/qapi-schema/nested-struct-data-invalid-dict.json
> > new file mode 100644
> > index 0000000000..efbe773ded
> > --- /dev/null
> > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json
> > @@ -0,0 +1,3 @@
> > +# inline subtypes collide with our desired future use of defaults
> > +{ 'command': 'foo',
> > +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.out 
> > b/tests/qapi-schema/nested-struct-data-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/nested-struct-data.json 
> > b/tests/qapi-schema/nested-struct-data.json
> > index efbe773ded..5b8a40cca3 100644
> > --- a/tests/qapi-schema/nested-struct-data.json
> > +++ b/tests/qapi-schema/nested-struct-data.json
> > @@ -1,3 +1,3 @@
> >  # inline subtypes collide with our desired future use of defaults
> >  { 'command': 'foo',
> > -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> > +  'data': { 'a' : { 'type': {} }, 'b' : 'str' } }
> > diff --git a/tests/qapi-schema/qapi-schema-test.json 
> > b/tests/qapi-schema/qapi-schema-test.json
> > index 22d9044a89..94570154c9 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -11,7 +11,7 @@
> >          'guest-sync' ] } }
> >
> >  { 'struct': 'TestStruct',
> > -  'data': { 'integer': 'int', 'boolean': 'bool', 'string': 'str' } }
> > +  'data': { 'integer': {'type': 'int'}, 'boolean': 'bool', 'string': 'str' 
> > } }
> >
> >  # for testing enums
> >  { 'struct': 'NestedEnumsOne',
> > @@ -77,7 +77,7 @@
> >  { 'union': 'UserDefFlatUnion',
> >    'base': 'UserDefUnionBase',   # intentional forward reference
> >    'discriminator': 'enum1',
> > -  'data': { 'value1' : 'UserDefA',
> > +  'data': { 'value1' : {'type': 'UserDefA'},
> >              'value2' : 'UserDefB',
> >              'value3' : 'UserDefB'
> >              # 'value4' defaults to empty
> > @@ -98,7 +98,7 @@
> >  { 'struct': 'WrapAlternate',
> >    'data': { 'alt': 'UserDefAlternate' } }
> >  { 'alternate': 'UserDefAlternate',
> > -  'data': { 'udfu': 'UserDefFlatUnion', 'e': 'EnumOne', 'i': 'int',
> > +  'data': { 'udfu': {'type': 'UserDefFlatUnion'}, 'e': 'EnumOne', 'i': 
> > 'int',
> >              'n': 'null' } }
> >
> >  { 'struct': 'UserDefC',
> > @@ -134,7 +134,7 @@
> >  { 'command': 'user_def_cmd', 'data': {} }
> >  { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
> >  { 'command': 'user_def_cmd2',
> > -  'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
> > +  'data': {'ud1a': {'type': 'UserDefOne'}, '*ud1b': 'UserDefOne'},
> >    'returns': 'UserDefTwo' }
> >
> >  { 'command': 'cmd-success-response', 'data': {}, 'success-response': false 
> > }
> > @@ -166,7 +166,7 @@
> >
> >  # testing event
> >  { 'struct': 'EventStructOne',
> > -  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' 
> > } }
> > +  'data': { 'struct1': {'type': 'UserDefOne'}, 'string': 'str', '*enum2': 
> > 'EnumOne' } }
> >
> >  { 'event': 'EVENT_A' }
> >  { 'event': 'EVENT_B',
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.err 
> > b/tests/qapi-schema/struct-member-invalid-dict.err
> > new file mode 100644
> > index 0000000000..6a765bc668
> > --- /dev/null
> > +++ b/tests/qapi-schema/struct-member-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/struct-member-invalid-dict.json:2: Key 'type' is missing 
> > from member '*a' of 'data' for struct 'foo'
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.exit 
> > b/tests/qapi-schema/struct-member-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/struct-member-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.json 
> > b/tests/qapi-schema/struct-member-invalid-dict.json
> > new file mode 100644
> > index 0000000000..9fe0d455a9
> > --- /dev/null
> > +++ b/tests/qapi-schema/struct-member-invalid-dict.json
> > @@ -0,0 +1,3 @@
> > +# Long form of member must have a value member 'type'
> > +{ 'struct': 'foo',
> > +  'data': { '*a': { 'case': 'foo' } } }
> > diff --git a/tests/qapi-schema/struct-member-invalid-dict.out 
> > b/tests/qapi-schema/struct-member-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.err 
> > b/tests/qapi-schema/union-branch-invalid-dict.err
> > new file mode 100644
> > index 0000000000..89f9b36791
> > --- /dev/null
> > +++ b/tests/qapi-schema/union-branch-invalid-dict.err
> > @@ -0,0 +1 @@
> > +tests/qapi-schema/union-branch-invalid-dict.json:2: Key 'type' is missing 
> > from member 'integer' of union 'UnionInvalidBranch'
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.exit 
> > b/tests/qapi-schema/union-branch-invalid-dict.exit
> > new file mode 100644
> > index 0000000000..d00491fd7e
> > --- /dev/null
> > +++ b/tests/qapi-schema/union-branch-invalid-dict.exit
> > @@ -0,0 +1 @@
> > +1
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.json 
> > b/tests/qapi-schema/union-branch-invalid-dict.json
> > new file mode 100644
> > index 0000000000..9778598dbd
> > --- /dev/null
> > +++ b/tests/qapi-schema/union-branch-invalid-dict.json
> > @@ -0,0 +1,4 @@
> > +# Long form of member must have a value member 'type'
> > +{ 'union': 'UnionInvalidBranch',
> > +  'data': { 'integer': { 'if': 'foo'},
> > +            's8': 'int8' } }
> > diff --git a/tests/qapi-schema/union-branch-invalid-dict.out 
> > b/tests/qapi-schema/union-branch-invalid-dict.out
> > new file mode 100644
> > index 0000000000..e69de29bb2
>
> Looks good, but I'd like to investigate whether my suggested placement
> of the normalization would simplify things before I give my R-by.
>

I am not sure I follow, you suggest to have normalization before
check? Then how can we produce an error that reflects the input
accurately? Should we carry additional information (original form etc)
on the source during normalization?

-- 
Marc-André Lureau
reply via email to
[Prev in Thread] Current Thread [Next in Thread]