From d08ac81a459258ce20b3184fa9325c6c1350ac9e Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 14 Oct 2015 16:30:25 -0600
Subject: [PATCH 01/12] qapi: Fix regression with '-netdev help'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit e36c714e causes 'qemu -netdev help' to dump core, because the
call to visit_end_union() is no longer conditional on whether *obj was
allocated.

Reported by Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444861825-19256-1-git-send-email-eblake@redhat.com>
[Commit message tweaked to say 'help' instead of '?']
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-visit.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4f97781348..d7f51ee250 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -301,7 +301,9 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
 out_obj:
     error_propagate(errp, err);
     err = NULL;
-    visit_end_union(v, !!(*obj)->data, &err);
+    if (*obj) {
+        visit_end_union(v, !!(*obj)->data, &err);
+    }
     error_propagate(errp, err);
     err = NULL;
     visit_end_struct(v, &err);

From 25a0d9c977c2f5db914b0a1619759fd77d97b016 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:21 -0600
Subject: [PATCH 02/12] qapi: Use predicate callback to determine visit
 filtering

Previously, qapi-types and qapi-visit filtered out implicit
objects during visit_object_type() by using 'info' (works since
implicit objects do not [yet] have associated info); meanwhile
qapi-introspect filtered out all schema types on the first pass
by returning a python type from visit_begin(), which was then
used at a distance in QAPISchema.visit() to do the filtering.

Rather than keeping these ad hoc approaches, add a new visitor
callback visit_needed() which returns False to skip a given
entity, and which defaults to True unless overridden.  Use the
new mechanism to simplify all three filtering visitors.

No change to the generated code.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-introspect.py |  5 ++++-
 scripts/qapi-types.py      | 19 +++++++++++--------
 scripts/qapi-visit.py      | 17 ++++++++++-------
 scripts/qapi.py            | 12 ++++++++----
 4 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 7d39320174..c0dad6679c 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -54,7 +54,6 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
         self._jsons = []
         self._used_types = []
         self._name_map = {}
-        return QAPISchemaType   # don't visit types for now
 
     def visit_end(self):
         # visit the types that are actually used
@@ -82,6 +81,10 @@ const char %(c_name)s[] = %(c_string)s;
         self._used_types = None
         self._name_map = None
 
+    def visit_needed(self, entity):
+        # Ignore types on first pass; visit_end() will pick up used types
+        return not isinstance(entity, QAPISchemaType)
+
     def _name(self, name):
         if self._unmask:
             return name
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index d405f8d670..2a29c6e106 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -233,6 +233,10 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
         self.decl = self._btin + self.decl
         self._btin = None
 
+    def visit_needed(self, entity):
+        # Visit everything except implicit objects
+        return not isinstance(entity, QAPISchemaObjectType) or entity.info
+
     def _gen_type_cleanup(self, name):
         self.decl += gen_type_cleanup_decl(name)
         self.defn += gen_type_cleanup(name)
@@ -254,14 +258,13 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
             self._gen_type_cleanup(name)
 
     def visit_object_type(self, name, info, base, members, variants):
-        if info:
-            self._fwdecl += gen_fwd_object_or_array(name)
-            if variants:
-                assert not members      # not implemented
-                self.decl += gen_union(name, base, variants)
-            else:
-                self.decl += gen_struct(name, base, members)
-            self._gen_type_cleanup(name)
+        self._fwdecl += gen_fwd_object_or_array(name)
+        if variants:
+            assert not members      # not implemented
+            self.decl += gen_union(name, base, variants)
+        else:
+            self.decl += gen_struct(name, base, members)
+        self._gen_type_cleanup(name)
 
     def visit_alternate_type(self, name, info, variants):
         self._fwdecl += gen_fwd_object_or_array(name)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index d7f51ee250..b7a647076d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -335,6 +335,10 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
         self.decl = self._btin + self.decl
         self._btin = None
 
+    def visit_needed(self, entity):
+        # Visit everything except implicit objects
+        return not isinstance(entity, QAPISchemaObjectType) or entity.info
+
     def visit_enum_type(self, name, info, values, prefix):
         self.decl += gen_visit_decl(name, scalar=True)
         self.defn += gen_visit_enum(name)
@@ -351,13 +355,12 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
             self.defn += defn
 
     def visit_object_type(self, name, info, base, members, variants):
-        if info:
-            self.decl += gen_visit_decl(name)
-            if variants:
-                assert not members      # not implemented
-                self.defn += gen_visit_union(name, base, variants)
-            else:
-                self.defn += gen_visit_struct(name, base, members)
+        self.decl += gen_visit_decl(name)
+        if variants:
+            assert not members      # not implemented
+            self.defn += gen_visit_union(name, base, variants)
+        else:
+            self.defn += gen_visit_struct(name, base, members)
 
     def visit_alternate_type(self, name, info, variants):
         self.decl += gen_visit_decl(name)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 26cff3f05c..543b378d7e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -811,6 +811,10 @@ class QAPISchemaVisitor(object):
     def visit_end(self):
         pass
 
+    def visit_needed(self, entity):
+        # Default to visiting everything
+        return True
+
     def visit_builtin_type(self, name, info, json_type):
         pass
 
@@ -1304,10 +1308,10 @@ class QAPISchema(object):
             ent.check(self)
 
     def visit(self, visitor):
-        ignore = visitor.visit_begin(self)
-        for name in sorted(self._entity_dict.keys()):
-            if not ignore or not isinstance(self._entity_dict[name], ignore):
-                self._entity_dict[name].visit(visitor)
+        visitor.visit_begin(self)
+        for (name, entity) in sorted(self._entity_dict.items()):
+            if visitor.visit_needed(entity):
+                entity.visit(visitor)
         visitor.visit_end()
 
 

From 7618b91ff80ec42b84b29be24d8ef53ddb377110 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:22 -0600
Subject: [PATCH 03/12] qapi: Prepare for errors during check()

The next few patches will start migrating error checking from
ad hoc parse methods into the QAPISchema*.check() methods.  But
for an error message to display, we first have to fix the
overall 'try' to catch those errors.  We also want to enable a
few more assertions, such as making sure every attempt to
raise a semantic error is passed a valid location info, or that
various preconditions hold.

