Skip to content

Commit

Permalink
qapi: Enforce union and alternate branch naming rules
Browse files Browse the repository at this point in the history
Union branch names should use '-', not '_'.  Enforce this.  The only
offenders are in tests/.  Fix them.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-29-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message typo fixed]
  • Loading branch information
Markus Armbruster committed Mar 23, 2021
1 parent 407efbf commit d83b476
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 11 deletions.
4 changes: 2 additions & 2 deletions scripts/qapi/expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ def check_union(expr, info):
for (key, value) in members.items():
source = "'data' member '%s'" % key
if discriminator is None:
check_name_lower(key, info, source, permit_underscore=True)
check_name_lower(key, info, source)
# else: name is in discriminator enum, which gets checked
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
Expand All @@ -288,7 +288,7 @@ def check_alternate(expr, info):
raise QAPISemError(info, "'data' must not be empty")
for (key, value) in members.items():
source = "'data' member '%s'" % key
check_name_lower(key, info, source, permit_underscore=True)
check_name_lower(key, info, source)
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
check_type(value['type'], info, source)
Expand Down
2 changes: 1 addition & 1 deletion tests/qapi-schema/alternate-clash.err
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
alternate-clash.json: In alternate 'Alt1':
alternate-clash.json:4: branch 'a_b' collides with branch 'a-b'
alternate-clash.json:6: name of 'data' member 'a_b' must not use uppercase or '_'
6 changes: 4 additions & 2 deletions tests/qapi-schema/alternate-clash.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Alternate branch name collision
# Reject an alternate that would result in a collision in generated C
# names (this would try to generate two union members named 'a_b').
# Naming rules make collision impossible (even with the pragma). If
# that wasn't the case, then we'd get a collision in generated C: two
# union members a_b.
{ 'pragma': { 'member-name-exceptions': [ 'Alt1' ] } }
{ 'alternate': 'Alt1',
'data': { 'a-b': 'bool', 'a_b': 'int' } }
2 changes: 1 addition & 1 deletion tests/qapi-schema/qapi-schema-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@

{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
'bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }

{ 'command': 'test-if-union-cmd',
Expand Down
4 changes: 2 additions & 2 deletions tests/qapi-schema/qapi-schema-test.out
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,14 @@ object q_obj_TestStruct-wrapper
member data: TestStruct optional=False
enum TestIfUnionKind
member foo
member union_bar
member bar
if ['defined(TEST_IF_UNION_BAR)']
if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
object TestIfUnion
member type: TestIfUnionKind optional=False
tag type
case foo: q_obj_TestStruct-wrapper
case union_bar: q_obj_str-wrapper
case bar: q_obj_str-wrapper
if ['defined(TEST_IF_UNION_BAR)']
if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
object q_obj_test-if-union-cmd-arg
Expand Down
2 changes: 1 addition & 1 deletion tests/qapi-schema/union-clash-branches.err
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
union-clash-branches.json: In union 'TestUnion':
union-clash-branches.json:4: branch 'a_b' collides with branch 'a-b'
union-clash-branches.json:6: name of 'data' member 'a_b' must not use uppercase or '_'
6 changes: 4 additions & 2 deletions tests/qapi-schema/union-clash-branches.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Union branch name collision
# Reject a union that would result in a collision in generated C names (this
# would try to generate two members 'a_b').
# Naming rules make collision impossible (even with the pragma). If
# that wasn't the case, then we'd get collisions in generated C: two
# union members a_b, and two enum members TEST_UNION_A_B.
{ 'pragma': { 'member-name-exceptions': [ 'TestUnion' ] } }
{ 'union': 'TestUnion',
'data': { 'a-b': 'int', 'a_b': 'str' } }

0 comments on commit d83b476

Please sign in to comment.