Re: [Qemu-devel] [PATCH 3/9] qapi.py: Restructure lexer and parser

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

On 07/26/2013 06:39 AM, Markus Armbruster wrote:
> The parser has a rather unorthodox structure:
> 

> 
> Replace this piece of original art by a thoroughly unoriginal design.

I sense a bit of tongue-in-cheek urge to use more colorful language at
this point, describing the horrors of the old implementation.  Thank you
for thinking of the children :)

> Takes care of (1), (2), (5), (6) and (7), and lays the groundwork for
> addressing the others.  Generated source files remain unchanged.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py                                | 163 +++++++++++++------------
>  tests/qapi-schema/indented-expr.out            |   2 +-
>  tests/qapi-schema/missing-colon.out            |   4 +-
>  tests/qapi-schema/quoted-structural-chars.err  |   1 +
>  tests/qapi-schema/quoted-structural-chars.exit |   2 +-
>  tests/qapi-schema/quoted-structural-chars.out  |   3 -
>  6 files changed, 88 insertions(+), 87 deletions(-)

My python review skills are weak, so take this review with a grain of salt.

> +
> +    def get_members(self):
> +        expr = OrderedDict()
> +        while self.tok != '}':
> +            key = self.val
> +            self.accept()
> +            self.accept()        # :
> +            expr[key] = self.get_expr()
> +            if self.tok == ',':
> +                self.accept()

My comments in 1/9 about a test of trailing comma behavior may impact
this code.

> +        self.accept()
> +        return expr
> +
> +    def get_values(self):
> +        expr = []
> +        while self.tok != ']':
> +            expr.append(self.get_expr())
> +            if self.tok == ',':
> +                self.accept()

and this code.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]