The general approach for moving error checking will then be to
relax an assertion into an if that raises an exception if the
condition does not hold, and removing the counterpart ad hoc
check done during the parse phase.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 543b378d7e..4573599d7c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -103,6 +103,7 @@ class QAPISchemaError(Exception):
 class QAPIExprError(Exception):
     def __init__(self, expr_info, msg):
         Exception.__init__(self)
+        assert expr_info
         self.info = expr_info
         self.msg = msg
 
@@ -964,6 +965,7 @@ class QAPISchemaObjectType(QAPISchemaType):
             members = []
         seen = {}
         for m in members:
+            assert c_name(m.name) not in seen
             seen[m.name] = m
         for m in self.local_members:
             m.check(schema, members, seen)
@@ -1116,13 +1118,13 @@ class QAPISchema(object):
     def __init__(self, fname):
         try:
             self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs)
+            self._entity_dict = {}
+            self._def_predefineds()
+            self._def_exprs()
+            self.check()
         except (QAPISchemaError, QAPIExprError), err:
             print >>sys.stderr, err
             exit(1)
-        self._entity_dict = {}
-        self._def_predefineds()
-        self._def_exprs()
-        self.check()
 
     def _def_entity(self, ent):
         assert ent.name not in self._entity_dict

From baabb84c5b27115b979d864cb2af4d63b823983f Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:23 -0600
Subject: [PATCH 04/12] qapi: Drop redundant alternate-good test

The alternate-good.json test was already covered by
qapi-schema-test.json.  As future commits will be tweaking
how alternates are laid out, removing the duplicate test now
reduces churn.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile                        |  1 -
 tests/qapi-schema/alternate-good.err  |  0
 tests/qapi-schema/alternate-good.exit |  1 -
 tests/qapi-schema/alternate-good.json |  9 ---------
 tests/qapi-schema/alternate-good.out  | 10 ----------
 5 files changed, 21 deletions(-)
 delete mode 100644 tests/qapi-schema/alternate-good.err
 delete mode 100644 tests/qapi-schema/alternate-good.exit
 delete mode 100644 tests/qapi-schema/alternate-good.json
 delete mode 100644 tests/qapi-schema/alternate-good.out

diff --git a/tests/Makefile b/tests/Makefile
index 209eca9bae..9a6601d014 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -230,7 +230,6 @@ qapi-schema += alternate-clash.json
 qapi-schema += alternate-conflict-dict.json
 qapi-schema += alternate-conflict-string.json
 qapi-schema += alternate-empty.json
-qapi-schema += alternate-good.json
 qapi-schema += alternate-nested.json
 qapi-schema += alternate-unknown.json
 qapi-schema += args-alternate.json
diff --git a/tests/qapi-schema/alternate-good.err b/tests/qapi-schema/alternate-good.err
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/alternate-good.exit b/tests/qapi-schema/alternate-good.exit
deleted file mode 100644
index 573541ac97..0000000000
--- a/tests/qapi-schema/alternate-good.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/alternate-good.json b/tests/qapi-schema/alternate-good.json
deleted file mode 100644
index 33717704ce..0000000000
--- a/tests/qapi-schema/alternate-good.json
+++ /dev/null
@@ -1,9 +0,0 @@
-# Working example of alternate
-{ 'struct': 'Data',
-  'data': { '*number': 'int', '*name': 'str' } }
-{ 'enum': 'Enum',
-  'data': [ 'hello', 'world' ] }
-{ 'alternate': 'Alt',
-  'data': { 'value': 'int',
-            'string': 'Enum',
-            'struct': 'Data' } }
diff --git a/tests/qapi-schema/alternate-good.out b/tests/qapi-schema/alternate-good.out
deleted file mode 100644
index 65af7278f4..0000000000
--- a/tests/qapi-schema/alternate-good.out
+++ /dev/null
@@ -1,10 +0,0 @@
-object :empty
-alternate Alt
-    case value: int
-    case string: Enum
-    case struct: Data
-enum AltKind ['value', 'string', 'struct']
-object Data
-    member number: int optional=True
-    member name: str optional=True
-enum Enum ['hello', 'world']

From 625b251c69d983959efaeadeee12405b1a66d331 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:24 -0600
Subject: [PATCH 05/12] qapi: Move empty-enum to compile-time test

Rather than just asserting that we can parse an empty enum,
let's also make sure we can compile it, by including it in
qapi-schema-test.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile                          | 1 -
 tests/qapi-schema/enum-empty.err        | 0
 tests/qapi-schema/enum-empty.exit       | 1 -
 tests/qapi-schema/enum-empty.json       | 2 --
 tests/qapi-schema/enum-empty.out        | 2 --
 tests/qapi-schema/qapi-schema-test.json | 6 ++++++
 tests/qapi-schema/qapi-schema-test.out  | 1 +
 7 files changed, 7 insertions(+), 6 deletions(-)
 delete mode 100644 tests/qapi-schema/enum-empty.err
 delete mode 100644 tests/qapi-schema/enum-empty.exit
 delete mode 100644 tests/qapi-schema/enum-empty.json
 delete mode 100644 tests/qapi-schema/enum-empty.out

diff --git a/tests/Makefile b/tests/Makefile
index 9a6601d014..e058312726 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -260,7 +260,6 @@ qapi-schema += enum-bad-name.json
 qapi-schema += enum-bad-prefix.json
 qapi-schema += enum-clash-member.json
 qapi-schema += enum-dict-member.json
-qapi-schema += enum-empty.json
 qapi-schema += enum-int-member.json
 qapi-schema += enum-max-member.json
 qapi-schema += enum-missing-data.json
diff --git a/tests/qapi-schema/enum-empty.err b/tests/qapi-schema/enum-empty.err
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/enum-empty.exit b/tests/qapi-schema/enum-empty.exit
deleted file mode 100644
index 573541ac97..0000000000
--- a/tests/qapi-schema/enum-empty.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/enum-empty.json b/tests/qapi-schema/enum-empty.json
deleted file mode 100644
index 40d4e85a2f..0000000000
--- a/tests/qapi-schema/enum-empty.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# An empty enum, although unusual, is currently acceptable
-{ 'enum': 'MyEnum', 'data': [ ] }
diff --git a/tests/qapi-schema/enum-empty.out b/tests/qapi-schema/enum-empty.out
deleted file mode 100644
index a449d455fb..0000000000
--- a/tests/qapi-schema/enum-empty.out
+++ /dev/null
@@ -1,2 +0,0 @@
-object :empty
-enum MyEnum []
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index abe59fd137..c8cd2ddb92 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -1,11 +1,17 @@
 # *-*- Mode: Python -*-*
 
