diff mbox

[RFC,v4,29/32] qapi: Pseudo-type '**' is now unused, drop it

Message ID 1441290623-13631-30-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Sept. 3, 2015, 2:30 p.m. UTC
'gen': false needs to stay for now, because netdev_add is still using
it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt                    | 18 ++++++------------
 scripts/qapi.py                           | 20 ++++----------------
 tests/Makefile                            |  2 +-
 tests/qapi-schema/type-bypass-no-gen.err  |  1 -
 tests/qapi-schema/type-bypass-no-gen.exit |  1 -
 tests/qapi-schema/type-bypass-no-gen.json |  2 --
 tests/qapi-schema/type-bypass-no-gen.out  |  0
 tests/qapi-schema/type-bypass.err         |  0
 tests/qapi-schema/type-bypass.exit        |  1 -
 tests/qapi-schema/type-bypass.json        |  2 --
 tests/qapi-schema/type-bypass.out         |  4 ----
 11 files changed, 11 insertions(+), 40 deletions(-)
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.err
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.exit
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.json
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.out
 delete mode 100644 tests/qapi-schema/type-bypass.err
 delete mode 100644 tests/qapi-schema/type-bypass.exit
 delete mode 100644 tests/qapi-schema/type-bypass.json
 delete mode 100644 tests/qapi-schema/type-bypass.out

Comments

Eric Blake Sept. 3, 2015, 8:42 p.m. UTC | #1
On 09/03/2015 08:30 AM, Markus Armbruster wrote:
> 'gen': false needs to stay for now, because netdev_add is still using
> it.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---

> +++ b/tests/Makefile
> @@ -228,7 +228,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
>  	bad-type-dict.json double-data.json unknown-expr-key.json \
>  	redefined-type.json redefined-command.json redefined-builtin.json \
>  	redefined-event.json command-int.json bad-data.json event-max.json \
> -	type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
> +	type-bypass-bad-gen.json \
>  	args-invalid.json \
>  	args-array-empty.json args-array-unknown.json args-int.json \
>  	args-unknown.json args-member-unknown.json args-member-array.json \

Not for this patch, but we aren't very consistent on any form of sorting
or line length in this section. It might be nicer if it were one test
per line (lots more line continuations) and/or alphabetical order.  If
that sounds nice, then it's a trivial patch to add in as one of the
followups after this series lands.
Markus Armbruster Sept. 4, 2015, 7:14 a.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 09/03/2015 08:30 AM, Markus Armbruster wrote:
>> 'gen': false needs to stay for now, because netdev_add is still using
>> it.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>
>> +++ b/tests/Makefile
>> @@ -228,7 +228,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
>>  	bad-type-dict.json double-data.json unknown-expr-key.json \
>>  	redefined-type.json redefined-command.json redefined-builtin.json \
>>  	redefined-event.json command-int.json bad-data.json event-max.json \
>> -	type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
>> +	type-bypass-bad-gen.json \
>>  	args-invalid.json \
>>  	args-array-empty.json args-array-unknown.json args-int.json \
>>  	args-unknown.json args-member-unknown.json args-member-array.json \
>
> Not for this patch, but we aren't very consistent on any form of sorting
> or line length in this section. It might be nicer if it were one test
> per line (lots more line continuations) and/or alphabetical order.  If
> that sounds nice, then it's a trivial patch to add in as one of the
> followups after this series lands.

Sorting: yes, please!  One per line: meh.
diff mbox

Patch

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a5fccd4..ce32d74 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -111,10 +111,7 @@  and field names within a type, should be all lower case with words
 separated by a hyphen.  However, some existing older commands and
 complex types use underscore; when extending such expressions,
 consistency is preferred over blindly avoiding underscore.  Event
-names should be ALL_CAPS with words separated by underscore.  The
-special string '**' appears for some commands that manually perform
-their own type checking rather than relying on the type-safe code
-produced by the qapi code generators.
+names should be ALL_CAPS with words separated by underscore.
 
 Any name (command, event, type, field, or enum value) beginning with
 "x-" is marked experimental, and may be withdrawn or changed
@@ -453,14 +450,11 @@  which would validate this Client JSON Protocol transaction:
  <= { "return": [ { "value": "one" }, { } ] }
 
 In rare cases, QAPI cannot express a type-safe representation of a
-corresponding Client JSON Protocol command.  In these cases, if the
-command expression includes the key 'gen' with boolean value false,
-then the 'data' or 'returns' member that intends to bypass generated
-type-safety and do its own manual validation should use an inline
-dictionary definition, with a value of '**' rather than a valid type
-name for the keys that the generated code will not validate.  Please
-try to avoid adding new commands that rely on this, and instead use
-type-safe unions.  For an example of bypass usage:
+corresponding Client JSON Protocol command.  You then have to suppress
+generation of a marshalling function by including a key 'gen' with
+boolean value false, and instead write your own function.  Please try
+to avoid adding new commands that rely on this, and instead use
+type-safe unions.  For an example of this usage:
 
  { 'command': 'netdev_add',
    'data': {'type': 'str', 'id': 'str'},
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ba1d6ec..7fe9f30 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -428,15 +428,12 @@  def is_enum(name):
 
 def check_type(expr_info, source, value, allow_array = False,
                allow_dict = False, allow_optional = False,
-               allow_star = False, allow_metas = []):
+               allow_metas = []):
     global all_names
 
     if value is None:
         return
 
