[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table |
Date: |
Wed, 23 Sep 2020 16:36:44 -0400 |
On Tue, Sep 22, 2020 at 05:13:13PM -0400, John Snow wrote:
> This enforces a type signature against all of the top-level expression
> check routines without necessarily needing to create some
> overcomplicated class hierarchy for them.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/expr.py | 69 ++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 74b2681ef8..cfd342aa04 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -31,8 +31,11 @@
> structures and contextual semantic validation.
> """
>
> +from enum import Enum
> import re
> from typing import (
> + Callable,
> + Dict,
> Iterable,
> List,
> MutableMapping,
> @@ -494,6 +497,26 @@ def check_event(expr: Expression, info: QAPISourceInfo)
> -> None:
> check_type(args, info, "'data'", allow_dict=not boxed)
>
>
> +class ExpressionType(str, Enum):
> + INCLUDE = 'include'
> + ENUM = 'enum'
> + UNION = 'union'
> + ALTERNATE = 'alternate'
> + STRUCT = 'struct'
> + COMMAND = 'command'
> + EVENT = 'event'
> +
> +
> +_CHECK_FN: Dict[str, Callable[[Expression, QAPISourceInfo], None]] = {
> + 'enum': check_enum,
> + 'union': check_union,
> + 'alternate': check_alternate,
> + 'struct': check_struct,
> + 'command': check_command,
> + 'event': check_event,
> +}
> +
> +
> def check_exprs(exprs: List[_JSObject]) -> List[_JSObject]:
> """
> Validate and normalize a list of parsed QAPI schema expressions. [RW]
> @@ -519,28 +542,20 @@ def check_exprs(exprs: List[_JSObject]) ->
> List[_JSObject]:
> assert tmp is None or isinstance(tmp, QAPIDoc)
> doc: Optional[QAPIDoc] = tmp
>
> - if 'include' in expr:
> - continue
> -
> - if 'enum' in expr:
> - meta = 'enum'
> - elif 'union' in expr:
> - meta = 'union'
> - elif 'alternate' in expr:
> - meta = 'alternate'
> - elif 'struct' in expr:
> - meta = 'struct'
> - elif 'command' in expr:
> - meta = 'command'
> - elif 'event' in expr:
> - meta = 'event'
> + for kind in ExpressionType:
> + if kind in expr:
> + meta = kind
I see lots of meta.value expressions below. Maybe setting
meta = kind.value
will make the code more readable?
I don't think this should block an obvious improvement to the
code, so:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> + break
> else:
> raise QAPISemError(info, "expression is missing metatype")
>
> + if meta == ExpressionType.INCLUDE:
> + continue
> +
> name = cast(str, expr[meta]) # asserted right below:
> - check_name_is_str(name, info, "'%s'" % meta)
> - info.set_defn(meta, name)
> - check_defn_name_str(name, info, meta)
> + check_name_is_str(name, info, "'%s'" % meta.value)
> + info.set_defn(meta.value, name)
> + check_defn_name_str(name, info, meta.value)
>
> if doc:
> if doc.symbol != name:
> @@ -551,22 +566,8 @@ def check_exprs(exprs: List[_JSObject]) ->
> List[_JSObject]:
> raise QAPISemError(info,
> "documentation comment required")
>
> - if meta == 'enum':
> - check_enum(expr, info)
> - elif meta == 'union':
> - check_union(expr, info)
> - elif meta == 'alternate':
> - check_alternate(expr, info)
> - elif meta == 'struct':
> - check_struct(expr, info)
> - elif meta == 'command':
> - check_command(expr, info)
> - elif meta == 'event':
> - check_event(expr, info)
> - else:
> - assert False, 'unexpected meta type'
> -
> - check_if(expr, info, meta)
> + _CHECK_FN[meta](expr, info)
> + check_if(expr, info, meta.value)
> check_features(expr.get('features'), info)
> check_flags(expr, info)
>
> --
> 2.26.2
>
--
Eduardo
- Re: [PATCH 13/16] qapi/expr.py: Modify check_keys to accept any Iterable, (continued)
- [PATCH 15/16] qapi/expr.py: move related checks inside check_xxx functions, John Snow, 2020/09/22
- [PATCH 14/16] qapi/expr.py: Use tuples instead of lists for static data, John Snow, 2020/09/22
- [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, John Snow, 2020/09/22
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table,
Eduardo Habkost <=
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, Cleber Rosa, 2020/09/24
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, John Snow, 2020/09/24
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, Helio Loureiro, 2020/09/25
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, John Snow, 2020/09/25
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, Helio Loureiro, 2020/09/26
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, John Snow, 2020/09/30
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, Cleber Rosa, 2020/09/25
- Re: [PATCH 16/16] qapi/expr.py: Use an expression checker dispatch table, John Snow, 2020/09/25
Re: [PATCH 00/16] qapi: static typing conversion, pt2, John Snow, 2020/09/25