+# This file is a stress test of supported qapi constructs that must
+# parse and compile correctly.
+
 # for testing enums
 { 'enum': 'EnumOne',
   'data': [ 'value1', 'value2', 'value3' ] }
 { 'struct': 'NestedEnumsOne',
   'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
 
+# An empty enum, although unusual, is currently acceptable
+{ 'enum': 'MyEnum', 'data': [ ] }
+
 # for testing override of default naming heuristic
 { 'enum': 'QEnumTwo',
   'prefix': 'QENUM_TWO',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 8f817842df..e3504abe3d 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -86,6 +86,7 @@ object EventStructOne
     member struct1: UserDefOne optional=False
     member string: str optional=False
     member enum2: EnumOne optional=True
+enum MyEnum []
 object NestedEnumsOne
     member enum1: EnumOne optional=False
     member enum2: EnumOne optional=True

From cae95eae6270c1ea28a9ba8eee75c441b1015f68 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:25 -0600
Subject: [PATCH 06/12] qapi: Drop redundant returns-int test

qapi-schema-test was already testing that we could have a
command returning int, but burned a command name in the whitelist.
Merge the redundant positive test returns-int, and pick a name
that reduces the whitelist size.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py                         |  3 ---
 tests/Makefile                          |  1 -
 tests/qapi-schema/qapi-schema-test.json |  5 +++--
 tests/qapi-schema/qapi-schema-test.out  | 10 +++++-----
 tests/qapi-schema/returns-int.err       |  0
 tests/qapi-schema/returns-int.exit      |  1 -
 tests/qapi-schema/returns-int.json      |  3 ---
 tests/qapi-schema/returns-int.out       |  3 ---
 tests/test-qmp-commands.c               |  4 ++--
 9 files changed, 10 insertions(+), 20 deletions(-)
 delete mode 100644 tests/qapi-schema/returns-int.err
 delete mode 100644 tests/qapi-schema/returns-int.exit
 delete mode 100644 tests/qapi-schema/returns-int.json
 delete mode 100644 tests/qapi-schema/returns-int.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 4573599d7c..68f97a14bb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -56,9 +56,6 @@ returns_whitelist = [
     'guest-set-vcpus',
     'guest-sync',
     'guest-sync-delimited',
-
-    # From qapi-schema-test:
-    'user_def_cmd3',
 ]
 
 enum_types = []
diff --git a/tests/Makefile b/tests/Makefile
index e058312726..3512f8ca19 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -318,7 +318,6 @@ qapi-schema += redefined-type.json
 qapi-schema += returns-alternate.json
 qapi-schema += returns-array-bad.json
 qapi-schema += returns-dict.json
-qapi-schema += returns-int.json
 qapi-schema += returns-unknown.json
 qapi-schema += returns-whitelist.json
 qapi-schema += struct-base-clash-base.json
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c8cd2ddb92..67c6bca845 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -104,9 +104,10 @@
 { 'command': 'user_def_cmd2',
   'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
   'returns': 'UserDefTwo' }
-{ 'command': 'user_def_cmd3', 'data': {'a': 'int', '*b': 'int' },
+
+# Returning a non-dictionary requires a name from the whitelist
+{ 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
   'returns': 'int' }
-# note: command name 'guest-sync' chosen to avoid "cannot use built-in" error
 { 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
 
 # For testing integer range flattening in opts-visitor. The following schema
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index e3504abe3d..93f62503e7 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -17,6 +17,9 @@ object :obj-anyList-wrapper
     member data: anyList optional=False
 object :obj-boolList-wrapper
     member data: boolList optional=False
+object :obj-guest-get-time-arg
+    member a: int optional=False
+    member b: int optional=True
 object :obj-guest-sync-arg
     member arg: any optional=False
 object :obj-int16List-wrapper
@@ -50,9 +53,6 @@ object :obj-user_def_cmd1-arg
 object :obj-user_def_cmd2-arg
     member ud1a: UserDefOne optional=False
     member ud1b: UserDefOne optional=True
-object :obj-user_def_cmd3-arg
-    member a: int optional=False
-    member b: int optional=True
 alternate AltIntNum
     case i: int
     case n: number
@@ -184,6 +184,8 @@ object __org.qemu_x-Union2
     case __org.qemu_x-value: __org.qemu_x-Struct2
 command __org.qemu_x-command :obj-__org.qemu_x-command-arg -> __org.qemu_x-Union1
    gen=True success_response=True
+command guest-get-time :obj-guest-get-time-arg -> int
+   gen=True success_response=True
 command guest-sync :obj-guest-sync-arg -> any
    gen=True success_response=True
 command user_def_cmd None -> None
@@ -192,5 +194,3 @@ command user_def_cmd1 :obj-user_def_cmd1-arg -> None
    gen=True success_response=True
 command user_def_cmd2 :obj-user_def_cmd2-arg -> UserDefTwo
    gen=True success_response=True
-command user_def_cmd3 :obj-user_def_cmd3-arg -> int
-   gen=True success_response=True
diff --git a/tests/qapi-schema/returns-int.err b/tests/qapi-schema/returns-int.err
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/returns-int.exit b/tests/qapi-schema/returns-int.exit
deleted file mode 100644
index 573541ac97..0000000000
--- a/tests/qapi-schema/returns-int.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/returns-int.json b/tests/qapi-schema/returns-int.json
deleted file mode 100644
index 870ec6366b..0000000000
--- a/tests/qapi-schema/returns-int.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# It is okay (although not extensible) to return a non-dictionary
-# But to make it work, the name must be in a whitelist
-{ 'command': 'guest-get-time', 'returns': 'int' }
diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out
deleted file mode 100644
index a2da259be4..0000000000
--- a/tests/qapi-schema/returns-int.out
+++ /dev/null
@@ -1,3 +0,0 @@
-object :empty
-command guest-get-time None -> int
-   gen=True success_response=True
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 8d5249e7e4..bc59835835 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -46,7 +46,7 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
     return ret;
 }
 
-int64_t qmp_user_def_cmd3(int64_t a, bool has_b, int64_t b, Error **errp)
+int64_t qmp_guest_get_time(int64_t a, bool has_b, int64_t b, Error **errp)
 {
     return a + (has_b ? b : 0);
 }
@@ -160,7 +160,7 @@ static void test_dispatch_cmd_io(void)
 
     qdict_put(args3, "a", qint_from_int(66));
     qdict_put(req, "arguments", args3);
-    qdict_put(req, "execute", qstring_from_str("user_def_cmd3"));
+    qdict_put(req, "execute", qstring_from_str("guest-get-time"));
 
     ret3 = qobject_to_qint(test_qmp_dispatch(req));
     assert(qint_get_int(ret3) == 66);

From 70478cef837e86b1cff08073153081ce480e0e6c Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:26 -0600
Subject: [PATCH 07/12] qapi: Drop redundant flat-union-reverse-define test

As of commit 8c3f8e77, we test compilation of forward references
for a struct base type (UserDefOne), flat union base type
(UserDefUnionBase), and flat union branch type
(UserDefFlatUnion2). The only remaining forward reference being
tested for parsing in flat-union-reverse-define was a forward
enum declaration.  Once we make sure that always compiles,
the smaller parse-only test is redundant and can be deleted.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-7-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile                                  |  1 -
 tests/qapi-schema/flat-union-reverse-define.err |  0
 .../qapi-schema/flat-union-reverse-define.exit  |  1 -
 .../qapi-schema/flat-union-reverse-define.json  | 17 -----------------
 tests/qapi-schema/flat-union-reverse-define.out | 13 -------------
 tests/qapi-schema/qapi-schema-test.json         | 11 +++++++----
 6 files changed, 7 insertions(+), 36 deletions(-)
 delete mode 100644 tests/qapi-schema/flat-union-reverse-define.err
 delete mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
 delete mode 100644 tests/qapi-schema/flat-union-reverse-define.json
 delete mode 100644 tests/qapi-schema/flat-union-reverse-define.out

diff --git a/tests/Makefile b/tests/Makefile
index 3512f8ca19..35a63f30e6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -286,7 +286,6 @@ qapi-schema += flat-union-invalid-branch-key.json
 qapi-schema += flat-union-invalid-discriminator.json
 qapi-schema += flat-union-no-base.json
 qapi-schema += flat-union-optional-discriminator.json
-qapi-schema += flat-union-reverse-define.json
 qapi-schema += flat-union-string-discriminator.json
 qapi-schema += funny-char.json
 qapi-schema += ident-with-escape.json
diff --git a/tests/qapi-schema/flat-union-reverse-define.err b/tests/qapi-schema/flat-union-reverse-define.err
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/flat-union-reverse-define.exit b/tests/qapi-schema/flat-union-reverse-define.exit
deleted file mode 100644
index 573541ac97..0000000000
--- a/tests/qapi-schema/flat-union-reverse-define.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/flat-union-reverse-define.json b/tests/qapi-schema/flat-union-reverse-define.json
deleted file mode 100644
index 648bbfe2b7..0000000000
--- a/tests/qapi-schema/flat-union-reverse-define.json
+++ /dev/null
@@ -1,17 +0,0 @@
-{ 'union': 'TestUnion',
-  'base': 'TestBase',
-  'discriminator': 'enum1',
-  'data': { 'value1': 'TestTypeA',
-            'value2': 'TestTypeB' } }
-
-{ 'struct': 'TestBase',
-  'data': { 'enum1': 'TestEnum' } }
-
-{ 'enum': 'TestEnum',
-  'data': [ 'value1', 'value2' ] }
-
-{ 'struct': 'TestTypeA',
-  'data': { 'string': 'str' } }
-
-{ 'struct': 'TestTypeB',
-  'data': { 'integer': 'int' } }
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
deleted file mode 100644
index a5a9134e7e..0000000000
--- a/tests/qapi-schema/flat-union-reverse-define.out
+++ /dev/null
@@ -1,13 +0,0 @@
-object :empty
-object TestBase
-    member enum1: TestEnum optional=False
-enum TestEnum ['value1', 'value2']
-object TestTypeA
-    member string: str optional=False
-object TestTypeB
-    member integer: int optional=False
-object TestUnion
-    base TestBase
-    tag enum1
-    case value1: TestTypeA
-    case value2: TestTypeB
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 67c6bca845..b48c6bd740 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -4,10 +4,9 @@
 # parse and compile correctly.
 
 # for testing enums
-{ 'enum': 'EnumOne',
-  'data': [ 'value1', 'value2', 'value3' ] }
 { 'struct': 'NestedEnumsOne',
-  'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
+  'data': { 'enum1': 'EnumOne',   # Intentional forward reference
+            '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
 
 # An empty enum, although unusual, is currently acceptable
 { 'enum': 'MyEnum', 'data': [ ] }
@@ -20,7 +19,11 @@
 # for testing nested structs
 { 'struct': 'UserDefOne',
   'base': 'UserDefZero',        # intentional forward reference
-  'data': { 'string': 'str', '*enum1': 'EnumOne' } }
+  'data': { 'string': 'str',
+            '*enum1': 'EnumOne' } }   # intentional forward reference
+
+{ 'enum': 'EnumOne',
+  'data': [ 'value1', 'value2', 'value3' ] }
 
 { 'struct': 'UserDefZero',
   'data': { 'integer': 'int' } }

From 849ab13c1657b51b89693282ddd42ca1f6255354 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 13 Oct 2015 12:26:47 -0600
Subject: [PATCH 08/12] qapi: Drop redundant args-member-array test

qapi-schema-test already ensures that we can correctly compile
an array of enums (__org.qemu_x-command), an array of builtins
(UserDefNativeListUnion), and an array of structs (again
__org.qemu_x-command).  That means args-member-array is not
adding any additional parse-only test coverage, so drop it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444760807-11307-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile                           | 1 -
 tests/qapi-schema/args-member-array.err  | 0
 tests/qapi-schema/args-member-array.exit | 1 -
 tests/qapi-schema/args-member-array.json | 4 ----
 tests/qapi-schema/args-member-array.out  | 9 ---------
 5 files changed, 15 deletions(-)
 delete mode 100644 tests/qapi-schema/args-member-array.err
 delete mode 100644 tests/qapi-schema/args-member-array.exit
 delete mode 100644 tests/qapi-schema/args-member-array.json
 delete mode 100644 tests/qapi-schema/args-member-array.out

diff --git a/tests/Makefile b/tests/Makefile
index 35a63f30e6..cb221dec22 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -239,7 +239,6 @@ qapi-schema += args-array-unknown.json
 qapi-schema += args-int.json
 qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
-qapi-schema += args-member-array.json
 qapi-schema += args-member-unknown.json
 qapi-schema += args-name-clash.json
 qapi-schema += args-union.json
diff --git a/tests/qapi-schema/args-member-array.err b/tests/qapi-schema/args-member-array.err
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/args-member-array.exit b/tests/qapi-schema/args-member-array.exit
deleted file mode 100644
index 573541ac97..0000000000
--- a/tests/qapi-schema/args-member-array.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/args-member-array.json b/tests/qapi-schema/args-member-array.json
deleted file mode 100644
index e6f7f5da13..0000000000
--- a/tests/qapi-schema/args-member-array.json
+++ /dev/null
@@ -1,4 +0,0 @@
-# valid array members
-{ 'enum': 'abc', 'data': [ 'a', 'b', 'c' ] }
-{ 'struct': 'def', 'data': { 'array': [ 'abc' ] } }
-{ 'command': 'okay', 'data': { 'member1': [ 'int' ], 'member2': [ 'def' ] } }
diff --git a/tests/qapi-schema/args-member-array.out b/tests/qapi-schema/args-member-array.out
deleted file mode 100644
index b3b92dfc8e..0000000000
--- a/tests/qapi-schema/args-member-array.out
+++ /dev/null
@@ -1,9 +0,0 @@
-object :empty
-object :obj-okay-arg
-    member member1: intList optional=False
-    member member2: defList optional=False
-enum abc ['a', 'b', 'c']
-object def
-    member array: abcList optional=False
-command okay :obj-okay-arg -> None
-   gen=True success_response=True

From 49823c4b4304a3e4aa5d67e089946b12d6a52d64 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:27 -0600
Subject: [PATCH 09/12] qapi: Don't use info as witness of implicit object type

A future patch will enable error reporting from the various
QAPISchema*.check() methods.  But to report an error related
to an implicit type, we'll need to associate a location with
the type (the same location as the top-level entity that is
causing the creation of the implicit type), and once we do
that, keying off of whether foo.info exists is no longer a
viable way to determine if foo is an implicit type.

Instead, add an is_implicit() method to QAPISchemaEntity, and use it.
It can be overridden later for ObjectType and EnumType, when implicit
instances of those classes gain info.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py |  3 ++-
 scripts/qapi-visit.py |  3 ++-
 scripts/qapi.py       | 14 ++++++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 2a29c6e106..4fe618ef3c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -235,7 +235,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
 
     def visit_needed(self, entity):
         # Visit everything except implicit objects
-        return not isinstance(entity, QAPISchemaObjectType) or entity.info
+        return not (entity.is_implicit() and
+                    isinstance(entity, QAPISchemaObjectType))
 
     def _gen_type_cleanup(self, name):
         self.decl += gen_type_cleanup_decl(name)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index b7a647076d..d0759d739a 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -337,7 +337,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
 
     def visit_needed(self, entity):
         # Visit everything except implicit objects
-        return not isinstance(entity, QAPISchemaObjectType) or entity.info
+        return not (entity.is_implicit() and
+                    isinstance(entity, QAPISchemaObjectType))
 
     def visit_enum_type(self, name, info, values, prefix):
         self.decl += gen_visit_decl(name, scalar=True)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 68f97a14bb..d7cf0f3714 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -798,6 +798,9 @@ class QAPISchemaEntity(object):
     def check(self, schema):
         pass
 
+    def is_implicit(self):
+        return not self.info
+
     def visit(self, visitor):
         pass
 
@@ -971,11 +974,11 @@ class QAPISchemaObjectType(QAPISchemaType):
         self.members = members
 
     def c_name(self):
-        assert self.info
+        assert not self.is_implicit()
         return QAPISchemaType.c_name(self)
 
     def c_type(self, is_param=False):
-        assert self.info
+        assert not self.is_implicit()
         return QAPISchemaType.c_type(self)
 
     def json_type(self):
@@ -1043,7 +1046,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     # This function exists to support ugly simple union special cases
     # TODO get rid of them, and drop the function
     def simple_union_type(self):
-        if isinstance(self.type, QAPISchemaObjectType) and not self.type.info:
+        if (self.type.is_implicit() and
+                isinstance(self.type, QAPISchemaObjectType)):
             assert len(self.type.members) == 1
             assert not self.type.variants
             return self.type.members[0].type
@@ -1162,11 +1166,13 @@ class QAPISchema(object):
         self._def_entity(self.the_empty_object_type)
 
     def _make_implicit_enum_type(self, name, values):
-        name = name + 'Kind'
+        name = name + 'Kind'   # Use namespace reserved by add_name()
         self._def_entity(QAPISchemaEnumType(name, None, values, None))
         return name
 
     def _make_array_type(self, element_type):
+        # TODO fooList namespace is not reserved; user can create collisions,
+        # or abuse our type system with ['fooList'] for 2D array
         name = element_type + 'List'
         if not self.lookup_type(name):
             self._def_entity(QAPISchemaArrayType(name, None, element_type))

From 9f08c8ec73878122ad4b061ed334f0437afaaa32 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:28 -0600
Subject: [PATCH 10/12] qapi: Lazy creation of array types

Commit ac88219a had several TODO markers about whether we needed
to automatically create the corresponding array type alongside
any other type.  It turns out that most of the time, we don't!

There are a few exceptions: 1) We have a few situations where we
use an array type in internal code but do not expose that type
through QMP; fix it by declaring a dummy type that forces the
generator to see that we want to use the array type.

2) The builtin arrays (such as intList for QAPI ['int']) must
always be generated, because of the way our QAPI_TYPES_BUILTIN
compile guard works: we have situations (at the very least
tests/test-qmp-output-visitor.c) that include both top-level
"qapi-types.h" (via "error.h") and a secondary
"test-qapi-types.h". If we were to only emit the builtin types
when used locally, then the first .h file would not include all
types, but the second .h does not declare anything at all because
the first .h set QAPI_TYPES_BUILTIN, and we would end up with
compilation error due to things like unknown type 'int8List'.

Actually, we may need to revisit how we do type guards, and
change from a single QAPI_TYPES_BUILTIN over to a different
usage pattern that does one #ifdef per qapi type - right now,
the only types that are declared multiple times between two qapi
.json files for inclusion by a single .c file happen to be the
builtin arrays.  But now that we have QAPI 'include' statements,
it is logical to assume that we will soon reach a point where
we want to reuse non-builtin types (yes, I'm thinking about what
it will take to add introspection to QGA, where we will want to
reuse the SchemaInfo type and friends).  One #ifdef per type
will help ensure that generating the same qapi type into more
than one qapi-types.h won't cause collisions when both are
included in the same .c file; but we also have to solve how to
avoid creating duplicate qapi-types.c entry points.  So that
is a problem left for another day.

Generated code for qapi-types and qapi-visit is drastically
reduced; less than a third of the arrays that were blindly
created were actually needed (a quick grep shows we dropped
from 219 to 69 *List types), and the .o files lost more than
30% of their bulk.  [For best results, diff the generated
files with 'git diff --patience --no-index pre post'.]

Interestingly, the introspection output is unchanged - this is
because we already cull all types that are not indirectly
reachable from a command or event, so introspection was already
using only a subset of array types.  The subset of types
introspected is now a much larger percentage of the overall set
of array types emitted in qapi-types.h (since the larger set
shrunk), but still not 100% (evidence that the array types
emitted for our new Dummy structs, and the new struct itself,
don't affect QMP).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-9-git-send-email-eblake@redhat.com>
[Moved array info tracking to a later patch]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi-schema.json                        | 11 +++++++++++
 scripts/qapi.py                         | 11 ++++++-----
 tests/qapi-schema/qapi-schema-test.json |  4 ++++
 tests/qapi-schema/qapi-schema-test.out  |  3 +++
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index a386605b6a..702b7b5dbd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3415,6 +3415,17 @@
             'cpuid-register': 'X86CPURegister32',
             'features': 'int' } }
 
+##
+# @DummyForceArrays
+#
+# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
+#
+# Since 2.5
+##
+{ 'struct': 'DummyForceArrays',
+  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
+
+
 ##
 # @RxState:
 #
diff --git a/scripts/qapi.py b/scripts/qapi.py
index d7cf0f3714..9e017050c0 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1143,7 +1143,12 @@ class QAPISchema(object):
     def _def_builtin_type(self, name, json_type, c_type, c_null):
         self._def_entity(QAPISchemaBuiltinType(name, json_type,
                                                c_type, c_null))
-        self._make_array_type(name)     # TODO really needed?
+        # TODO As long as we have QAPI_TYPES_BUILTIN to share multiple
+        # qapi-types.h from a single .c, all arrays of builtins must be
+        # declared in the first file whether or not they are used.  Nicer
+        # would be to use lazy instantiation, while figuring out how to
+        # avoid compilation issues with multiple qapi-types.h.
+        self._make_array_type(name)
 
     def _def_predefineds(self):
         for t in [('str',    'string',  'char' + pointer_suffix, 'NULL'),
@@ -1192,7 +1197,6 @@ class QAPISchema(object):
         data = expr['data']
         prefix = expr.get('prefix')
         self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
-        self._make_array_type(name)     # TODO really needed?
 
     def _make_member(self, name, typ):
         optional = False
@@ -1215,7 +1219,6 @@ class QAPISchema(object):
         self._def_entity(QAPISchemaObjectType(name, info, base,
                                               self._make_members(data),
                                               None))
-        self._make_array_type(name)     # TODO really needed?
 
     def _make_variant(self, case, typ):
         return QAPISchemaObjectTypeVariant(case, typ)
@@ -1251,7 +1254,6 @@ class QAPISchema(object):
                                  QAPISchemaObjectTypeVariants(tag_name,
                                                               tag_enum,
                                                               variants)))
-        self._make_array_type(name)     # TODO really needed?
 
     def _def_alternate_type(self, expr, info):
         name = expr['alternate']
@@ -1264,7 +1266,6 @@ class QAPISchema(object):
                                     QAPISchemaObjectTypeVariants(None,
                                                                  tag_enum,
                                                                  variants)))