-    if allow_star and value == '**':
-        return
-
     # Check if array type for value is okay
     if isinstance(value, list):
         if not allow_array:
@@ -450,10 +447,6 @@  def check_type(expr_info, source, value, allow_array = False,
 
     # Check if type name for value is okay
     if isinstance(value, str):
-        if value == '**':
-            raise QAPIExprError(expr_info,
-                                "%s uses '**' but did not request 'gen':false"
-                                % source)
         if not value in all_names:
             raise QAPIExprError(expr_info,
                                 "%s uses unknown type '%s'"
@@ -479,7 +472,7 @@  def check_type(expr_info, source, value, allow_array = False,
         # Todo: allow dictionaries to represent default values of
         # an optional argument.
         check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
-                   allow_array=True, allow_star=allow_star,
+                   allow_array=True,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
                                 'enum'])
 
@@ -499,18 +492,16 @@  def check_member_clash(expr_info, base_name, data, source = ""):
 
 def check_command(expr, expr_info):
     name = expr['command']
-    allow_star = expr.has_key('gen')
 
     check_type(expr_info, "'data' for command '%s'" % name,
                expr.get('data'), allow_dict=True, allow_optional=True,
-               allow_metas=['struct'], allow_star=allow_star)
+               allow_metas=['struct'])
     returns_meta = ['union', 'struct']
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
     check_type(expr_info, "'returns' for command '%s'" % name,
                expr.get('returns'), allow_array=True,
-               allow_optional=True, allow_metas=returns_meta,
-               allow_star=allow_star)
+               allow_optional=True, allow_metas=returns_meta)
 
 def check_event(expr, expr_info):
     global events
@@ -1056,7 +1047,6 @@  class QAPISchema(object):
                   ('bool',   'boolean', 'bool',     'false'),
                   ('any',    'value',   'QObject' + pointer_suffix , 'NULL')]:
             self._def_builtin_type(*t)
-        self.entity_dict['**'] = self.lookup_type('any') # TODO drop this alias
 
     def _make_implicit_enum_type(self, name, values):
         name = name + 'Kind'
@@ -1203,8 +1193,6 @@  class QAPISchema(object):
     def visit(self, visitor):
         visitor.visit_begin(self)
         for name in sorted(self.entity_dict.keys()):
-            if self.entity_dict[name].name != name:
-                continue        # ignore alias TODO drop alias and remove
             self.entity_dict[name].visit(visitor)
         visitor.visit_end()
 
diff --git a/tests/Makefile b/tests/Makefile
index fc6169a..68adad4 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -228,7 +228,7 @@  check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	bad-type-dict.json double-data.json unknown-expr-key.json \
 	redefined-type.json redefined-command.json redefined-builtin.json \
 	redefined-event.json command-int.json bad-data.json event-max.json \
-	type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
+	type-bypass-bad-gen.json \
 	args-invalid.json \
 	args-array-empty.json args-array-unknown.json args-int.json \
 	args-unknown.json args-member-unknown.json args-member-array.json \
diff --git a/tests/qapi-schema/type-bypass-no-gen.err b/tests/qapi-schema/type-bypass-no-gen.err
deleted file mode 100644
index 20cef0a..0000000
--- a/tests/qapi-schema/type-bypass-no-gen.err
+++ /dev/null
@@ -1 +0,0 @@ 
-tests/qapi-schema/type-bypass-no-gen.json:2: Member 'arg' of 'data' for command 'unsafe' uses '**' but did not request 'gen':false
diff --git a/tests/qapi-schema/type-bypass-no-gen.exit b/tests/qapi-schema/type-bypass-no-gen.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/type-bypass-no-gen.exit
+++ /dev/null
@@ -1 +0,0 @@ 
-1
diff --git a/tests/qapi-schema/type-bypass-no-gen.json b/tests/qapi-schema/type-bypass-no-gen.json
deleted file mode 100644
index 4feae37..0000000
--- a/tests/qapi-schema/type-bypass-no-gen.json
+++ /dev/null
@@ -1,2 +0,0 @@ 
-# type bypass only works with 'gen':false
-{ 'command': 'unsafe', 'data': { 'arg': '**' }, 'returns': '**' }
diff --git a/tests/qapi-schema/type-bypass-no-gen.out b/tests/qapi-schema/type-bypass-no-gen.out
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/type-bypass.err b/tests/qapi-schema/type-bypass.err
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/type-bypass.exit b/tests/qapi-schema/type-bypass.exit
deleted file mode 100644
index 573541a..0000000
--- a/tests/qapi-schema/type-bypass.exit
+++ /dev/null
@@ -1 +0,0 @@ 
-0
diff --git a/tests/qapi-schema/type-bypass.json b/tests/qapi-schema/type-bypass.json
deleted file mode 100644
index 48b2137..0000000
--- a/tests/qapi-schema/type-bypass.json
+++ /dev/null
@@ -1,2 +0,0 @@ 
-# Use of 'gen':false allows bypassing type system
-{ 'command': 'unsafe', 'data': { 'arg': '**' }, 'returns': '**', 'gen': false }
diff --git a/tests/qapi-schema/type-bypass.out b/tests/qapi-schema/type-bypass.out
deleted file mode 100644
index db2a4e6..0000000
--- a/tests/qapi-schema/type-bypass.out
+++ /dev/null
@@ -1,4 +0,0 @@ 
-object :obj-unsafe-arg
-    member arg: any optional=False
-command unsafe :obj-unsafe-arg -> any
-   gen=False success_response=True