| Message ID | 20210323094025.3569441-4-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: Enforce naming rules | expand |
On 3/23/21 5:40 AM, Markus Armbruster wrote: > +# event 'data' member with dict value is (longhand) argument > +# definition, not inline complex type I have to be a weenie and say I don't know exactly what this comment is telling me. (1) What's a longhand argument? (2) What's an inline complex type?
On 3/23/21 8:00 AM, John Snow wrote: > On 3/23/21 5:40 AM, Markus Armbruster wrote: >> +# event 'data' member with dict value is (longhand) argument >> +# definition, not inline complex type > > I have to be a weenie and say I don't know exactly what this comment is > telling me. > > (1) What's a longhand argument? Writing { 'type':'str', 'name':'foo' } is longhand for the corresponding 'foo':'str' shorthand. > (2) What's an inline complex type? Writing { 'command':'Foo', 'returns': { 'foo':'str' } } or { 'union':'U', 'base': { 'foo':'Enum' } ... } are inline complex types; they are shorthand for: { 'struct':'FooReturn', 'data': { 'foo':'str' } } { 'struct':'UBase', 'data': { 'foo':'Enum' } } { 'command':'Foo', 'returns':'FooReturn' } { 'union':'U', 'base':'UBase' } At one point, I had the idea that we might want to write: { 'union':'U', ... 'data': { 'branch': { inline type } } } instead of having to pre-declare the type for the branch; but that idea is no longer feasible, since it would be awkward to distinguish from what we DO have of: { 'union':'U', ... 'data': { 'branch': { 'type':'Branch', 'if': 'COND' } } } and where I don't really see us wanting: { 'union':U', ... 'data': { 'branch': { 'type': { inline type }, 'if': 'COND' } } } (If the situation changes and we do want anonymous inline types anywhere a name can appear now, we'll have more work to do)
On 3/23/21 4:40 AM, Markus Armbruster wrote: > A few old comments talk about "desired future use of defaults" and > "anonymous inline branch types". Kind of misleading since commit > 87adbbffd4 "qapi: add a dictionary form for TYPE" added longhand > member definitions. Talk about that instead. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/qapi-schema/event-member-invalid-dict.err | 2 +- > tests/qapi-schema/event-member-invalid-dict.json | 2 ++ > tests/qapi-schema/flat-union-inline-invalid-dict.json | 4 ++-- > tests/qapi-schema/nested-struct-data-invalid-dict.err | 2 +- > tests/qapi-schema/nested-struct-data-invalid-dict.json | 3 ++- > tests/qapi-schema/nested-struct-data.json | 2 +- > tests/qapi-schema/struct-member-invalid-dict.err | 2 +- > tests/qapi-schema/struct-member-invalid-dict.json | 3 ++- > 8 files changed, 12 insertions(+), 8 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com>
On 3/23/21 9:58 AM, Eric Blake wrote: > On 3/23/21 8:00 AM, John Snow wrote: >> On 3/23/21 5:40 AM, Markus Armbruster wrote: >>> +# event 'data' member with dict value is (longhand) argument >>> +# definition, not inline complex type >> >> I have to be a weenie and say I don't know exactly what this comment is >> telling me. >> >> (1) What's a longhand argument? > > Writing { 'type':'str', 'name':'foo' } is longhand for the corresponding > 'foo':'str' shorthand. > Ah, okay. The canonical object/dict form of a member definition. >> (2) What's an inline complex type? > > Writing { 'command':'Foo', 'returns': { 'foo':'str' } } > or { 'union':'U', 'base': { 'foo':'Enum' } ... } > > are inline complex types; they are shorthand for: > > { 'struct':'FooReturn', 'data': { 'foo':'str' } } > { 'struct':'UBase', 'data': { 'foo':'Enum' } } > { 'command':'Foo', 'returns':'FooReturn' } > { 'union':'U', 'base':'UBase' } > > At one point, I had the idea that we might want to write: > > { 'union':'U', ... > 'data': { 'branch': { inline type } } } > > instead of having to pre-declare the type for the branch; but that idea > is no longer feasible, since it would be awkward to distinguish from > what we DO have of: > > { 'union':'U', ... > 'data': { 'branch': { 'type':'Branch', 'if': 'COND' } } } > > and where I don't really see us wanting: > > { 'union':U', ... > 'data': { 'branch': { 'type': { inline type }, 'if': 'COND' } } } > > (If the situation changes and we do want anonymous inline types anywhere > a name can appear now, we'll have more work to do) > I see, ok, thanks!
On 3/23/21 5:40 AM, Markus Armbruster wrote: > A few old comments talk about "desired future use of defaults" and > "anonymous inline branch types". Kind of misleading since commit > 87adbbffd4 "qapi: add a dictionary form for TYPE" added longhand > member definitions. Talk about that instead. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> I still feel a little warbley on the comment (I will probably forget what it means next week), but I don't have a better suggestion for it now that I know what it's trying to tell me. so, er, Reviewed-by: John Snow <jsnow@redhat.com> > --- > tests/qapi-schema/event-member-invalid-dict.err | 2 +- > tests/qapi-schema/event-member-invalid-dict.json | 2 ++ > tests/qapi-schema/flat-union-inline-invalid-dict.json | 4 ++-- > tests/qapi-schema/nested-struct-data-invalid-dict.err | 2 +- > tests/qapi-schema/nested-struct-data-invalid-dict.json | 3 ++- > tests/qapi-schema/nested-struct-data.json | 2 +- > tests/qapi-schema/struct-member-invalid-dict.err | 2 +- > tests/qapi-schema/struct-member-invalid-dict.json | 3 ++- > 8 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err > index c7a6a24305..82f8989344 100644 > --- a/tests/qapi-schema/event-member-invalid-dict.err > +++ b/tests/qapi-schema/event-member-invalid-dict.err > @@ -1,2 +1,2 @@ > event-member-invalid-dict.json: In event 'EVENT_A': > -event-member-invalid-dict.json:1: 'data' member 'a' misses key 'type' > +event-member-invalid-dict.json:3: 'data' member 'a' misses key 'type' > diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json > index ee6f3ecb6f..e58560abca 100644 > --- a/tests/qapi-schema/event-member-invalid-dict.json > +++ b/tests/qapi-schema/event-member-invalid-dict.json > @@ -1,2 +1,4 @@ > +# event 'data' member with dict value is (longhand) argument > +# definition, not inline complex type > { 'event': 'EVENT_A', > 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } > diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json > index 62c7cda617..1779712795 100644 > --- a/tests/qapi-schema/flat-union-inline-invalid-dict.json > +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json > @@ -1,5 +1,5 @@ > -# we require branches to be a struct name > -# TODO: should we allow anonymous inline branch types? > +# union 'data' member with dict value is (longhand) branch > +# definition, not inline complex type > { 'enum': 'TestEnum', > 'data': [ 'value1', 'value2' ] } > { 'struct': 'Base', > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err > index c044b2b17a..375e155fe6 100644 > --- a/tests/qapi-schema/nested-struct-data-invalid-dict.err > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err > @@ -1,2 +1,2 @@ > nested-struct-data-invalid-dict.json: In command 'foo': > -nested-struct-data-invalid-dict.json:2: 'data' member 'a' misses key 'type' > +nested-struct-data-invalid-dict.json:3: 'data' member 'a' misses key 'type' > diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json > index efbe773ded..aa37b85e19 100644 > --- a/tests/qapi-schema/nested-struct-data-invalid-dict.json > +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json > @@ -1,3 +1,4 @@ > -# inline subtypes collide with our desired future use of defaults > +# command 'data' member with dict value is (longhand) argument > +# definition, not inline complex type > { 'command': 'foo', > 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } > diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json > index 5b8a40cca3..2980d45d05 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 > +# {} is not a valid type reference > { 'command': 'foo', > 'data': { 'a' : { 'type': {} }, 'b' : 'str' } } > diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err > index 0621aecfbd..f9b3f33551 100644 > --- a/tests/qapi-schema/struct-member-invalid-dict.err > +++ b/tests/qapi-schema/struct-member-invalid-dict.err > @@ -1,2 +1,2 @@ > struct-member-invalid-dict.json: In struct 'foo': > -struct-member-invalid-dict.json:2: 'data' member '*a' misses key 'type' > +struct-member-invalid-dict.json:3: 'data' member '*a' misses key 'type' > diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json > index 9fe0d455a9..bc3d62ae63 100644 > --- a/tests/qapi-schema/struct-member-invalid-dict.json > +++ b/tests/qapi-schema/struct-member-invalid-dict.json > @@ -1,3 +1,4 @@ > -# Long form of member must have a value member 'type' > +# struct 'data' member with dict value is (longhand) member > +# definition, not inline complex type > { 'struct': 'foo', > 'data': { '*a': { 'case': 'foo' } } } >
diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err index c7a6a24305..82f8989344 100644 --- a/tests/qapi-schema/event-member-invalid-dict.err +++ b/tests/qapi-schema/event-member-invalid-dict.err @@ -1,2 +1,2 @@ event-member-invalid-dict.json: In event 'EVENT_A': -event-member-invalid-dict.json:1: 'data' member 'a' misses key 'type' +event-member-invalid-dict.json:3: 'data' member 'a' misses key 'type' diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json index ee6f3ecb6f..e58560abca 100644 --- a/tests/qapi-schema/event-member-invalid-dict.json +++ b/tests/qapi-schema/event-member-invalid-dict.json @@ -1,2 +1,4 @@ +# event 'data' member with dict value is (longhand) argument +# definition, not inline complex type { 'event': 'EVENT_A', 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json index 62c7cda617..1779712795 100644 --- a/tests/qapi-schema/flat-union-inline-invalid-dict.json +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json @@ -1,5 +1,5 @@ -# we require branches to be a struct name -# TODO: should we allow anonymous inline branch types? +# union 'data' member with dict value is (longhand) branch +# definition, not inline complex type { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'Base', diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err index c044b2b17a..375e155fe6 100644 --- a/tests/qapi-schema/nested-struct-data-invalid-dict.err +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err @@ -1,2 +1,2 @@ nested-struct-data-invalid-dict.json: In command 'foo': -nested-struct-data-invalid-dict.json:2: 'data' member 'a' misses key 'type' +nested-struct-data-invalid-dict.json:3: 'data' member 'a' misses key 'type' diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json index efbe773ded..aa37b85e19 100644 --- a/tests/qapi-schema/nested-struct-data-invalid-dict.json +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json @@ -1,3 +1,4 @@ -# inline subtypes collide with our desired future use of defaults +# command 'data' member with dict value is (longhand) argument +# definition, not inline complex type { 'command': 'foo', 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json index 5b8a40cca3..2980d45d05 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 +# {} is not a valid type reference { 'command': 'foo', 'data': { 'a' : { 'type': {} }, 'b' : 'str' } } diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err index 0621aecfbd..f9b3f33551 100644 --- a/tests/qapi-schema/struct-member-invalid-dict.err +++ b/tests/qapi-schema/struct-member-invalid-dict.err @@ -1,2 +1,2 @@ struct-member-invalid-dict.json: In struct 'foo': -struct-member-invalid-dict.json:2: 'data' member '*a' misses key 'type' +struct-member-invalid-dict.json:3: 'data' member '*a' misses key 'type' diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json index 9fe0d455a9..bc3d62ae63 100644 --- a/tests/qapi-schema/struct-member-invalid-dict.json +++ b/tests/qapi-schema/struct-member-invalid-dict.json @@ -1,3 +1,4 @@ -# Long form of member must have a value member 'type' +# struct 'data' member with dict value is (longhand) member +# definition, not inline complex type { 'struct': 'foo', 'data': { '*a': { 'case': 'foo' } } }
A few old comments talk about "desired future use of defaults" and "anonymous inline branch types". Kind of misleading since commit 87adbbffd4 "qapi: add a dictionary form for TYPE" added longhand member definitions. Talk about that instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/qapi-schema/event-member-invalid-dict.err | 2 +- tests/qapi-schema/event-member-invalid-dict.json | 2 ++ tests/qapi-schema/flat-union-inline-invalid-dict.json | 4 ++-- tests/qapi-schema/nested-struct-data-invalid-dict.err | 2 +- tests/qapi-schema/nested-struct-data-invalid-dict.json | 3 ++- tests/qapi-schema/nested-struct-data.json | 2 +- tests/qapi-schema/struct-member-invalid-dict.err | 2 +- tests/qapi-schema/struct-member-invalid-dict.json | 3 ++- 8 files changed, 12 insertions(+), 8 deletions(-)