-        self._make_array_type(name)     # TODO really needed?
 
     def _def_command(self, expr, info):
         name = expr['command']
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index b48c6bd740..4e2d7c2063 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -40,6 +40,10 @@
   'data': { 'string0': 'str',
             'dict1': 'UserDefTwoDict' } }
 
+# dummy struct to force generation of array types not otherwise mentioned
+{ 'struct': 'ForceArrays',
+  'data': { 'unused1':['UserDefOne'], 'unused2':['UserDefTwo'] } }
+
 # for testing unions
 # Among other things, test that a name collision between branches does
 # not cause any problems (since only one branch can be in use at a time),
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 93f62503e7..a6c80e04d7 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -86,6 +86,9 @@ object EventStructOne
     member struct1: UserDefOne optional=False
     member string: str optional=False
     member enum2: EnumOne optional=True
+object ForceArrays
+    member unused1: UserDefOneList optional=False
+    member unused2: UserDefTwoList optional=False
 enum MyEnum []
 object NestedEnumsOne
     member enum1: EnumOne optional=False

From 46292ba75c515baf733df18644052b2ce9492728 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:29 -0600
Subject: [PATCH 11/12] qapi: Create simple union type member earlier

For simple unions, we were creating the implicit 'type' tag
member during the QAPISchemaObjectTypeVariants constructor.
This is different from every other implicit QAPISchemaEntity
object, which get created by QAPISchema methods.  Hoist the
creation to the caller (renaming _make_tag_enum() to
_make_implicit_tag()), and pass the entity rather than the
string name, so that we have the nice property that no
entities are created as a side effect within a different
entity.  A later patch will then have an easier time of
associating location info with each entity creation.

