* [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name
@ 2014-03-05 2:44 Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 01/10] qapi script: remember explicitly defined enum values Wenchao Xia
` (12 more replies)
0 siblings, 13 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
This series address two issues:
1. support using enum as discriminator in union.
For example, if we have following define in qapi schema:
{ 'enum': 'EnumOne',
'data': [ 'value1', 'value2', 'value3' ] }
{ 'type': 'UserDefBase0',
'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
Before this series, discriminator in union must be a string, and a
hidden enum type as discriminator is generated. After this series,
qapi schema can directly use predefined enum type:
{ 'union': 'UserDefEnumDiscriminatorUnion',
'base': 'UserDefBase0',
'discriminator' : 'base-enum0',
'data': { 'value1' : 'UserDefA',
'value2' : 'UserDefInherit',
'value3' : 'UserDefB' } }
The benefit is that every thing is defined explicitly in schema file,
the discriminator enum type can be used in other API define in schema,
and a compile time check will be put to verify the correctness according
to enum define. Currently BlockdevOptions used discriminator which can
be converted, in the future other union can also use enum discriminator.
The implement is done by:
1.1 remember the enum defines by qapi scripts.(patch 1)
1.2 use the remembered enum define to check correctness at compile
time.(patch 3), more strict check(patch 2)
1.3 use the same enum name generation rule to avoid C code mismatch,
esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
1.4 switch the code path, when pre-defined enum type is used as discriminator,
don't generate a hidden enum type, use the enum type instead, add
docs/qapi-code-gen.txt.(Patch 6)
1.5 test case shows how it looks like.(Patch 7)
1.6 convert BlockdevOptions. (Patch 8)
2. Better enum name generation
Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
name generation codes into one function, it is done easily by modifying
it.(Patch 9)
Changes from RFC:
Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
enum value in union, add related test case, remove direct inherit support "_base"
and related test case. RFC series at:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
v2:
General:
3/8: use Raise exception instead of sys.error.write in qapi.py.
Address Eric's comments:
2/8,3/8: more check for enum value at compile time, not allow partly mapping.
8/8: correspond test case change.
v3:
General:
move enum name generation patch to last in the series, add convert patch
8/9.
Address Luiz and Kevin's comments:
Better introduction.
6/9: renamed this patch, add docs/qapi-code-gen.txt part.
v4:
Address Eric's comments:
5/9: better commit message.
6/9: typo fix in doc.
9/9: typo fix, fix indentation, better incode comment.
v5:
Address Eric's comments:
6/10: doc typo fix.
8/10: new patch to remove string discriminator.
9/10: removed the string discriminator test case.
v6:
rebased on upstream by adding "blgdebug" and "blkverify" in qapi-schema.json
in patch 7/10.
v7:
The series is rebased on Markus's tree:
git://repo.or.cz/qemu/armbru.git branch qapi-scripts
Address Markus's comments:
2/11: typo fix in error message.
3/11: new patch to recording addtional line info in schema parsing.
4/11: move the check into qapi.py, report better with fp/line info.
7/11: move the UnionKind adding in qapi.py after 1st parsing and
with better error info.
9/11: move the check into qapi.py with fp/line info, test case
corresond change.
11/11: new patch for errpr path test case.
v8:
The series is ontop of Markus's tree and rebased on upstream:
Address Markus's comments:
1/10: better commit title, simplify is_enum().
2/10: test case squashed into this patch.
3/10: no change, column computation is not touched.
4/10: simplify commit title and message, refine the semantic check
logic as comments in v7, check in discriminator_find_enum_define() is moved
out so that the function can be used without error info, squash related test
into this patch, re-orgnize 'expr_elem' with separate 'info' member,
QAPIExprError now takes only info to work, better error message, remove
check of whether all enum values are covered by branch, use expr['key']
instead of expr.get('key') when possible.
6/10: remove useless comments in generate_enum_full_value(), make line
shorter by change variable name.
7/10: building 'expr_elem' for discriminator_find_enum_define() is removed,
make line shorter in qapi-visit.py, add a test case that enum is used before
define.
8/10: rebased on uptream by adding 'quorum' driver type.
9/10: better doc and error message, add a test case for string discriminator.
use string concatenation rather than line continuation for string in C code.
v9:
Address Eric's comments:
Solve double SoB in commit messages.
2/10: typo fix in commit message.
4/10: add expr change in commit message, typo fix in comments for
discriminator_find_enum_define() and move it to patch 7/10.
9/10: better commit message.
Address Markus's comments:
4/10: add back commit message, discriminator_find_enum_define() is moved to
patch 7/10, since no caller in this patch now, simplify check for flat union.
Improve comments for TODO in check for every branch, remove unused define in
test schema.
Wenchao Xia (10):
1 qapi script: remember explicitly defined enum values
2 qapi script: add check for duplicated key
3 qapi script: remember line number in schema parsing
4 qapi script: check correctness of union
5 qapi script: code move for generate_enum_name()
6 qapi script: use same function to generate enum string
7 qapi script: support enum type as discriminator in union
8 qapi: convert BlockdevOptions to use enum discriminator
9 qapi script: do not allow string discriminator
10 qapi script: do not add "_" for every capitalized char in enum
docs/qapi-code-gen.txt | 5 +-
include/qapi/qmp/qerror.h | 2 +-
qapi-schema.json | 14 ++-
scripts/qapi-types.py | 34 ++--
scripts/qapi-visit.py | 42 ++++--
scripts/qapi.py | 184 ++++++++++++++++++--
target-i386/cpu.c | 2 +-
tests/Makefile | 6 +-
tests/qapi-schema/comments.out | 2 +-
tests/qapi-schema/duplicate-key.err | 1 +
tests/qapi-schema/duplicate-key.exit | 1 +
tests/qapi-schema/duplicate-key.json | 2 +
.../qapi-schema/flat-union-invalid-branch-key.err | 1 +
.../qapi-schema/flat-union-invalid-branch-key.exit | 1 +
.../qapi-schema/flat-union-invalid-branch-key.json | 17 ++
.../flat-union-invalid-discriminator.err | 1 +
.../flat-union-invalid-discriminator.exit | 1 +
.../flat-union-invalid-discriminator.json | 17 ++
tests/qapi-schema/flat-union-no-base.err | 1 +
tests/qapi-schema/flat-union-no-base.exit | 1 +
tests/qapi-schema/flat-union-no-base.json | 10 +
tests/qapi-schema/flat-union-reverse-define.exit | 1 +
tests/qapi-schema/flat-union-reverse-define.json | 17 ++
tests/qapi-schema/flat-union-reverse-define.out | 9 +
.../flat-union-string-discriminator.err | 1 +
.../flat-union-string-discriminator.exit | 1 +
.../flat-union-string-discriminator.json | 17 ++
tests/qapi-schema/qapi-schema-test.json | 9 +-
tests/qapi-schema/qapi-schema-test.out | 13 +-
tests/qapi-schema/union-invalid-base.err | 1 +
tests/qapi-schema/union-invalid-base.exit | 1 +
tests/qapi-schema/union-invalid-base.json | 10 +
tests/test-qmp-input-strict.c | 5 +-
tests/test-qmp-input-visitor.c | 10 +-
tests/test-qmp-output-visitor.c | 10 +-
35 files changed, 383 insertions(+), 67 deletions(-)
create mode 100644 tests/qapi-schema/duplicate-key.err
create mode 100644 tests/qapi-schema/duplicate-key.exit
create mode 100644 tests/qapi-schema/duplicate-key.json
create mode 100644 tests/qapi-schema/duplicate-key.out
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.exit
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.json
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.out
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.exit
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.json
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.out
create mode 100644 tests/qapi-schema/flat-union-no-base.err
create mode 100644 tests/qapi-schema/flat-union-no-base.exit
create mode 100644 tests/qapi-schema/flat-union-no-base.json
create mode 100644 tests/qapi-schema/flat-union-no-base.out
create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.err
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.exit
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.json
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.out
create mode 100644 tests/qapi-schema/union-invalid-base.err
create mode 100644 tests/qapi-schema/union-invalid-base.exit
create mode 100644 tests/qapi-schema/union-invalid-base.json
create mode 100644 tests/qapi-schema/union-invalid-base.out
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 01/10] qapi script: remember explicitly defined enum values
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 02/10] qapi script: add check for duplicated key Wenchao Xia
` (11 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Later other scripts will need to check the enum values.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 16 +++++++++++-----
| 2 +-
tests/qapi-schema/qapi-schema-test.out | 10 +++++-----
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f3c2a20..023930e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -169,7 +169,7 @@ def parse_schema(fp):
for expr in schema.exprs:
if expr.has_key('enum'):
- add_enum(expr['enum'])
+ add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
add_union(expr)
add_enum('%sKind' % expr['union'])
@@ -289,13 +289,19 @@ def find_union(name):
return union
return None
-def add_enum(name):
+def add_enum(name, enum_values = None):
global enum_types
- enum_types.append(name)
+ enum_types.append({"enum_name": name, "enum_values": enum_values})
-def is_enum(name):
+def find_enum(name):
global enum_types
- return (name in enum_types)
+ for enum in enum_types:
+ if enum['enum_name'] == name:
+ return enum
+ return None
+
+def is_enum(name):
+ return find_enum(name) != None
def c_type(name):
if name == 'str':
--git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index e3bd904..4ce3dcf 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,3 +1,3 @@
[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-['Status']
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
[]
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 89b53d4..01685d4 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -15,11 +15,11 @@
OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 'int'), ('*b', 'int')])), ('returns', 'int')]),
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
-['EnumOne',
- 'UserDefUnionKind',
- 'UserDefFlatUnionKind',
- 'UserDefAnonUnionKind',
- 'UserDefNativeListUnionKind']
+[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
+ {'enum_name': 'UserDefUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 02/10] qapi script: add check for duplicated key
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 01/10] qapi script: remember explicitly defined enum values Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 03/10] qapi script: remember line number in schema parsing Wenchao Xia
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
It is bad that same key was specified twice, especially when a union has
two branches with same condition. This patch can prevent it.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 2 ++
tests/Makefile | 3 ++-
tests/qapi-schema/duplicate-key.err | 1 +
tests/qapi-schema/duplicate-key.exit | 1 +
tests/qapi-schema/duplicate-key.json | 2 ++
5 files changed, 8 insertions(+), 1 deletions(-)
create mode 100644 tests/qapi-schema/duplicate-key.err
create mode 100644 tests/qapi-schema/duplicate-key.exit
create mode 100644 tests/qapi-schema/duplicate-key.json
create mode 100644 tests/qapi-schema/duplicate-key.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 023930e..d0e7934 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -116,6 +116,8 @@ class QAPISchema:
if self.tok != ':':
raise QAPISchemaError(self, 'Expected ":"')
self.accept()
+ if key in expr:
+ raise QAPISchemaError(self, 'Duplicate key "%s"' % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
diff --git a/tests/Makefile b/tests/Makefile
index b17d41e..dfe06eb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -142,7 +142,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
missing-comma-object.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
trailing-comma-list.json trailing-comma-object.json \
- unclosed-list.json unclosed-object.json unclosed-string.json)
+ unclosed-list.json unclosed-object.json unclosed-string.json \
+ duplicate-key.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err
new file mode 100644
index 0000000..0801c6a
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.err
@@ -0,0 +1 @@
+<stdin>:2:10: Duplicate key "key"
diff --git a/tests/qapi-schema/duplicate-key.exit b/tests/qapi-schema/duplicate-key.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json
new file mode 100644
index 0000000..1b55d88
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.json
@@ -0,0 +1,2 @@
+{ 'key': 'value',
+ 'key': 'value' }
diff --git a/tests/qapi-schema/duplicate-key.out b/tests/qapi-schema/duplicate-key.out
new file mode 100644
index 0000000..e69de29
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 03/10] qapi script: remember line number in schema parsing
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 01/10] qapi script: remember explicitly defined enum values Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 02/10] qapi script: add check for duplicated key Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 04/10] qapi script: check correctness of union Wenchao Xia
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Before this patch, 'QAPISchemaError' scans whole input until 'pos'
to get error line number. After this patch, the scan is avoided since
line number is remembered in schema parsing. This patch also benefits
other error report functions, which would be introduced later.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index d0e7934..1954292 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -39,12 +39,10 @@ class QAPISchemaError(Exception):
def __init__(self, schema, msg):
self.fp = schema.fp
self.msg = msg
- self.line = self.col = 1
- for ch in schema.src[0:schema.pos]:
- if ch == '\n':
- self.line += 1
- self.col = 1
- elif ch == '\t':
+ self.col = 1
+ self.line = schema.line
+ for ch in schema.src[schema.line_pos:schema.pos]:
+ if ch == '\t':
self.col = (self.col + 7) % 8 + 1
else:
self.col += 1
@@ -60,6 +58,8 @@ class QAPISchema:
if self.src == '' or self.src[-1] != '\n':
self.src += '\n'
self.cursor = 0
+ self.line = 1
+ self.line_pos = 0
self.exprs = []
self.accept()
@@ -100,6 +100,8 @@ class QAPISchema:
if self.cursor == len(self.src):
self.tok = None
return
+ self.line += 1
+ self.line_pos = self.cursor
elif not self.tok.isspace():
raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 04/10] qapi script: check correctness of union
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (2 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 03/10] qapi script: remember line number in schema parsing Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 05/10] qapi script: code move for generate_enum_name() Wenchao Xia
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Since line info is remembered as QAPISchema.line now, this patch
uses it as additional info for every expr in QAPISchema inside qapi.py,
then improves error message with it in checking of exprs.
For common union the patch will check whether base is a valid complex
type if specified. For flat union it will check whether base presents,
whether discriminator is found in base, whether the key of every branch
is correct when discriminator is an enum type.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
scripts/qapi.py | 88 +++++++++++++++++++-
tests/Makefile | 4 +-
.../qapi-schema/flat-union-invalid-branch-key.err | 1 +
.../qapi-schema/flat-union-invalid-branch-key.exit | 1 +
.../qapi-schema/flat-union-invalid-branch-key.json | 17 ++++
.../flat-union-invalid-discriminator.err | 1 +
.../flat-union-invalid-discriminator.exit | 1 +
.../flat-union-invalid-discriminator.json | 17 ++++
tests/qapi-schema/flat-union-no-base.err | 1 +
tests/qapi-schema/flat-union-no-base.exit | 1 +
tests/qapi-schema/flat-union-no-base.json | 10 ++
tests/qapi-schema/union-invalid-base.err | 1 +
tests/qapi-schema/union-invalid-base.exit | 1 +
tests/qapi-schema/union-invalid-base.json | 10 ++
14 files changed, 151 insertions(+), 3 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.exit
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.json
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.out
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.exit
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.json
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.out
create mode 100644 tests/qapi-schema/flat-union-no-base.err
create mode 100644 tests/qapi-schema/flat-union-no-base.exit
create mode 100644 tests/qapi-schema/flat-union-no-base.json
create mode 100644 tests/qapi-schema/flat-union-no-base.out
create mode 100644 tests/qapi-schema/union-invalid-base.err
create mode 100644 tests/qapi-schema/union-invalid-base.exit
create mode 100644 tests/qapi-schema/union-invalid-base.json
create mode 100644 tests/qapi-schema/union-invalid-base.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1954292..f1ca5b6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -50,6 +50,15 @@ class QAPISchemaError(Exception):
def __str__(self):
return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg)
+class QAPIExprError(Exception):
+ def __init__(self, expr_info, msg):
+ self.fp = expr_info['fp']
+ self.line = expr_info['line']
+ self.msg = msg
+
+ def __str__(self):
+ return "%s:%s: %s" % (self.fp.name, self.line, self.msg)
+
class QAPISchema:
def __init__(self, fp):
@@ -64,7 +73,10 @@ class QAPISchema:
self.accept()
while self.tok != None:
- self.exprs.append(self.get_expr(False))
+ expr_info = {'fp': fp, 'line': self.line}
+ expr_elem = {'expr': self.get_expr(False),
+ 'info': expr_info}
+ self.exprs.append(expr_elem)
def accept(self):
while True:
@@ -162,6 +174,71 @@ class QAPISchema:
raise QAPISchemaError(self, 'Expected "{", "[" or string')
return expr
+def find_base_fields(base):
+ base_struct_define = find_struct(base)
+ if not base_struct_define:
+ return None
+ return base_struct_define['data']
+
+def check_union(expr, expr_info):
+ name = expr['union']
+ base = expr.get('base')
+ discriminator = expr.get('discriminator')
+ members = expr['data']
+
+ # If the object has a member 'base', its value must name a complex type.
+ if base:
+ base_fields = find_base_fields(base)
+ if not base_fields:
+ raise QAPIExprError(expr_info,
+ "Base '%s' is not a valid type"
+ % base)
+
+ # If the union object has no member 'discriminator', it's an
+ # ordinary union.
+ if not discriminator:
+ enum_define = None
+
+ # Else if the value of member 'discriminator' is {}, it's an
+ # anonymous union.
+ elif discriminator == {}:
+ enum_define = None
+
+ # Else, it's a flat union.
+ else:
+ # The object must have a member 'base'.
+ if not base:
+ raise QAPIExprError(expr_info,
+ "Flat union '%s' must have a base field"
+ % name)
+ # The value of member 'discriminator' must name a member of the
+ # base type.
+ discriminator_type = base_fields.get(discriminator)
+ if not discriminator_type:
+ raise QAPIExprError(expr_info,
+ "Discriminator '%s' is not a member of base "
+ "type '%s'"
+ % (discriminator, base))
+ enum_define = find_enum(discriminator_type)
+
+ # Check every branch
+ for (key, value) in members.items():
+ # If this named member's value names an enum type, then all members
+ # of 'data' must also be members of the enum type.
+ if enum_define and not key in enum_define['enum_values']:
+ raise QAPIExprError(expr_info,
+ "Discriminator value '%s' is not found in "
+ "enum '%s'" %
+ (key, enum_define["enum_name"]))
+ # Todo: add checking for values. Key is checked as above, value can be
+ # also checked here, but we need more functions to handle array case.
+
+def check_exprs(schema):
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
+ if expr.has_key('union'):
+ check_union(expr, expr_elem['info'])
+
def parse_schema(fp):
try:
schema = QAPISchema(fp)
@@ -171,7 +248,8 @@ def parse_schema(fp):
exprs = []
- for expr in schema.exprs:
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
if expr.has_key('enum'):
add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
@@ -181,6 +259,12 @@ def parse_schema(fp):
add_struct(expr)
exprs.append(expr)
+ try:
+ check_exprs(schema)
+ except QAPIExprError, e:
+ print >>sys.stderr, e
+ exit(1)
+
return exprs
def parse_args(typeinfo):
diff --git a/tests/Makefile b/tests/Makefile
index dfe06eb..6ac9889 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -143,7 +143,9 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
qapi-schema-test.json quoted-structural-chars.json \
trailing-comma-list.json trailing-comma-object.json \
unclosed-list.json unclosed-object.json unclosed-string.json \
- duplicate-key.json)
+ duplicate-key.json union-invalid-base.json flat-union-no-base.json \
+ flat-union-invalid-discriminator.json \
+ flat-union-invalid-branch-key.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err
new file mode 100644
index 0000000..1125caf
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-branch-key.err
@@ -0,0 +1 @@
+<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum'
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.exit b/tests/qapi-schema/flat-union-invalid-branch-key.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-branch-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.json b/tests/qapi-schema/flat-union-invalid-branch-key.json
new file mode 100644
index 0000000..a624282
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-branch-key.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value_wrong': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.out b/tests/qapi-schema/flat-union-invalid-branch-key.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err
new file mode 100644
index 0000000..cad9dbf
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.err
@@ -0,0 +1 @@
+<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase'
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.exit b/tests/qapi-schema/flat-union-invalid-discriminator.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json b/tests/qapi-schema/flat-union-invalid-discriminator.json
new file mode 100644
index 0000000..887157e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum_wrong',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.out b/tests/qapi-schema/flat-union-invalid-discriminator.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err
new file mode 100644
index 0000000..e2d7443
--- /dev/null
+++ b/tests/qapi-schema/flat-union-no-base.err
@@ -0,0 +1 @@
+<stdin>:7: Flat union 'TestUnion' must have a base field
diff --git a/tests/qapi-schema/flat-union-no-base.exit b/tests/qapi-schema/flat-union-no-base.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-no-base.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-no-base.json b/tests/qapi-schema/flat-union-no-base.json
new file mode 100644
index 0000000..50f2673
--- /dev/null
+++ b/tests/qapi-schema/flat-union-no-base.json
@@ -0,0 +1,10 @@
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'discriminator': 'enum1',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-no-base.out b/tests/qapi-schema/flat-union-no-base.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err
new file mode 100644
index 0000000..dd8e3d1
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-base.err
@@ -0,0 +1 @@
+<stdin>:7: Base 'TestBaseWrong' is not a valid type
diff --git a/tests/qapi-schema/union-invalid-base.exit b/tests/qapi-schema/union-invalid-base.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-base.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-invalid-base.json b/tests/qapi-schema/union-invalid-base.json
new file mode 100644
index 0000000..1fa4930
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-base.json
@@ -0,0 +1,10 @@
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBaseWrong',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/union-invalid-base.out b/tests/qapi-schema/union-invalid-base.out
new file mode 100644
index 0000000..e69de29
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 05/10] qapi script: code move for generate_enum_name()
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (3 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 04/10] qapi script: check correctness of union Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 06/10] qapi script: use same function to generate enum string Wenchao Xia
` (7 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Later both qapi-types.py and qapi-visit.py need a common function
for enum name generation.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-types.py | 10 ----------
scripts/qapi.py | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 2c6e0dc..35ad993 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -127,16 +127,6 @@ const char *%(name)s_lookup[] = {
''')
return ret
-def generate_enum_name(name):
- if name.isupper():
- return c_fun(name, False)
- new_name = ''
- for c in c_fun(name, False):
- if c.isupper():
- new_name += '_'
- new_name += c
- return new_name.lstrip('_').upper()
-
def generate_enum(name, values):
lookup_decl = mcgen('''
extern const char *%(name)s_lookup[];
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f1ca5b6..0de9fe2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -467,3 +467,13 @@ def guardend(name):
''',
name=guardname(name))
+
+def generate_enum_name(name):
+ if name.isupper():
+ return c_fun(name, False)
+ new_name = ''
+ for c in c_fun(name, False):
+ if c.isupper():
+ new_name += '_'
+ new_name += c
+ return new_name.lstrip('_').upper()
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 06/10] qapi script: use same function to generate enum string
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (4 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 05/10] qapi script: code move for generate_enum_name() Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union Wenchao Xia
` (6 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Prior to this patch, qapi-visit.py used custom code to generate enum
names used for handling a qapi union. Fix it to instead reuse common
code, with identical generated results, and allowing future updates to
generation to only need to touch one place.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-types.py | 6 +++---
scripts/qapi-visit.py | 19 +++++++++++++------
scripts/qapi.py | 13 +++++++++----
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 35ad993..5885bac 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -144,11 +144,11 @@ typedef enum %(name)s
i = 0
for value in enum_values:
+ enum_full_value = generate_enum_full_value(name, value)
enum_decl += mcgen('''
- %(abbrev)s_%(value)s = %(i)d,
+ %(enum_full_value)s = %(i)d,
''',
- abbrev=de_camel_case(name).upper(),
- value=generate_enum_name(value),
+ enum_full_value = enum_full_value,
i=i)
i += 1
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c6de9ae..0baaf60 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -214,18 +214,22 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
''',
name=name)
+ # For anon union, always use the default enum type automatically generated
+ # as "'%sKind' % (name)"
+ disc_type = '%sKind' % (name)
+
for key in members:
assert (members[key] in builtin_types
or find_struct(members[key])
or find_union(members[key])), "Invalid anonymous union member"
+ enum_full_value = generate_enum_full_value(disc_type, key)
ret += mcgen('''
- case %(abbrev)s_KIND_%(enum)s:
+ case %(enum_full_value)s:
visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
break;
''',
- abbrev = de_camel_case(name).upper(),
- enum = c_fun(de_camel_case(key),False).upper(),
+ enum_full_value = enum_full_value,
c_type = type_name(members[key]),
c_name = c_fun(key))
@@ -255,7 +259,10 @@ def generate_visit_union(expr):
assert not base
return generate_visit_anon_union(name, members)
+ # There will always be a discriminator in the C switch code, by default it
+ # is an enum type generated silently as "'%sKind' % (name)"
ret = generate_visit_enum('%sKind' % name, members.keys())
+ disc_type = '%sKind' % (name)
if base:
base_fields = find_struct(base)['data']
@@ -313,13 +320,13 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
visit_end_implicit_struct(m, &err);
}'''
+ enum_full_value = generate_enum_full_value(disc_type, key)
ret += mcgen('''
- case %(abbrev)s_KIND_%(enum)s:
+ case %(enum_full_value)s:
''' + fmt + '''
break;
''',
- abbrev = de_camel_case(name).upper(),
- enum = c_fun(de_camel_case(key),False).upper(),
+ enum_full_value = enum_full_value,
c_type=type_name(members[key]),
c_name=c_fun(key))
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0de9fe2..eebc8a7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -468,12 +468,17 @@ def guardend(name):
''',
name=guardname(name))
-def generate_enum_name(name):
- if name.isupper():
- return c_fun(name, False)
+def _generate_enum_value_string(value):
+ if value.isupper():
+ return c_fun(value, False)
new_name = ''
- for c in c_fun(name, False):
+ for c in c_fun(value, False):
if c.isupper():
new_name += '_'
new_name += c
return new_name.lstrip('_').upper()
+
+def generate_enum_full_value(enum_name, enum_value):
+ abbrev_string = de_camel_case(enum_name).upper()
+ value_string = _generate_enum_value_string(enum_value)
+ return "%s_%s" % (abbrev_string, value_string)
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (5 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 06/10] qapi script: use same function to generate enum string Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-06 8:25 ` Markus Armbruster
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 08/10] qapi: convert BlockdevOptions to use enum discriminator Wenchao Xia
` (5 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
By default, any union will automatically generate a enum type as
"[UnionName]Kind" in C code, and it is duplicated when the discriminator
is specified as a pre-defined enum type in schema. After this patch,
the pre-defined enum type will be really used as the switch case
condition in generated C code, if discriminator is an enum field.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/qapi-code-gen.txt | 8 ++++-
scripts/qapi-types.py | 18 +++++++++---
scripts/qapi-visit.py | 29 +++++++++++++------
scripts/qapi.py | 32 +++++++++++++++++++++-
tests/Makefile | 2 +-
tests/qapi-schema/flat-union-reverse-define.exit | 1 +
tests/qapi-schema/flat-union-reverse-define.json | 17 +++++++++++
tests/qapi-schema/flat-union-reverse-define.out | 9 ++++++
8 files changed, 99 insertions(+), 17 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..a2e7921 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -123,11 +123,15 @@ And it looks like this on the wire:
Flat union types avoid the nesting on the wire. They are used whenever a
specific field of the base type is declared as the discriminator ('type' is
-then no longer generated). The discriminator must always be a string field.
+then no longer generated). The discriminator can be a string field or a
+predefined enum field. If it is a string field, a hidden enum type will be
+generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
+will be done to verify the correctness. It is recommended to use an enum field.
The above example can then be modified as follows:
+ { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
{ 'type': 'BlockdevCommonOptions',
- 'data': { 'driver': 'str', 'readonly': 'bool' } }
+ 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
{ 'union': 'BlockdevOptions',
'base': 'BlockdevCommonOptions',
'discriminator': 'driver',
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5885bac..10864ef 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -201,14 +201,21 @@ def generate_union(expr):
base = expr.get('base')
discriminator = expr.get('discriminator')
+ enum_define = discriminator_find_enum_define(expr)
+ if enum_define:
+ discriminator_type_name = enum_define['enum_name']
+ else:
+ discriminator_type_name = '%sKind' % (name)
+
ret = mcgen('''
struct %(name)s
{
- %(name)sKind kind;
+ %(discriminator_type_name)s kind;
union {
void *data;
''',
- name=name)
+ name=name,
+ discriminator_type_name=discriminator_type_name)
for key in typeinfo:
ret += mcgen('''
@@ -389,8 +396,11 @@ for expr in exprs:
fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
elif expr.has_key('union'):
ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
- ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
- fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
+ enum_define = discriminator_find_enum_define(expr)
+ if not enum_define:
+ ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
+ fdef.write(generate_enum_lookup('%sKind' % expr['union'],
+ expr['data'].keys()))
if expr.get('discriminator') == {}:
fdef.write(generate_anon_union_qtypes(expr))
else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 0baaf60..45ce3a9 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -259,10 +259,16 @@ def generate_visit_union(expr):
assert not base
return generate_visit_anon_union(name, members)
- # There will always be a discriminator in the C switch code, by default it
- # is an enum type generated silently as "'%sKind' % (name)"
- ret = generate_visit_enum('%sKind' % name, members.keys())
- disc_type = '%sKind' % (name)
+ enum_define = discriminator_find_enum_define(expr)
+ if enum_define:
+ # Use the enum type as discriminator
+ ret = ""
+ disc_type = enum_define['enum_name']
+ else:
+ # There will always be a discriminator in the C switch code, by default it
+ # is an enum type generated silently as "'%sKind' % (name)"
+ ret = generate_visit_enum('%sKind' % name, members.keys())
+ disc_type = '%sKind' % (name)
if base:
base_fields = find_struct(base)['data']
@@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
pop_indent()
if not discriminator:
- desc_type = "type"
+ disc_key = "type"
else:
- desc_type = discriminator
+ disc_key = discriminator
ret += mcgen('''
- visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
+ visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
if (!err) {
switch ((*obj)->kind) {
''',
- name=name, type=desc_type)
+ disc_type = disc_type,
+ disc_key = disc_key)
for key in members:
if not discriminator:
@@ -517,7 +524,11 @@ for expr in exprs:
ret += generate_visit_list(expr['union'], expr['data'])
fdef.write(ret)
- ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
+ enum_define = discriminator_find_enum_define(expr)
+ ret = ""
+ if not enum_define:
+ ret = generate_decl_enum('%sKind' % expr['union'],
+ expr['data'].keys())
ret += generate_declaration(expr['union'], expr['data'])
fdecl.write(ret)
elif expr.has_key('enum'):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index eebc8a7..0a504c4 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -180,6 +180,25 @@ def find_base_fields(base):
return None
return base_struct_define['data']
+# Return the discriminator enum define if discriminator is specified as an
+# enum type, otherwise return None.
+def discriminator_find_enum_define(expr):
+ base = expr.get('base')
+ discriminator = expr.get('discriminator')
+
+ if not (discriminator and base):
+ return None
+
+ base_fields = find_base_fields(base)
+ if not base_fields:
+ return None
+
+ discriminator_type = base_fields.get(discriminator)
+ if not discriminator_type:
+ return None
+
+ return find_enum(discriminator_type)
+
def check_union(expr, expr_info):
name = expr['union']
base = expr.get('base')
@@ -254,11 +273,22 @@ def parse_schema(fp):
add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
add_union(expr)
- add_enum('%sKind' % expr['union'])
elif expr.has_key('type'):
add_struct(expr)
exprs.append(expr)
+ # Try again for hidden UnionKind enum
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
+ if expr.has_key('union'):
+ try:
+ enum_define = discriminator_find_enum_define(expr)
+ except QAPIExprError, e:
+ print >>sys.stderr, e
+ exit(1)
+ if not enum_define:
+ add_enum('%sKind' % expr['union'])
+
try:
check_exprs(schema)
except QAPIExprError, e:
diff --git a/tests/Makefile b/tests/Makefile
index 6ac9889..88fca31 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -145,7 +145,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
unclosed-list.json unclosed-object.json unclosed-string.json \
duplicate-key.json union-invalid-base.json flat-union-no-base.json \
flat-union-invalid-discriminator.json \
- flat-union-invalid-branch-key.json)
+ flat-union-invalid-branch-key.json flat-union-reverse-define.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-reverse-define.err b/tests/qapi-schema/flat-union-reverse-define.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-reverse-define.exit b/tests/qapi-schema/flat-union-reverse-define.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/flat-union-reverse-define.json b/tests/qapi-schema/flat-union-reverse-define.json
new file mode 100644
index 0000000..9ea7e72
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.json
@@ -0,0 +1,17 @@
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
new file mode 100644
index 0000000..03c952e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.out
@@ -0,0 +1,9 @@
+[OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))]),
+ OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
+ OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
+ OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
+ OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
+[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']}]
+[OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
+ OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
+ OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 08/10] qapi: convert BlockdevOptions to use enum discriminator
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (6 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 09/10] qapi script: do not allow string discriminator Wenchao Xia
` (4 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
After this patch, hidden enum type BlockdevOptionsKind will not
be generated, and other API can use enum BlockdevDriver.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi-schema.json | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 193e7e4..ddd5ba4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4246,6 +4246,18 @@
'*no-flush': 'bool' } }
##
+# @BlockdevDriver
+#
+# Drivers that are supported in block device operations.
+#
+# Since: 2.0
+##
+{ 'enum': 'BlockdevDriver',
+ 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug',
+ 'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow',
+ 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] }
+
+##
# @BlockdevOptionsBase
#
# Options that are available for all block devices, independent of the block
@@ -4269,7 +4281,7 @@
# Since: 1.7
##
{ 'type': 'BlockdevOptionsBase',
- 'data': { 'driver': 'str',
+ 'data': { 'driver': 'BlockdevDriver',
'*id': 'str',
'*node-name': 'str',
'*discard': 'BlockdevDiscardOptions',
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 09/10] qapi script: do not allow string discriminator
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (7 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 08/10] qapi: convert BlockdevOptions to use enum discriminator Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 10/10] qapi script: do not add "_" for every capitalized char in enum Wenchao Xia
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Since enum based discriminators provide better type-safety and
ensure that future qapi additions do not forget to adjust dependent
unions, forbid using string as discriminator from now on.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/qapi-code-gen.txt | 5 +----
scripts/qapi.py | 5 +++++
tests/Makefile | 3 ++-
.../flat-union-string-discriminator.err | 1 +
.../flat-union-string-discriminator.exit | 1 +
.../flat-union-string-discriminator.json | 17 +++++++++++++++++
tests/qapi-schema/qapi-schema-test.json | 9 ++++++---
tests/qapi-schema/qapi-schema-test.out | 5 +++--
tests/test-qmp-input-strict.c | 5 ++++-
tests/test-qmp-input-visitor.c | 10 +++++++---
tests/test-qmp-output-visitor.c | 10 ++++++----
11 files changed, 53 insertions(+), 18 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.err
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.exit
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.json
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.out
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a2e7921..d78921f 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -123,10 +123,7 @@ And it looks like this on the wire:
Flat union types avoid the nesting on the wire. They are used whenever a
specific field of the base type is declared as the discriminator ('type' is
-then no longer generated). The discriminator can be a string field or a
-predefined enum field. If it is a string field, a hidden enum type will be
-generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
-will be done to verify the correctness. It is recommended to use an enum field.
+then no longer generated). The discriminator must be of enumeration type.
The above example can then be modified as follows:
{ 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0a504c4..16e691a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -239,6 +239,11 @@ def check_union(expr, expr_info):
"type '%s'"
% (discriminator, base))
enum_define = find_enum(discriminator_type)
+ # Do not allow string discriminator
+ if not enum_define:
+ raise QAPIExprError(expr_info,
+ "Discriminator '%s' must be of enumeration "
+ "type" % discriminator)
# Check every branch
for (key, value) in members.items():
diff --git a/tests/Makefile b/tests/Makefile
index 88fca31..e146f81 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -145,7 +145,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
unclosed-list.json unclosed-object.json unclosed-string.json \
duplicate-key.json union-invalid-base.json flat-union-no-base.json \
flat-union-invalid-discriminator.json \
- flat-union-invalid-branch-key.json flat-union-reverse-define.json)
+ flat-union-invalid-branch-key.json flat-union-reverse-define.json \
+ flat-union-string-discriminator.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err
new file mode 100644
index 0000000..8748270
--- /dev/null
+++ b/tests/qapi-schema/flat-union-string-discriminator.err
@@ -0,0 +1 @@
+<stdin>:13: Discriminator 'kind' must be of enumeration type
diff --git a/tests/qapi-schema/flat-union-string-discriminator.exit b/tests/qapi-schema/flat-union-string-discriminator.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-string-discriminator.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-string-discriminator.json b/tests/qapi-schema/flat-union-string-discriminator.json
new file mode 100644
index 0000000..e966aeb
--- /dev/null
+++ b/tests/qapi-schema/flat-union-string-discriminator.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'kind',
+ 'data': { 'kind1': 'TestTypeA',
+ 'kind2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-string-discriminator.out b/tests/qapi-schema/flat-union-string-discriminator.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 471ba47..818c06d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -37,10 +37,13 @@
'base': 'UserDefZero',
'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+{ 'type': 'UserDefUnionBase',
+ 'data': { 'string': 'str', 'enum1': 'EnumOne' } }
+
{ 'union': 'UserDefFlatUnion',
- 'base': 'UserDefOne',
- 'discriminator': 'string',
- 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+ 'base': 'UserDefUnionBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } }
# FIXME generated struct UserDefFlatUnion has members for direct base
# UserDefOne, but lacks members for indirect base UserDefZero
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 01685d4..6cd03f3 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -7,7 +7,8 @@
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
- OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefOne'), ('discriminator', 'string'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
OrderedDict([('union', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
@@ -17,7 +18,6 @@
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': 'UserDefUnionKind', 'enum_values': None},
- {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None},
{'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
{'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
@@ -27,4 +27,5 @@
OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 64d72f6..38b5e95 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -146,7 +146,10 @@ static void test_validate_union_flat(TestInputVisitorData *data,
Visitor *v;
Error *errp = NULL;
- v = validate_test_init(data, "{ 'string': 'a', 'boolean': true }");
+ v = validate_test_init(data,
+ "{ 'enum1': 'value1', "
+ "'string': 'str', "
+ "'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp);
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 2dffafc..1729667 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -310,14 +310,18 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
Error *err = NULL;
UserDefFlatUnion *tmp;
- v = visitor_input_test_init(data, "{ 'string': 'a', 'boolean': true }");
+ v = visitor_input_test_init(data,
+ "{ 'enum1': 'value1', "
+ "'string': 'str', "
+ "'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
g_assert(err == NULL);
- g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_A);
+ g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1);
+ g_assert_cmpstr(tmp->string, ==, "str");
/* TODO g_assert_cmpint(tmp->integer, ==, 41); */
- g_assert_cmpint(tmp->a->boolean, ==, true);
+ g_assert_cmpint(tmp->value1->boolean, ==, true);
qapi_free_UserDefFlatUnion(tmp);
}
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 105f4cf..da27971 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -449,10 +449,11 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
Error *err = NULL;
UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
- tmp->kind = USER_DEF_UNION_KIND_A;
- tmp->a = g_malloc0(sizeof(UserDefA));
+ tmp->kind = ENUM_ONE_VALUE1;
+ tmp->string = g_strdup("str");
+ tmp->value1 = g_malloc0(sizeof(UserDefA));
/* TODO when generator bug is fixed: tmp->integer = 41; */
- tmp->a->boolean = true;
+ tmp->value1->boolean = true;
visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err);
g_assert(err == NULL);
@@ -461,7 +462,8 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
g_assert(qobject_type(arg) == QTYPE_QDICT);
qdict = qobject_to_qdict(arg);
- g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "a");
+ g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1");
+ g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str");
/* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */
g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH V9 10/10] qapi script: do not add "_" for every capitalized char in enum
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (8 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 09/10] qapi script: do not allow string discriminator Wenchao Xia
@ 2014-03-05 2:44 ` Wenchao Xia
2014-03-06 8:26 ` [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Markus Armbruster
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-05 2:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
Now "enum AIOContext" will generate AIO_CONTEXT instead of A_I_O_CONTEXT,
"X86CPU" will generate X86_CPU instead of X86_C_P_U.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/qapi/qmp/qerror.h | 2 +-
scripts/qapi.py | 26 +++++++++++++++++++-------
target-i386/cpu.c | 2 +-
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 25193c9..da75abf 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -159,7 +159,7 @@ void qerror_report_err(Error *err);
ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
#define QERR_KVM_MISSING_CAP \
- ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
+ ERROR_CLASS_KVM_MISSING_CAP, "Using KVM without %s, %s unavailable"
#define QERR_MIGRATION_ACTIVE \
ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 16e691a..53918b2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -503,17 +503,29 @@ def guardend(name):
''',
name=guardname(name))
-def _generate_enum_value_string(value):
+# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
+# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
+# ENUM24_Name -> ENUM24_NAME
+def _generate_enum_string(value):
+ c_fun_str = c_fun(value, False)
if value.isupper():
- return c_fun(value, False)
+ return c_fun_str
+
new_name = ''
- for c in c_fun(value, False):
- if c.isupper():
- new_name += '_'
+ l = len(c_fun_str)
+ for i in range(l):
+ c = c_fun_str[i]
+ # When c is upper and no "_" appears before, do more checks
+ if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
+ # Case 1: next string is lower
+ # Case 2: previous string is digit
+ if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
+ c_fun_str[i - 1].isdigit():
+ new_name += '_'
new_name += c
return new_name.lstrip('_').upper()
def generate_enum_full_value(enum_name, enum_value):
- abbrev_string = de_camel_case(enum_name).upper()
- value_string = _generate_enum_value_string(enum_value)
+ abbrev_string = _generate_enum_string(enum_name)
+ value_string = _generate_enum_string(enum_value)
return "%s_%s" % (abbrev_string, value_string)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e8812a..c83ab0f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -315,7 +315,7 @@ typedef struct X86RegisterInfo32 {
} X86RegisterInfo32;
#define REGISTER(reg) \
- [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
+ [R_##reg] = { .name = #reg, .qapi_enum = X86_CPU_REGISTER32_##reg }
X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
REGISTER(EAX),
REGISTER(ECX),
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union Wenchao Xia
@ 2014-03-06 8:25 ` Markus Armbruster
2014-03-06 11:54 ` Wenchao Xia
0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2014-03-06 8:25 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, lcapitulino, qemu-devel, mdroth
Wenchao Xia <wenchaoqemu@gmail.com> writes:
> By default, any union will automatically generate a enum type as
> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
> is specified as a pre-defined enum type in schema. After this patch,
> the pre-defined enum type will be really used as the switch case
> condition in generated C code, if discriminator is an enum field.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> docs/qapi-code-gen.txt | 8 ++++-
> scripts/qapi-types.py | 18 +++++++++---
> scripts/qapi-visit.py | 29 +++++++++++++------
> scripts/qapi.py | 32 +++++++++++++++++++++-
> tests/Makefile | 2 +-
> tests/qapi-schema/flat-union-reverse-define.exit | 1 +
> tests/qapi-schema/flat-union-reverse-define.json | 17 +++++++++++
> tests/qapi-schema/flat-union-reverse-define.out | 9 ++++++
> 8 files changed, 99 insertions(+), 17 deletions(-)
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 0728f36..a2e7921 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -123,11 +123,15 @@ And it looks like this on the wire:
>
> Flat union types avoid the nesting on the wire. They are used whenever a
> specific field of the base type is declared as the discriminator ('type' is
> -then no longer generated). The discriminator must always be a string field.
> +then no longer generated). The discriminator can be a string field or a
> +predefined enum field. If it is a string field, a hidden enum type will be
> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
> +will be done to verify the correctness. It is recommended to use an enum field.
> The above example can then be modified as follows:
>
> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
> { 'type': 'BlockdevCommonOptions',
> - 'data': { 'driver': 'str', 'readonly': 'bool' } }
> + 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
> { 'union': 'BlockdevOptions',
> 'base': 'BlockdevCommonOptions',
> 'discriminator': 'driver',
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 5885bac..10864ef 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -201,14 +201,21 @@ def generate_union(expr):
> base = expr.get('base')
> discriminator = expr.get('discriminator')
>
> + enum_define = discriminator_find_enum_define(expr)
> + if enum_define:
> + discriminator_type_name = enum_define['enum_name']
> + else:
> + discriminator_type_name = '%sKind' % (name)
> +
> ret = mcgen('''
> struct %(name)s
> {
> - %(name)sKind kind;
> + %(discriminator_type_name)s kind;
> union {
> void *data;
> ''',
> - name=name)
> + name=name,
> + discriminator_type_name=discriminator_type_name)
>
> for key in typeinfo:
> ret += mcgen('''
> @@ -389,8 +396,11 @@ for expr in exprs:
> fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
> elif expr.has_key('union'):
> ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
> - ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
> - fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
> + enum_define = discriminator_find_enum_define(expr)
> + if not enum_define:
> + ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
> + fdef.write(generate_enum_lookup('%sKind' % expr['union'],
> + expr['data'].keys()))
> if expr.get('discriminator') == {}:
> fdef.write(generate_anon_union_qtypes(expr))
> else:
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 0baaf60..45ce3a9 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -259,10 +259,16 @@ def generate_visit_union(expr):
> assert not base
> return generate_visit_anon_union(name, members)
>
> - # There will always be a discriminator in the C switch code, by default it
> - # is an enum type generated silently as "'%sKind' % (name)"
> - ret = generate_visit_enum('%sKind' % name, members.keys())
> - disc_type = '%sKind' % (name)
> + enum_define = discriminator_find_enum_define(expr)
> + if enum_define:
> + # Use the enum type as discriminator
> + ret = ""
> + disc_type = enum_define['enum_name']
> + else:
> + # There will always be a discriminator in the C switch code, by default it
> + # is an enum type generated silently as "'%sKind' % (name)"
> + ret = generate_visit_enum('%sKind' % name, members.keys())
> + disc_type = '%sKind' % (name)
>
> if base:
> base_fields = find_struct(base)['data']
> @@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
> pop_indent()
>
> if not discriminator:
> - desc_type = "type"
> + disc_key = "type"
> else:
> - desc_type = discriminator
> + disc_key = discriminator
> ret += mcgen('''
> - visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
> + visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
> if (!err) {
> switch ((*obj)->kind) {
> ''',
> - name=name, type=desc_type)
> + disc_type = disc_type,
> + disc_key = disc_key)
>
> for key in members:
> if not discriminator:
> @@ -517,7 +524,11 @@ for expr in exprs:
> ret += generate_visit_list(expr['union'], expr['data'])
> fdef.write(ret)
>
> - ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
> + enum_define = discriminator_find_enum_define(expr)
> + ret = ""
> + if not enum_define:
> + ret = generate_decl_enum('%sKind' % expr['union'],
> + expr['data'].keys())
> ret += generate_declaration(expr['union'], expr['data'])
> fdecl.write(ret)
> elif expr.has_key('enum'):
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index eebc8a7..0a504c4 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -180,6 +180,25 @@ def find_base_fields(base):
> return None
> return base_struct_define['data']
>
> +# Return the discriminator enum define if discriminator is specified as an
> +# enum type, otherwise return None.
> +def discriminator_find_enum_define(expr):
> + base = expr.get('base')
> + discriminator = expr.get('discriminator')
> +
> + if not (discriminator and base):
> + return None
> +
> + base_fields = find_base_fields(base)
> + if not base_fields:
> + return None
> +
> + discriminator_type = base_fields.get(discriminator)
> + if not discriminator_type:
> + return None
> +
> + return find_enum(discriminator_type)
> +
> def check_union(expr, expr_info):
> name = expr['union']
> base = expr.get('base')
> @@ -254,11 +273,22 @@ def parse_schema(fp):
> add_enum(expr['enum'], expr['data'])
> elif expr.has_key('union'):
> add_union(expr)
> - add_enum('%sKind' % expr['union'])
> elif expr.has_key('type'):
> add_struct(expr)
> exprs.append(expr)
>
> + # Try again for hidden UnionKind enum
> + for expr_elem in schema.exprs:
> + expr = expr_elem['expr']
> + if expr.has_key('union'):
> + try:
> + enum_define = discriminator_find_enum_define(expr)
> + except QAPIExprError, e:
> + print >>sys.stderr, e
> + exit(1)
How can we get QAPIExprError here?
> + if not enum_define:
> + add_enum('%sKind' % expr['union'])
> +
> try:
> check_exprs(schema)
> except QAPIExprError, e:
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (9 preceding siblings ...)
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 10/10] qapi script: do not add "_" for every capitalized char in enum Wenchao Xia
@ 2014-03-06 8:26 ` Markus Armbruster
2014-03-06 12:21 ` Markus Armbruster
2014-03-07 13:55 ` Luiz Capitulino
12 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2014-03-06 8:26 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, lcapitulino, qemu-devel, mdroth
Wenchao Xia <wenchaoqemu@gmail.com> writes:
> This series address two issues:
>
> 1. support using enum as discriminator in union.
> For example, if we have following define in qapi schema:
> { 'enum': 'EnumOne',
> 'data': [ 'value1', 'value2', 'value3' ] }
>
> { 'type': 'UserDefBase0',
> 'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
>
> Before this series, discriminator in union must be a string, and a
> hidden enum type as discriminator is generated. After this series,
> qapi schema can directly use predefined enum type:
> { 'union': 'UserDefEnumDiscriminatorUnion',
> 'base': 'UserDefBase0',
> 'discriminator' : 'base-enum0',
> 'data': { 'value1' : 'UserDefA',
> 'value2' : 'UserDefInherit',
> 'value3' : 'UserDefB' } }
>
> The benefit is that every thing is defined explicitly in schema file,
> the discriminator enum type can be used in other API define in schema,
> and a compile time check will be put to verify the correctness according
> to enum define. Currently BlockdevOptions used discriminator which can
> be converted, in the future other union can also use enum discriminator.
>
> The implement is done by:
> 1.1 remember the enum defines by qapi scripts.(patch 1)
> 1.2 use the remembered enum define to check correctness at compile
> time.(patch 3), more strict check(patch 2)
> 1.3 use the same enum name generation rule to avoid C code mismatch,
> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> 1.4 switch the code path, when pre-defined enum type is used as discriminator,
> don't generate a hidden enum type, use the enum type instead, add
> docs/qapi-code-gen.txt.(Patch 6)
> 1.5 test case shows how it looks like.(Patch 7)
> 1.6 convert BlockdevOptions. (Patch 8)
>
> 2. Better enum name generation
> Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
> AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
> name generation codes into one function, it is done easily by modifying
> it.(Patch 9)
Applies cleanly to master.
We're down to a single, simple question on PATCH 07/10.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union
2014-03-06 8:25 ` Markus Armbruster
@ 2014-03-06 11:54 ` Wenchao Xia
2014-03-06 13:03 ` Luiz Capitulino
0 siblings, 1 reply; 21+ messages in thread
From: Wenchao Xia @ 2014-03-06 11:54 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, lcapitulino, qemu-devel, mdroth
于 2014/3/6 16:25, Markus Armbruster 写道:
> Wenchao Xia <wenchaoqemu@gmail.com> writes:
>
>> By default, any union will automatically generate a enum type as
>> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
>> is specified as a pre-defined enum type in schema. After this patch,
>> the pre-defined enum type will be really used as the switch case
>> condition in generated C code, if discriminator is an enum field.
>>
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> docs/qapi-code-gen.txt | 8 ++++-
>> scripts/qapi-types.py | 18 +++++++++---
>> scripts/qapi-visit.py | 29 +++++++++++++------
>> scripts/qapi.py | 32 +++++++++++++++++++++-
>> tests/Makefile | 2 +-
>> tests/qapi-schema/flat-union-reverse-define.exit | 1 +
>> tests/qapi-schema/flat-union-reverse-define.json | 17 +++++++++++
>> tests/qapi-schema/flat-union-reverse-define.out | 9 ++++++
>> 8 files changed, 99 insertions(+), 17 deletions(-)
>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
>>
>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>> index 0728f36..a2e7921 100644
>> --- a/docs/qapi-code-gen.txt
>> +++ b/docs/qapi-code-gen.txt
>> @@ -123,11 +123,15 @@ And it looks like this on the wire:
>>
>> Flat union types avoid the nesting on the wire. They are used whenever a
>> specific field of the base type is declared as the discriminator ('type' is
>> -then no longer generated). The discriminator must always be a string field.
>> +then no longer generated). The discriminator can be a string field or a
>> +predefined enum field. If it is a string field, a hidden enum type will be
>> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
>> +will be done to verify the correctness. It is recommended to use an enum field.
>> The above example can then be modified as follows:
>>
>> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>> { 'type': 'BlockdevCommonOptions',
>> - 'data': { 'driver': 'str', 'readonly': 'bool' } }
>> + 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
>> { 'union': 'BlockdevOptions',
>> 'base': 'BlockdevCommonOptions',
>> 'discriminator': 'driver',
>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
>> index 5885bac..10864ef 100644
>> --- a/scripts/qapi-types.py
>> +++ b/scripts/qapi-types.py
>> @@ -201,14 +201,21 @@ def generate_union(expr):
>> base = expr.get('base')
>> discriminator = expr.get('discriminator')
>>
>> + enum_define = discriminator_find_enum_define(expr)
>> + if enum_define:
>> + discriminator_type_name = enum_define['enum_name']
>> + else:
>> + discriminator_type_name = '%sKind' % (name)
>> +
>> ret = mcgen('''
>> struct %(name)s
>> {
>> - %(name)sKind kind;
>> + %(discriminator_type_name)s kind;
>> union {
>> void *data;
>> ''',
>> - name=name)
>> + name=name,
>> + discriminator_type_name=discriminator_type_name)
>>
>> for key in typeinfo:
>> ret += mcgen('''
>> @@ -389,8 +396,11 @@ for expr in exprs:
>> fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
>> elif expr.has_key('union'):
>> ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
>> - ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
>> - fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
>> + enum_define = discriminator_find_enum_define(expr)
>> + if not enum_define:
>> + ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
>> + fdef.write(generate_enum_lookup('%sKind' % expr['union'],
>> + expr['data'].keys()))
>> if expr.get('discriminator') == {}:
>> fdef.write(generate_anon_union_qtypes(expr))
>> else:
>> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
>> index 0baaf60..45ce3a9 100644
>> --- a/scripts/qapi-visit.py
>> +++ b/scripts/qapi-visit.py
>> @@ -259,10 +259,16 @@ def generate_visit_union(expr):
>> assert not base
>> return generate_visit_anon_union(name, members)
>>
>> - # There will always be a discriminator in the C switch code, by default it
>> - # is an enum type generated silently as "'%sKind' % (name)"
>> - ret = generate_visit_enum('%sKind' % name, members.keys())
>> - disc_type = '%sKind' % (name)
>> + enum_define = discriminator_find_enum_define(expr)
>> + if enum_define:
>> + # Use the enum type as discriminator
>> + ret = ""
>> + disc_type = enum_define['enum_name']
>> + else:
>> + # There will always be a discriminator in the C switch code, by default it
>> + # is an enum type generated silently as "'%sKind' % (name)"
>> + ret = generate_visit_enum('%sKind' % name, members.keys())
>> + disc_type = '%sKind' % (name)
>>
>> if base:
>> base_fields = find_struct(base)['data']
>> @@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
>> pop_indent()
>>
>> if not discriminator:
>> - desc_type = "type"
>> + disc_key = "type"
>> else:
>> - desc_type = discriminator
>> + disc_key = discriminator
>> ret += mcgen('''
>> - visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
>> + visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
>> if (!err) {
>> switch ((*obj)->kind) {
>> ''',
>> - name=name, type=desc_type)
>> + disc_type = disc_type,
>> + disc_key = disc_key)
>>
>> for key in members:
>> if not discriminator:
>> @@ -517,7 +524,11 @@ for expr in exprs:
>> ret += generate_visit_list(expr['union'], expr['data'])
>> fdef.write(ret)
>>
>> - ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
>> + enum_define = discriminator_find_enum_define(expr)
>> + ret = ""
>> + if not enum_define:
>> + ret = generate_decl_enum('%sKind' % expr['union'],
>> + expr['data'].keys())
>> ret += generate_declaration(expr['union'], expr['data'])
>> fdecl.write(ret)
>> elif expr.has_key('enum'):
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index eebc8a7..0a504c4 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -180,6 +180,25 @@ def find_base_fields(base):
>> return None
>> return base_struct_define['data']
>>
>> +# Return the discriminator enum define if discriminator is specified as an
>> +# enum type, otherwise return None.
>> +def discriminator_find_enum_define(expr):
>> + base = expr.get('base')
>> + discriminator = expr.get('discriminator')
>> +
>> + if not (discriminator and base):
>> + return None
>> +
>> + base_fields = find_base_fields(base)
>> + if not base_fields:
>> + return None
>> +
>> + discriminator_type = base_fields.get(discriminator)
>> + if not discriminator_type:
>> + return None
>> +
>> + return find_enum(discriminator_type)
>> +
>> def check_union(expr, expr_info):
>> name = expr['union']
>> base = expr.get('base')
>> @@ -254,11 +273,22 @@ def parse_schema(fp):
>> add_enum(expr['enum'], expr['data'])
>> elif expr.has_key('union'):
>> add_union(expr)
>> - add_enum('%sKind' % expr['union'])
>> elif expr.has_key('type'):
>> add_struct(expr)
>> exprs.append(expr)
>>
>> + # Try again for hidden UnionKind enum
>> + for expr_elem in schema.exprs:
>> + expr = expr_elem['expr']
>> + if expr.has_key('union'):
>> + try:
>> + enum_define = discriminator_find_enum_define(expr)
>> + except QAPIExprError, e:
>> + print >>sys.stderr, e
>> + exit(1)
> How can we get QAPIExprError here?
>
Ops, for this version Exception wouldn't happen, I forgot
to remove the "try except". It should be simply:
+ # Try again for hidden UnionKind enum
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
+ if expr.has_key('union'):
+ enum_define = discriminator_find_enum_define(expr)
Luiz, do you mind apply this series and correct above directly?
>> + if not enum_define:
>> + add_enum('%sKind' % expr['union'])
>> +
>> try:
>> check_exprs(schema)
>> except QAPIExprError, e:
> [...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (10 preceding siblings ...)
2014-03-06 8:26 ` [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Markus Armbruster
@ 2014-03-06 12:21 ` Markus Armbruster
2014-03-07 13:55 ` Luiz Capitulino
2014-03-07 13:55 ` Luiz Capitulino
12 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2014-03-06 12:21 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, lcapitulino, qemu-devel, mdroth
Wenchao Xia <wenchaoqemu@gmail.com> writes:
> This series address two issues:
>
> 1. support using enum as discriminator in union.
> For example, if we have following define in qapi schema:
> { 'enum': 'EnumOne',
> 'data': [ 'value1', 'value2', 'value3' ] }
>
> { 'type': 'UserDefBase0',
> 'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
>
> Before this series, discriminator in union must be a string, and a
> hidden enum type as discriminator is generated. After this series,
> qapi schema can directly use predefined enum type:
> { 'union': 'UserDefEnumDiscriminatorUnion',
> 'base': 'UserDefBase0',
> 'discriminator' : 'base-enum0',
> 'data': { 'value1' : 'UserDefA',
> 'value2' : 'UserDefInherit',
> 'value3' : 'UserDefB' } }
>
> The benefit is that every thing is defined explicitly in schema file,
> the discriminator enum type can be used in other API define in schema,
> and a compile time check will be put to verify the correctness according
> to enum define. Currently BlockdevOptions used discriminator which can
> be converted, in the future other union can also use enum discriminator.
>
> The implement is done by:
> 1.1 remember the enum defines by qapi scripts.(patch 1)
> 1.2 use the remembered enum define to check correctness at compile
> time.(patch 3), more strict check(patch 2)
> 1.3 use the same enum name generation rule to avoid C code mismatch,
> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> 1.4 switch the code path, when pre-defined enum type is used as discriminator,
> don't generate a hidden enum type, use the enum type instead, add
> docs/qapi-code-gen.txt.(Patch 6)
> 1.5 test case shows how it looks like.(Patch 7)
> 1.6 convert BlockdevOptions. (Patch 8)
>
> 2. Better enum name generation
> Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
> AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
> name generation codes into one function, it is done easily by modifying
> it.(Patch 9)
With the unwanted try ... except removed from 07/10, series
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks again for rebasing onto my work!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union
2014-03-06 11:54 ` Wenchao Xia
@ 2014-03-06 13:03 ` Luiz Capitulino
2014-03-07 1:08 ` [Qemu-devel] [PATCH " Wenchao Xia
2014-03-07 1:11 ` [Qemu-devel] [PATCH V9 " Wenchao Xia
0 siblings, 2 replies; 21+ messages in thread
From: Luiz Capitulino @ 2014-03-06 13:03 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, mdroth, Markus Armbruster, qemu-devel
On Thu, 06 Mar 2014 19:54:33 +0800
Wenchao Xia <wenchaoqemu@gmail.com> wrote:
> 于 2014/3/6 16:25, Markus Armbruster 写道:
> > Wenchao Xia <wenchaoqemu@gmail.com> writes:
> >
> >> By default, any union will automatically generate a enum type as
> >> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
> >> is specified as a pre-defined enum type in schema. After this patch,
> >> the pre-defined enum type will be really used as the switch case
> >> condition in generated C code, if discriminator is an enum field.
> >>
> >> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >> ---
> >> docs/qapi-code-gen.txt | 8 ++++-
> >> scripts/qapi-types.py | 18 +++++++++---
> >> scripts/qapi-visit.py | 29 +++++++++++++------
> >> scripts/qapi.py | 32 +++++++++++++++++++++-
> >> tests/Makefile | 2 +-
> >> tests/qapi-schema/flat-union-reverse-define.exit | 1 +
> >> tests/qapi-schema/flat-union-reverse-define.json | 17 +++++++++++
> >> tests/qapi-schema/flat-union-reverse-define.out | 9 ++++++
> >> 8 files changed, 99 insertions(+), 17 deletions(-)
> >> create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
> >> create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
> >> create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
> >> create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
> >>
> >> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> >> index 0728f36..a2e7921 100644
> >> --- a/docs/qapi-code-gen.txt
> >> +++ b/docs/qapi-code-gen.txt
> >> @@ -123,11 +123,15 @@ And it looks like this on the wire:
> >>
> >> Flat union types avoid the nesting on the wire. They are used whenever a
> >> specific field of the base type is declared as the discriminator ('type' is
> >> -then no longer generated). The discriminator must always be a string field.
> >> +then no longer generated). The discriminator can be a string field or a
> >> +predefined enum field. If it is a string field, a hidden enum type will be
> >> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
> >> +will be done to verify the correctness. It is recommended to use an enum field.
> >> The above example can then be modified as follows:
> >>
> >> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
> >> { 'type': 'BlockdevCommonOptions',
> >> - 'data': { 'driver': 'str', 'readonly': 'bool' } }
> >> + 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
> >> { 'union': 'BlockdevOptions',
> >> 'base': 'BlockdevCommonOptions',
> >> 'discriminator': 'driver',
> >> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> >> index 5885bac..10864ef 100644
> >> --- a/scripts/qapi-types.py
> >> +++ b/scripts/qapi-types.py
> >> @@ -201,14 +201,21 @@ def generate_union(expr):
> >> base = expr.get('base')
> >> discriminator = expr.get('discriminator')
> >>
> >> + enum_define = discriminator_find_enum_define(expr)
> >> + if enum_define:
> >> + discriminator_type_name = enum_define['enum_name']
> >> + else:
> >> + discriminator_type_name = '%sKind' % (name)
> >> +
> >> ret = mcgen('''
> >> struct %(name)s
> >> {
> >> - %(name)sKind kind;
> >> + %(discriminator_type_name)s kind;
> >> union {
> >> void *data;
> >> ''',
> >> - name=name)
> >> + name=name,
> >> + discriminator_type_name=discriminator_type_name)
> >>
> >> for key in typeinfo:
> >> ret += mcgen('''
> >> @@ -389,8 +396,11 @@ for expr in exprs:
> >> fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
> >> elif expr.has_key('union'):
> >> ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
> >> - ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
> >> - fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
> >> + enum_define = discriminator_find_enum_define(expr)
> >> + if not enum_define:
> >> + ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
> >> + fdef.write(generate_enum_lookup('%sKind' % expr['union'],
> >> + expr['data'].keys()))
> >> if expr.get('discriminator') == {}:
> >> fdef.write(generate_anon_union_qtypes(expr))
> >> else:
> >> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> >> index 0baaf60..45ce3a9 100644
> >> --- a/scripts/qapi-visit.py
> >> +++ b/scripts/qapi-visit.py
> >> @@ -259,10 +259,16 @@ def generate_visit_union(expr):
> >> assert not base
> >> return generate_visit_anon_union(name, members)
> >>
> >> - # There will always be a discriminator in the C switch code, by default it
> >> - # is an enum type generated silently as "'%sKind' % (name)"
> >> - ret = generate_visit_enum('%sKind' % name, members.keys())
> >> - disc_type = '%sKind' % (name)
> >> + enum_define = discriminator_find_enum_define(expr)
> >> + if enum_define:
> >> + # Use the enum type as discriminator
> >> + ret = ""
> >> + disc_type = enum_define['enum_name']
> >> + else:
> >> + # There will always be a discriminator in the C switch code, by default it
> >> + # is an enum type generated silently as "'%sKind' % (name)"
> >> + ret = generate_visit_enum('%sKind' % name, members.keys())
> >> + disc_type = '%sKind' % (name)
> >>
> >> if base:
> >> base_fields = find_struct(base)['data']
> >> @@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
> >> pop_indent()
> >>
> >> if not discriminator:
> >> - desc_type = "type"
> >> + disc_key = "type"
> >> else:
> >> - desc_type = discriminator
> >> + disc_key = discriminator
> >> ret += mcgen('''
> >> - visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
> >> + visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
> >> if (!err) {
> >> switch ((*obj)->kind) {
> >> ''',
> >> - name=name, type=desc_type)
> >> + disc_type = disc_type,
> >> + disc_key = disc_key)
> >>
> >> for key in members:
> >> if not discriminator:
> >> @@ -517,7 +524,11 @@ for expr in exprs:
> >> ret += generate_visit_list(expr['union'], expr['data'])
> >> fdef.write(ret)
> >>
> >> - ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
> >> + enum_define = discriminator_find_enum_define(expr)
> >> + ret = ""
> >> + if not enum_define:
> >> + ret = generate_decl_enum('%sKind' % expr['union'],
> >> + expr['data'].keys())
> >> ret += generate_declaration(expr['union'], expr['data'])
> >> fdecl.write(ret)
> >> elif expr.has_key('enum'):
> >> diff --git a/scripts/qapi.py b/scripts/qapi.py
> >> index eebc8a7..0a504c4 100644
> >> --- a/scripts/qapi.py
> >> +++ b/scripts/qapi.py
> >> @@ -180,6 +180,25 @@ def find_base_fields(base):
> >> return None
> >> return base_struct_define['data']
> >>
> >> +# Return the discriminator enum define if discriminator is specified as an
> >> +# enum type, otherwise return None.
> >> +def discriminator_find_enum_define(expr):
> >> + base = expr.get('base')
> >> + discriminator = expr.get('discriminator')
> >> +
> >> + if not (discriminator and base):
> >> + return None
> >> +
> >> + base_fields = find_base_fields(base)
> >> + if not base_fields:
> >> + return None
> >> +
> >> + discriminator_type = base_fields.get(discriminator)
> >> + if not discriminator_type:
> >> + return None
> >> +
> >> + return find_enum(discriminator_type)
> >> +
> >> def check_union(expr, expr_info):
> >> name = expr['union']
> >> base = expr.get('base')
> >> @@ -254,11 +273,22 @@ def parse_schema(fp):
> >> add_enum(expr['enum'], expr['data'])
> >> elif expr.has_key('union'):
> >> add_union(expr)
> >> - add_enum('%sKind' % expr['union'])
> >> elif expr.has_key('type'):
> >> add_struct(expr)
> >> exprs.append(expr)
> >>
> >> + # Try again for hidden UnionKind enum
> >> + for expr_elem in schema.exprs:
> >> + expr = expr_elem['expr']
> >> + if expr.has_key('union'):
> >> + try:
> >> + enum_define = discriminator_find_enum_define(expr)
> >> + except QAPIExprError, e:
> >> + print >>sys.stderr, e
> >> + exit(1)
> > How can we get QAPIExprError here?
> >
> Ops, for this version Exception wouldn't happen, I forgot
> to remove the "try except". It should be simply:
>
> + # Try again for hidden UnionKind enum
> + for expr_elem in schema.exprs:
> + expr = expr_elem['expr']
> + if expr.has_key('union'):
> + enum_define = discriminator_find_enum_define(expr)
>
> Luiz, do you mind apply this series and correct above directly?
I do. Please, respin 07/10 only and send it as a reply to this thread.
> >> + if not enum_define:
> >> + add_enum('%sKind' % expr['union'])
> >> +
> >> try:
> >> check_exprs(schema)
> >> except QAPIExprError, e:
> > [...]
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 07/10] qapi script: support enum type as discriminator in union
2014-03-06 13:03 ` Luiz Capitulino
@ 2014-03-07 1:08 ` Wenchao Xia
2014-03-07 14:50 ` Markus Armbruster
2014-03-07 1:11 ` [Qemu-devel] [PATCH V9 " Wenchao Xia
1 sibling, 1 reply; 21+ messages in thread
From: Wenchao Xia @ 2014-03-07 1:08 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Wenchao Xia, mdroth, armbru, lcapitulino
By default, any union will automatically generate a enum type as
"[UnionName]Kind" in C code, and it is duplicated when the discriminator
is specified as a pre-defined enum type in schema. After this patch,
the pre-defined enum type will be really used as the switch case
condition in generated C code, if discriminator is an enum field.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qapi-code-gen.txt | 8 ++++-
scripts/qapi-types.py | 18 ++++++++++---
scripts/qapi-visit.py | 29 +++++++++++++++-------
scripts/qapi.py | 27 +++++++++++++++++++-
tests/Makefile | 2 +-
tests/qapi-schema/flat-union-reverse-define.exit | 1 +
tests/qapi-schema/flat-union-reverse-define.json | 17 +++++++++++++
tests/qapi-schema/flat-union-reverse-define.out | 9 +++++++
8 files changed, 94 insertions(+), 17 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..a2e7921 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -123,11 +123,15 @@ And it looks like this on the wire:
Flat union types avoid the nesting on the wire. They are used whenever a
specific field of the base type is declared as the discriminator ('type' is
-then no longer generated). The discriminator must always be a string field.
+then no longer generated). The discriminator can be a string field or a
+predefined enum field. If it is a string field, a hidden enum type will be
+generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
+will be done to verify the correctness. It is recommended to use an enum field.
The above example can then be modified as follows:
+ { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
{ 'type': 'BlockdevCommonOptions',
- 'data': { 'driver': 'str', 'readonly': 'bool' } }
+ 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
{ 'union': 'BlockdevOptions',
'base': 'BlockdevCommonOptions',
'discriminator': 'driver',
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5885bac..10864ef 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -201,14 +201,21 @@ def generate_union(expr):
base = expr.get('base')
discriminator = expr.get('discriminator')
+ enum_define = discriminator_find_enum_define(expr)
+ if enum_define:
+ discriminator_type_name = enum_define['enum_name']
+ else:
+ discriminator_type_name = '%sKind' % (name)
+
ret = mcgen('''
struct %(name)s
{
- %(name)sKind kind;
+ %(discriminator_type_name)s kind;
union {
void *data;
''',
- name=name)
+ name=name,
+ discriminator_type_name=discriminator_type_name)
for key in typeinfo:
ret += mcgen('''
@@ -389,8 +396,11 @@ for expr in exprs:
fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
elif expr.has_key('union'):
ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
- ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
- fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
+ enum_define = discriminator_find_enum_define(expr)
+ if not enum_define:
+ ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
+ fdef.write(generate_enum_lookup('%sKind' % expr['union'],
+ expr['data'].keys()))
if expr.get('discriminator') == {}:
fdef.write(generate_anon_union_qtypes(expr))
else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 0baaf60..45ce3a9 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -259,10 +259,16 @@ def generate_visit_union(expr):
assert not base
return generate_visit_anon_union(name, members)
- # There will always be a discriminator in the C switch code, by default it
- # is an enum type generated silently as "'%sKind' % (name)"
- ret = generate_visit_enum('%sKind' % name, members.keys())
- disc_type = '%sKind' % (name)
+ enum_define = discriminator_find_enum_define(expr)
+ if enum_define:
+ # Use the enum type as discriminator
+ ret = ""
+ disc_type = enum_define['enum_name']
+ else:
+ # There will always be a discriminator in the C switch code, by default it
+ # is an enum type generated silently as "'%sKind' % (name)"
+ ret = generate_visit_enum('%sKind' % name, members.keys())
+ disc_type = '%sKind' % (name)
if base:
base_fields = find_struct(base)['data']
@@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
pop_indent()
if not discriminator:
- desc_type = "type"
+ disc_key = "type"
else:
- desc_type = discriminator
+ disc_key = discriminator
ret += mcgen('''
- visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
+ visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
if (!err) {
switch ((*obj)->kind) {
''',
- name=name, type=desc_type)
+ disc_type = disc_type,
+ disc_key = disc_key)
for key in members:
if not discriminator:
@@ -517,7 +524,11 @@ for expr in exprs:
ret += generate_visit_list(expr['union'], expr['data'])
fdef.write(ret)
- ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
+ enum_define = discriminator_find_enum_define(expr)
+ ret = ""
+ if not enum_define:
+ ret = generate_decl_enum('%sKind' % expr['union'],
+ expr['data'].keys())
ret += generate_declaration(expr['union'], expr['data'])
fdecl.write(ret)
elif expr.has_key('enum'):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index eebc8a7..2b43ad2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -180,6 +180,25 @@ def find_base_fields(base):
return None
return base_struct_define['data']
+# Return the discriminator enum define if discriminator is specified as an
+# enum type, otherwise return None.
+def discriminator_find_enum_define(expr):
+ base = expr.get('base')
+ discriminator = expr.get('discriminator')
+
+ if not (discriminator and base):
+ return None
+
+ base_fields = find_base_fields(base)
+ if not base_fields:
+ return None
+
+ discriminator_type = base_fields.get(discriminator)
+ if not discriminator_type:
+ return None
+
+ return find_enum(discriminator_type)
+
def check_union(expr, expr_info):
name = expr['union']
base = expr.get('base')
@@ -254,11 +273,17 @@ def parse_schema(fp):
add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
add_union(expr)
- add_enum('%sKind' % expr['union'])
elif expr.has_key('type'):
add_struct(expr)
exprs.append(expr)
+ # Try again for hidden UnionKind enum
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
+ if expr.has_key('union'):
+ if not discriminator_find_enum_define(expr):
+ add_enum('%sKind' % expr['union'])
+
try:
check_exprs(schema)
except QAPIExprError, e:
diff --git a/tests/Makefile b/tests/Makefile
index 6ac9889..88fca31 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -145,7 +145,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
unclosed-list.json unclosed-object.json unclosed-string.json \
duplicate-key.json union-invalid-base.json flat-union-no-base.json \
flat-union-invalid-discriminator.json \
- flat-union-invalid-branch-key.json)
+ flat-union-invalid-branch-key.json flat-union-reverse-define.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-reverse-define.err b/tests/qapi-schema/flat-union-reverse-define.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-reverse-define.exit b/tests/qapi-schema/flat-union-reverse-define.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/flat-union-reverse-define.json b/tests/qapi-schema/flat-union-reverse-define.json
new file mode 100644
index 0000000..9ea7e72
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.json
@@ -0,0 +1,17 @@
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
new file mode 100644
index 0000000..03c952e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.out
@@ -0,0 +1,9 @@
+[OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))]),
+ OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
+ OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
+ OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
+ OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
+[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']}]
+[OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
+ OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
+ OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union
2014-03-06 13:03 ` Luiz Capitulino
2014-03-07 1:08 ` [Qemu-devel] [PATCH " Wenchao Xia
@ 2014-03-07 1:11 ` Wenchao Xia
1 sibling, 0 replies; 21+ messages in thread
From: Wenchao Xia @ 2014-03-07 1:11 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: kwolf, mdroth, Markus Armbruster, qemu-devel
于 2014/3/6 21:03, Luiz Capitulino 写道:
> On Thu, 06 Mar 2014 19:54:33 +0800
> Wenchao Xia<wenchaoqemu@gmail.com> wrote:
>
>> 于 2014/3/6 16:25, Markus Armbruster 写道:
>>> Wenchao Xia<wenchaoqemu@gmail.com> writes:
>>>
>>>> By default, any union will automatically generate a enum type as
>>>> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
>>>> is specified as a pre-defined enum type in schema. After this patch,
>>>> the pre-defined enum type will be really used as the switch case
>>>> condition in generated C code, if discriminator is an enum field.
>>>>
>>>> Signed-off-by: Wenchao Xia<wenchaoqemu@gmail.com>
>>>> Reviewed-by: Eric Blake<eblake@redhat.com>
>>>> ---
>>>> docs/qapi-code-gen.txt | 8 ++++-
>>>> scripts/qapi-types.py | 18 +++++++++---
>>>> scripts/qapi-visit.py | 29 +++++++++++++------
>>>> scripts/qapi.py | 32 +++++++++++++++++++++-
>>>> tests/Makefile | 2 +-
>>>> tests/qapi-schema/flat-union-reverse-define.exit | 1 +
>>>> tests/qapi-schema/flat-union-reverse-define.json | 17 +++++++++++
>>>> tests/qapi-schema/flat-union-reverse-define.out | 9 ++++++
>>>> 8 files changed, 99 insertions(+), 17 deletions(-)
>>>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
>>>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
>>>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
>>>> create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
>>>>
>>>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>>>> index 0728f36..a2e7921 100644
>>>> --- a/docs/qapi-code-gen.txt
>>>> +++ b/docs/qapi-code-gen.txt
>>>> @@ -123,11 +123,15 @@ And it looks like this on the wire:
>>>>
>>>> Flat union types avoid the nesting on the wire. They are used whenever a
>>>> specific field of the base type is declared as the discriminator ('type' is
>>>> -then no longer generated). The discriminator must always be a string field.
>>>> +then no longer generated). The discriminator can be a string field or a
>>>> +predefined enum field. If it is a string field, a hidden enum type will be
>>>> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
>>>> +will be done to verify the correctness. It is recommended to use an enum field.
>>>> The above example can then be modified as follows:
>>>>
>>>> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>>>> { 'type': 'BlockdevCommonOptions',
>>>> - 'data': { 'driver': 'str', 'readonly': 'bool' } }
>>>> + 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
>>>> { 'union': 'BlockdevOptions',
>>>> 'base': 'BlockdevCommonOptions',
>>>> 'discriminator': 'driver',
>>>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
>>>> index 5885bac..10864ef 100644
>>>> --- a/scripts/qapi-types.py
>>>> +++ b/scripts/qapi-types.py
>>>> @@ -201,14 +201,21 @@ def generate_union(expr):
>>>> base = expr.get('base')
>>>> discriminator = expr.get('discriminator')
>>>>
>>>> + enum_define = discriminator_find_enum_define(expr)
>>>> + if enum_define:
>>>> + discriminator_type_name = enum_define['enum_name']
>>>> + else:
>>>> + discriminator_type_name = '%sKind' % (name)
>>>> +
>>>> ret = mcgen('''
>>>> struct %(name)s
>>>> {
>>>> - %(name)sKind kind;
>>>> + %(discriminator_type_name)s kind;
>>>> union {
>>>> void *data;
>>>> ''',
>>>> - name=name)
>>>> + name=name,
>>>> + discriminator_type_name=discriminator_type_name)
>>>>
>>>> for key in typeinfo:
>>>> ret += mcgen('''
>>>> @@ -389,8 +396,11 @@ for expr in exprs:
>>>> fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
>>>> elif expr.has_key('union'):
>>>> ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
>>>> - ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
>>>> - fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
>>>> + enum_define = discriminator_find_enum_define(expr)
>>>> + if not enum_define:
>>>> + ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
>>>> + fdef.write(generate_enum_lookup('%sKind' % expr['union'],
>>>> + expr['data'].keys()))
>>>> if expr.get('discriminator') == {}:
>>>> fdef.write(generate_anon_union_qtypes(expr))
>>>> else:
>>>> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
>>>> index 0baaf60..45ce3a9 100644
>>>> --- a/scripts/qapi-visit.py
>>>> +++ b/scripts/qapi-visit.py
>>>> @@ -259,10 +259,16 @@ def generate_visit_union(expr):
>>>> assert not base
>>>> return generate_visit_anon_union(name, members)
>>>>
>>>> - # There will always be a discriminator in the C switch code, by default it
>>>> - # is an enum type generated silently as "'%sKind' % (name)"
>>>> - ret = generate_visit_enum('%sKind' % name, members.keys())
>>>> - disc_type = '%sKind' % (name)
>>>> + enum_define = discriminator_find_enum_define(expr)
>>>> + if enum_define:
>>>> + # Use the enum type as discriminator
>>>> + ret = ""
>>>> + disc_type = enum_define['enum_name']
>>>> + else:
>>>> + # There will always be a discriminator in the C switch code, by default it
>>>> + # is an enum type generated silently as "'%sKind' % (name)"
>>>> + ret = generate_visit_enum('%sKind' % name, members.keys())
>>>> + disc_type = '%sKind' % (name)
>>>>
>>>> if base:
>>>> base_fields = find_struct(base)['data']
>>>> @@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
>>>> pop_indent()
>>>>
>>>> if not discriminator:
>>>> - desc_type = "type"
>>>> + disc_key = "type"
>>>> else:
>>>> - desc_type = discriminator
>>>> + disc_key = discriminator
>>>> ret += mcgen('''
>>>> - visit_type_%(name)sKind(m,&(*obj)->kind, "%(type)s",&err);
>>>> + visit_type_%(disc_type)s(m,&(*obj)->kind, "%(disc_key)s",&err);
>>>> if (!err) {
>>>> switch ((*obj)->kind) {
>>>> ''',
>>>> - name=name, type=desc_type)
>>>> + disc_type = disc_type,
>>>> + disc_key = disc_key)
>>>>
>>>> for key in members:
>>>> if not discriminator:
>>>> @@ -517,7 +524,11 @@ for expr in exprs:
>>>> ret += generate_visit_list(expr['union'], expr['data'])
>>>> fdef.write(ret)
>>>>
>>>> - ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
>>>> + enum_define = discriminator_find_enum_define(expr)
>>>> + ret = ""
>>>> + if not enum_define:
>>>> + ret = generate_decl_enum('%sKind' % expr['union'],
>>>> + expr['data'].keys())
>>>> ret += generate_declaration(expr['union'], expr['data'])
>>>> fdecl.write(ret)
>>>> elif expr.has_key('enum'):
>>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>>> index eebc8a7..0a504c4 100644
>>>> --- a/scripts/qapi.py
>>>> +++ b/scripts/qapi.py
>>>> @@ -180,6 +180,25 @@ def find_base_fields(base):
>>>> return None
>>>> return base_struct_define['data']
>>>>
>>>> +# Return the discriminator enum define if discriminator is specified as an
>>>> +# enum type, otherwise return None.
>>>> +def discriminator_find_enum_define(expr):
>>>> + base = expr.get('base')
>>>> + discriminator = expr.get('discriminator')
>>>> +
>>>> + if not (discriminator and base):
>>>> + return None
>>>> +
>>>> + base_fields = find_base_fields(base)
>>>> + if not base_fields:
>>>> + return None
>>>> +
>>>> + discriminator_type = base_fields.get(discriminator)
>>>> + if not discriminator_type:
>>>> + return None
>>>> +
>>>> + return find_enum(discriminator_type)
>>>> +
>>>> def check_union(expr, expr_info):
>>>> name = expr['union']
>>>> base = expr.get('base')
>>>> @@ -254,11 +273,22 @@ def parse_schema(fp):
>>>> add_enum(expr['enum'], expr['data'])
>>>> elif expr.has_key('union'):
>>>> add_union(expr)
>>>> - add_enum('%sKind' % expr['union'])
>>>> elif expr.has_key('type'):
>>>> add_struct(expr)
>>>> exprs.append(expr)
>>>>
>>>> + # Try again for hidden UnionKind enum
>>>> + for expr_elem in schema.exprs:
>>>> + expr = expr_elem['expr']
>>>> + if expr.has_key('union'):
>>>> + try:
>>>> + enum_define = discriminator_find_enum_define(expr)
>>>> + except QAPIExprError, e:
>>>> + print>>sys.stderr, e
>>>> + exit(1)
>>> How can we get QAPIExprError here?
>>>
>> Ops, for this version Exception wouldn't happen, I forgot
>> to remove the "try except". It should be simply:
>>
>> + # Try again for hidden UnionKind enum
>> + for expr_elem in schema.exprs:
>> + expr = expr_elem['expr']
>> + if expr.has_key('union'):
>> + enum_define = discriminator_find_enum_define(expr)
>>
>> Luiz, do you mind apply this series and correct above directly?
> I do. Please, respin 07/10 only and send it as a reply to this thread.
>
Modified and rebased, hope the line removed in qapi.py wouldn't
interrupt patch 9,10's
appliance.
>>>> + if not enum_define:
>>>> + add_enum('%sKind' % expr['union'])
>>>> +
>>>> try:
>>>> check_exprs(schema)
>>>> except QAPIExprError, e:
>>> [...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
` (11 preceding siblings ...)
2014-03-06 12:21 ` Markus Armbruster
@ 2014-03-07 13:55 ` Luiz Capitulino
12 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2014-03-07 13:55 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, mdroth, qemu-devel, armbru
On Tue, 4 Mar 2014 18:44:30 -0800
Wenchao Xia <wenchaoqemu@gmail.com> wrote:
> This series address two issues:
>
> 1. support using enum as discriminator in union.
> For example, if we have following define in qapi schema:
> { 'enum': 'EnumOne',
> 'data': [ 'value1', 'value2', 'value3' ] }
>
> { 'type': 'UserDefBase0',
> 'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
>
> Before this series, discriminator in union must be a string, and a
> hidden enum type as discriminator is generated. After this series,
> qapi schema can directly use predefined enum type:
> { 'union': 'UserDefEnumDiscriminatorUnion',
> 'base': 'UserDefBase0',
> 'discriminator' : 'base-enum0',
> 'data': { 'value1' : 'UserDefA',
> 'value2' : 'UserDefInherit',
> 'value3' : 'UserDefB' } }
>
> The benefit is that every thing is defined explicitly in schema file,
> the discriminator enum type can be used in other API define in schema,
> and a compile time check will be put to verify the correctness according
> to enum define. Currently BlockdevOptions used discriminator which can
> be converted, in the future other union can also use enum discriminator.
I've applied this series to the qmp tree, with the new 07/10 you sent.
>
> The implement is done by:
> 1.1 remember the enum defines by qapi scripts.(patch 1)
> 1.2 use the remembered enum define to check correctness at compile
> time.(patch 3), more strict check(patch 2)
> 1.3 use the same enum name generation rule to avoid C code mismatch,
> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> 1.4 switch the code path, when pre-defined enum type is used as discriminator,
> don't generate a hidden enum type, use the enum type instead, add
> docs/qapi-code-gen.txt.(Patch 6)
> 1.5 test case shows how it looks like.(Patch 7)
> 1.6 convert BlockdevOptions. (Patch 8)
>
> 2. Better enum name generation
> Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
> AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
> name generation codes into one function, it is done easily by modifying
> it.(Patch 9)
>
>
> Changes from RFC:
> Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
> enum value in union, add related test case, remove direct inherit support "_base"
> and related test case. RFC series at:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
>
> v2:
> General:
> 3/8: use Raise exception instead of sys.error.write in qapi.py.
> Address Eric's comments:
> 2/8,3/8: more check for enum value at compile time, not allow partly mapping.
> 8/8: correspond test case change.
>
> v3:
> General:
> move enum name generation patch to last in the series, add convert patch
> 8/9.
> Address Luiz and Kevin's comments:
> Better introduction.
> 6/9: renamed this patch, add docs/qapi-code-gen.txt part.
>
> v4:
> Address Eric's comments:
> 5/9: better commit message.
> 6/9: typo fix in doc.
> 9/9: typo fix, fix indentation, better incode comment.
>
> v5:
> Address Eric's comments:
> 6/10: doc typo fix.
> 8/10: new patch to remove string discriminator.
> 9/10: removed the string discriminator test case.
>
> v6:
> rebased on upstream by adding "blgdebug" and "blkverify" in qapi-schema.json
> in patch 7/10.
>
> v7:
> The series is rebased on Markus's tree:
> git://repo.or.cz/qemu/armbru.git branch qapi-scripts
>
> Address Markus's comments:
> 2/11: typo fix in error message.
> 3/11: new patch to recording addtional line info in schema parsing.
> 4/11: move the check into qapi.py, report better with fp/line info.
> 7/11: move the UnionKind adding in qapi.py after 1st parsing and
> with better error info.
> 9/11: move the check into qapi.py with fp/line info, test case
> corresond change.
> 11/11: new patch for errpr path test case.
>
> v8:
> The series is ontop of Markus's tree and rebased on upstream:
> Address Markus's comments:
> 1/10: better commit title, simplify is_enum().
> 2/10: test case squashed into this patch.
> 3/10: no change, column computation is not touched.
> 4/10: simplify commit title and message, refine the semantic check
> logic as comments in v7, check in discriminator_find_enum_define() is moved
> out so that the function can be used without error info, squash related test
> into this patch, re-orgnize 'expr_elem' with separate 'info' member,
> QAPIExprError now takes only info to work, better error message, remove
> check of whether all enum values are covered by branch, use expr['key']
> instead of expr.get('key') when possible.
> 6/10: remove useless comments in generate_enum_full_value(), make line
> shorter by change variable name.
> 7/10: building 'expr_elem' for discriminator_find_enum_define() is removed,
> make line shorter in qapi-visit.py, add a test case that enum is used before
> define.
> 8/10: rebased on uptream by adding 'quorum' driver type.
> 9/10: better doc and error message, add a test case for string discriminator.
> use string concatenation rather than line continuation for string in C code.
>
> v9:
> Address Eric's comments:
> Solve double SoB in commit messages.
> 2/10: typo fix in commit message.
> 4/10: add expr change in commit message, typo fix in comments for
> discriminator_find_enum_define() and move it to patch 7/10.
> 9/10: better commit message.
>
> Address Markus's comments:
> 4/10: add back commit message, discriminator_find_enum_define() is moved to
> patch 7/10, since no caller in this patch now, simplify check for flat union.
> Improve comments for TODO in check for every branch, remove unused define in
> test schema.
>
> Wenchao Xia (10):
> 1 qapi script: remember explicitly defined enum values
> 2 qapi script: add check for duplicated key
> 3 qapi script: remember line number in schema parsing
> 4 qapi script: check correctness of union
> 5 qapi script: code move for generate_enum_name()
> 6 qapi script: use same function to generate enum string
> 7 qapi script: support enum type as discriminator in union
> 8 qapi: convert BlockdevOptions to use enum discriminator
> 9 qapi script: do not allow string discriminator
> 10 qapi script: do not add "_" for every capitalized char in enum
>
> docs/qapi-code-gen.txt | 5 +-
> include/qapi/qmp/qerror.h | 2 +-
> qapi-schema.json | 14 ++-
> scripts/qapi-types.py | 34 ++--
> scripts/qapi-visit.py | 42 ++++--
> scripts/qapi.py | 184 ++++++++++++++++++--
> target-i386/cpu.c | 2 +-
> tests/Makefile | 6 +-
> tests/qapi-schema/comments.out | 2 +-
> tests/qapi-schema/duplicate-key.err | 1 +
> tests/qapi-schema/duplicate-key.exit | 1 +
> tests/qapi-schema/duplicate-key.json | 2 +
> .../qapi-schema/flat-union-invalid-branch-key.err | 1 +
> .../qapi-schema/flat-union-invalid-branch-key.exit | 1 +
> .../qapi-schema/flat-union-invalid-branch-key.json | 17 ++
> .../flat-union-invalid-discriminator.err | 1 +
> .../flat-union-invalid-discriminator.exit | 1 +
> .../flat-union-invalid-discriminator.json | 17 ++
> tests/qapi-schema/flat-union-no-base.err | 1 +
> tests/qapi-schema/flat-union-no-base.exit | 1 +
> tests/qapi-schema/flat-union-no-base.json | 10 +
> tests/qapi-schema/flat-union-reverse-define.exit | 1 +
> tests/qapi-schema/flat-union-reverse-define.json | 17 ++
> tests/qapi-schema/flat-union-reverse-define.out | 9 +
> .../flat-union-string-discriminator.err | 1 +
> .../flat-union-string-discriminator.exit | 1 +
> .../flat-union-string-discriminator.json | 17 ++
> tests/qapi-schema/qapi-schema-test.json | 9 +-
> tests/qapi-schema/qapi-schema-test.out | 13 +-
> tests/qapi-schema/union-invalid-base.err | 1 +
> tests/qapi-schema/union-invalid-base.exit | 1 +
> tests/qapi-schema/union-invalid-base.json | 10 +
> tests/test-qmp-input-strict.c | 5 +-
> tests/test-qmp-input-visitor.c | 10 +-
> tests/test-qmp-output-visitor.c | 10 +-
> 35 files changed, 383 insertions(+), 67 deletions(-)
> create mode 100644 tests/qapi-schema/duplicate-key.err
> create mode 100644 tests/qapi-schema/duplicate-key.exit
> create mode 100644 tests/qapi-schema/duplicate-key.json
> create mode 100644 tests/qapi-schema/duplicate-key.out
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.exit
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.json
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.out
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.exit
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.json
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.out
> create mode 100644 tests/qapi-schema/flat-union-no-base.err
> create mode 100644 tests/qapi-schema/flat-union-no-base.exit
> create mode 100644 tests/qapi-schema/flat-union-no-base.json
> create mode 100644 tests/qapi-schema/flat-union-no-base.out
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.err
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.exit
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.json
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.out
> create mode 100644 tests/qapi-schema/union-invalid-base.err
> create mode 100644 tests/qapi-schema/union-invalid-base.exit
> create mode 100644 tests/qapi-schema/union-invalid-base.json
> create mode 100644 tests/qapi-schema/union-invalid-base.out
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name
2014-03-06 12:21 ` Markus Armbruster
@ 2014-03-07 13:55 ` Luiz Capitulino
0 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2014-03-07 13:55 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, mdroth, Wenchao Xia, qemu-devel
On Thu, 06 Mar 2014 13:21:21 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> Wenchao Xia <wenchaoqemu@gmail.com> writes:
>
> > This series address two issues:
> >
> > 1. support using enum as discriminator in union.
> > For example, if we have following define in qapi schema:
> > { 'enum': 'EnumOne',
> > 'data': [ 'value1', 'value2', 'value3' ] }
> >
> > { 'type': 'UserDefBase0',
> > 'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
> >
> > Before this series, discriminator in union must be a string, and a
> > hidden enum type as discriminator is generated. After this series,
> > qapi schema can directly use predefined enum type:
> > { 'union': 'UserDefEnumDiscriminatorUnion',
> > 'base': 'UserDefBase0',
> > 'discriminator' : 'base-enum0',
> > 'data': { 'value1' : 'UserDefA',
> > 'value2' : 'UserDefInherit',
> > 'value3' : 'UserDefB' } }
> >
> > The benefit is that every thing is defined explicitly in schema file,
> > the discriminator enum type can be used in other API define in schema,
> > and a compile time check will be put to verify the correctness according
> > to enum define. Currently BlockdevOptions used discriminator which can
> > be converted, in the future other union can also use enum discriminator.
> >
> > The implement is done by:
> > 1.1 remember the enum defines by qapi scripts.(patch 1)
> > 1.2 use the remembered enum define to check correctness at compile
> > time.(patch 3), more strict check(patch 2)
> > 1.3 use the same enum name generation rule to avoid C code mismatch,
> > esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> > 1.4 switch the code path, when pre-defined enum type is used as discriminator,
> > don't generate a hidden enum type, use the enum type instead, add
> > docs/qapi-code-gen.txt.(Patch 6)
> > 1.5 test case shows how it looks like.(Patch 7)
> > 1.6 convert BlockdevOptions. (Patch 8)
> >
> > 2. Better enum name generation
> > Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
> > AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
> > name generation codes into one function, it is done easily by modifying
> > it.(Patch 9)
>
> With the unwanted try ... except removed from 07/10, series
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
I've added your reviewed-by to all patches but patch 07/10, you still have
time to add it there though.
>
> Thanks again for rebasing onto my work!
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 07/10] qapi script: support enum type as discriminator in union
2014-03-07 1:08 ` [Qemu-devel] [PATCH " Wenchao Xia
@ 2014-03-07 14:50 ` Markus Armbruster
0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2014-03-07 14:50 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, lcapitulino, qemu-devel, mdroth
Wenchao Xia <wenchaoqemu@gmail.com> writes:
> By default, any union will automatically generate a enum type as
> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
> is specified as a pre-defined enum type in schema. After this patch,
> the pre-defined enum type will be really used as the switch case
> condition in generated C code, if discriminator is an enum field.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2014-03-07 14:50 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-05 2:44 [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 01/10] qapi script: remember explicitly defined enum values Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 02/10] qapi script: add check for duplicated key Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 03/10] qapi script: remember line number in schema parsing Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 04/10] qapi script: check correctness of union Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 05/10] qapi script: code move for generate_enum_name() Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 06/10] qapi script: use same function to generate enum string Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union Wenchao Xia
2014-03-06 8:25 ` Markus Armbruster
2014-03-06 11:54 ` Wenchao Xia
2014-03-06 13:03 ` Luiz Capitulino
2014-03-07 1:08 ` [Qemu-devel] [PATCH " Wenchao Xia
2014-03-07 14:50 ` Markus Armbruster
2014-03-07 1:11 ` [Qemu-devel] [PATCH V9 " Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 08/10] qapi: convert BlockdevOptions to use enum discriminator Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 09/10] qapi script: do not allow string discriminator Wenchao Xia
2014-03-05 2:44 ` [Qemu-devel] [PATCH V9 10/10] qapi script: do not add "_" for every capitalized char in enum Wenchao Xia
2014-03-06 8:26 ` [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name Markus Armbruster
2014-03-06 12:21 ` Markus Armbruster
2014-03-07 13:55 ` Luiz Capitulino
2014-03-07 13:55 ` Luiz Capitulino
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.