No change to generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9e017050c0..471bbfc406 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1010,18 +1010,18 @@ class QAPISchemaObjectTypeMember(object):
 
 
 class QAPISchemaObjectTypeVariants(object):
-    def __init__(self, tag_name, tag_enum, variants):
-        assert tag_name is None or isinstance(tag_name, str)
-        assert tag_enum is None or isinstance(tag_enum, str)
+    def __init__(self, tag_name, tag_member, variants):
+        # Flat unions pass tag_name but not tag_member.
+        # Simple unions and alternates pass tag_member but not tag_name.
+        # After check(), tag_member is always set, and tag_name remains
+        # a reliable witness of being used by a flat union.
+        assert bool(tag_member) != bool(tag_name)
+        assert (isinstance(tag_name, str) or
+                isinstance(tag_member, QAPISchemaObjectTypeMember))
         for v in variants:
             assert isinstance(v, QAPISchemaObjectTypeVariant)
         self.tag_name = tag_name
-        if tag_name:
-            assert not tag_enum
-            self.tag_member = None
-        else:
-            self.tag_member = QAPISchemaObjectTypeMember('type', tag_enum,
-                                                         False)
+        self.tag_member = tag_member
         self.variants = variants
 
     def check(self, schema, members, seen):
@@ -1231,28 +1231,29 @@ class QAPISchema(object):
                                               [self._make_member('data', typ)])
         return QAPISchemaObjectTypeVariant(case, typ)
 
-    def _make_tag_enum(self, type_name, variants):
-        return self._make_implicit_enum_type(type_name,
-                                             [v.name for v in variants])
+    def _make_implicit_tag(self, type_name, variants):
+        typ = self._make_implicit_enum_type(type_name,
+                                            [v.name for v in variants])
+        return QAPISchemaObjectTypeMember('type', typ, False)
 
     def _def_union_type(self, expr, info):
         name = expr['union']
         data = expr['data']
         base = expr.get('base')
         tag_name = expr.get('discriminator')
-        tag_enum = None
+        tag_member = None
         if tag_name:
             variants = [self._make_variant(key, value)
                         for (key, value) in data.iteritems()]
         else:
             variants = [self._make_simple_variant(key, value)
                         for (key, value) in data.iteritems()]
-            tag_enum = self._make_tag_enum(name, variants)
+            tag_member = self._make_implicit_tag(name, variants)
         self._def_entity(
             QAPISchemaObjectType(name, info, base,
                                  self._make_members(OrderedDict()),
                                  QAPISchemaObjectTypeVariants(tag_name,
-                                                              tag_enum,
+                                                              tag_member,
                                                               variants)))
 
     def _def_alternate_type(self, expr, info):
@@ -1260,11 +1261,11 @@ class QAPISchema(object):
         data = expr['data']
         variants = [self._make_variant(key, value)
                     for (key, value) in data.iteritems()]
-        tag_enum = self._make_tag_enum(name, variants)
+        tag_member = self._make_implicit_tag(name, variants)
         self._def_entity(
             QAPISchemaAlternateType(name, info,
                                     QAPISchemaObjectTypeVariants(None,
-                                                                 tag_enum,
+                                                                 tag_member,
                                                                  variants)))
 
     def _def_command(self, expr, info):

From 99df5289d8c7ebf373c3570d8fba3f3a73360281 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 12 Oct 2015 22:22:32 -0600
Subject: [PATCH 12/12] qapi: Track location that created an implicit type

A future patch will move some error checking from the parser
to the various QAPISchema*.check() methods, which run only
after parsing completes.  It will thus be possible to create
a python instance representing an implicit QAPI type that
parses fine but will fail validation during check().  Since
all errors have to have an associated 'info' location, we
need a location to be associated with those implicit types.
The intuitive info to use is the location of the enclosing
entity that caused the creation of the implicit type.

Note that we do not anticipate builtin types being used in
an error message (as they are not part of the user's QAPI
input, the user can't cause a semantic error in their
behavior), so we exempt those types from requiring info, by
setting a flag to track the completion of _def_predefineds(),
and tracking that flag in _def_entity().

No change to the generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-13-git-send-email-eblake@redhat.com>
[Missing QAPISchemaArrayType.is_implicit() supplied]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 74 +++++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 27 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 471bbfc406..9d53255320 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -790,6 +790,11 @@ class QAPISchemaEntity(object):
     def __init__(self, name, info):
         assert isinstance(name, str)
         self.name = name
+        # For explicitly defined entities, info points to the (explicit)
+        # definition.  For builtins (and their arrays), info is None.
+        # For implicitly defined entities, info points to a place that
+        # triggered the implicit definition (there may be more than one
+        # such place).
         self.info = info
 
     def c_name(self):
@@ -903,6 +908,10 @@ class QAPISchemaEnumType(QAPISchemaType):
     def check(self, schema):
         assert len(set(self.values)) == len(self.values)
 
+    def is_implicit(self):
+        # See QAPISchema._make_implicit_enum_type()
+        return self.name[-4:] == 'Kind'
+
     def c_type(self, is_param=False):
         return c_name(self.name)
 
@@ -929,6 +938,9 @@ class QAPISchemaArrayType(QAPISchemaType):
         self.element_type = schema.lookup_type(self._element_type_name)
         assert self.element_type
 
+    def is_implicit(self):
+        return True
+
     def json_type(self):
         return 'array'
 
@@ -973,6 +985,10 @@ class QAPISchemaObjectType(QAPISchemaType):
             self.variants.check(schema, members, seen)
         self.members = members
 
+    def is_implicit(self):
+        # See QAPISchema._make_implicit_object_type()
+        return self.name[0] == ':'
+
     def c_name(self):
         assert not self.is_implicit()
         return QAPISchemaType.c_name(self)
@@ -1120,7 +1136,9 @@ class QAPISchema(object):
         try:
             self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs)
             self._entity_dict = {}
+            self._predefining = True
             self._def_predefineds()
+            self._predefining = False
             self._def_exprs()
             self.check()
         except (QAPISchemaError, QAPIExprError), err:
@@ -1128,6 +1146,8 @@ class QAPISchema(object):
             exit(1)
 
     def _def_entity(self, ent):
+        # Only the predefined types are allowed to not have info
+        assert ent.info or self._predefining
         assert ent.name not in self._entity_dict
         self._entity_dict[ent.name] = ent
 
@@ -1148,7 +1168,7 @@ class QAPISchema(object):
         # declared in the first file whether or not they are used.  Nicer
         # would be to use lazy instantiation, while figuring out how to
         # avoid compilation issues with multiple qapi-types.h.
-        self._make_array_type(name)
+        self._make_array_type(name, None)
 
     def _def_predefineds(self):
         for t in [('str',    'string',  'char' + pointer_suffix, 'NULL'),
@@ -1170,25 +1190,25 @@ class QAPISchema(object):
                                                           [], None)
         self._def_entity(self.the_empty_object_type)
 
-    def _make_implicit_enum_type(self, name, values):
+    def _make_implicit_enum_type(self, name, info, values):
         name = name + 'Kind'   # Use namespace reserved by add_name()
-        self._def_entity(QAPISchemaEnumType(name, None, values, None))
+        self._def_entity(QAPISchemaEnumType(name, info, values, None))
         return name
 
-    def _make_array_type(self, element_type):
+    def _make_array_type(self, element_type, info):
         # TODO fooList namespace is not reserved; user can create collisions,
         # or abuse our type system with ['fooList'] for 2D array
         name = element_type + 'List'
         if not self.lookup_type(name):
-            self._def_entity(QAPISchemaArrayType(name, None, element_type))
+            self._def_entity(QAPISchemaArrayType(name, info, element_type))
         return name
 
-    def _make_implicit_object_type(self, name, role, members):
+    def _make_implicit_object_type(self, name, info, role, members):
         if not members:
             return None
         name = ':obj-%s-%s' % (name, role)
         if not self.lookup_entity(name, QAPISchemaObjectType):
-            self._def_entity(QAPISchemaObjectType(name, None, None,
+            self._def_entity(QAPISchemaObjectType(name, info, None,
                                                   members, None))
         return name
 
@@ -1198,18 +1218,18 @@ class QAPISchema(object):
         prefix = expr.get('prefix')
         self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
 
-    def _make_member(self, name, typ):
+    def _make_member(self, name, typ, info):
         optional = False
         if name.startswith('*'):
             name = name[1:]
             optional = True
         if isinstance(typ, list):
             assert len(typ) == 1
-            typ = self._make_array_type(typ[0])
+            typ = self._make_array_type(typ[0], info)
         return QAPISchemaObjectTypeMember(name, typ, optional)
 
-    def _make_members(self, data):
-        return [self._make_member(key, value)
+    def _make_members(self, data, info):
+        return [self._make_member(key, value, info)
                 for (key, value) in data.iteritems()]
 
     def _def_struct_type(self, expr, info):
@@ -1217,22 +1237,22 @@ class QAPISchema(object):
         base = expr.get('base')
         data = expr['data']
         self._def_entity(QAPISchemaObjectType(name, info, base,
-                                              self._make_members(data),
+                                              self._make_members(data, info),
                                               None))
 
     def _make_variant(self, case, typ):
         return QAPISchemaObjectTypeVariant(case, typ)
 
-    def _make_simple_variant(self, case, typ):
+    def _make_simple_variant(self, case, typ, info):
         if isinstance(typ, list):
             assert len(typ) == 1
-            typ = self._make_array_type(typ[0])
-        typ = self._make_implicit_object_type(typ, 'wrapper',
-                                              [self._make_member('data', typ)])
+            typ = self._make_array_type(typ[0], info)
+        typ = self._make_implicit_object_type(
+            typ, info, 'wrapper', [self._make_member('data', typ, info)])
         return QAPISchemaObjectTypeVariant(case, typ)
 
-    def _make_implicit_tag(self, type_name, variants):
-        typ = self._make_implicit_enum_type(type_name,
+    def _make_implicit_tag(self, type_name, info, variants):
+        typ = self._make_implicit_enum_type(type_name, info,
                                             [v.name for v in variants])
         return QAPISchemaObjectTypeMember('type', typ, False)
 
@@ -1246,12 +1266,12 @@ class QAPISchema(object):
             variants = [self._make_variant(key, value)
                         for (key, value) in data.iteritems()]
         else:
-            variants = [self._make_simple_variant(key, value)
+            variants = [self._make_simple_variant(key, value, info)
                         for (key, value) in data.iteritems()]
-            tag_member = self._make_implicit_tag(name, variants)
+            tag_member = self._make_implicit_tag(name, info, variants)
         self._def_entity(
             QAPISchemaObjectType(name, info, base,
-                                 self._make_members(OrderedDict()),
+                                 self._make_members(OrderedDict(), info),
                                  QAPISchemaObjectTypeVariants(tag_name,
                                                               tag_member,
                                                               variants)))
@@ -1261,7 +1281,7 @@ class QAPISchema(object):
         data = expr['data']
         variants = [self._make_variant(key, value)
                     for (key, value) in data.iteritems()]
-        tag_member = self._make_implicit_tag(name, variants)
+        tag_member = self._make_implicit_tag(name, info, variants)
         self._def_entity(
             QAPISchemaAlternateType(name, info,
                                     QAPISchemaObjectTypeVariants(None,
@@ -1275,11 +1295,11 @@ class QAPISchema(object):
         gen = expr.get('gen', True)
         success_response = expr.get('success-response', True)
         if isinstance(data, OrderedDict):
-            data = self._make_implicit_object_type(name, 'arg',
-                                                   self._make_members(data))
+            data = self._make_implicit_object_type(
+                name, info, 'arg', self._make_members(data, info))
         if isinstance(rets, list):
             assert len(rets) == 1
-            rets = self._make_array_type(rets[0])
+            rets = self._make_array_type(rets[0], info)
         self._def_entity(QAPISchemaCommand(name, info, data, rets, gen,
                                            success_response))
 
@@ -1287,8 +1307,8 @@ class QAPISchema(object):
         name = expr['event']
         data = expr.get('data')
         if isinstance(data, OrderedDict):
-            data = self._make_implicit_object_type(name, 'arg',
-                                                   self._make_members(data))
+            data = self._make_implicit_object_type(
+                name, info, 'arg', self._make_members(data, info))
         self._def_entity(QAPISchemaEvent(name, info, data))
 
     def _def_exprs(self):