| Message ID | 20210223003408.964543-14-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt3 | expand |
John Snow <jsnow@redhat.com> writes: > This is a minor adjustment that allows the 'required' and 'optional' > keys fields to take a default value of an empty, immutable sequence (the > empty tuple). Quite marginal, but simple enough for me not to say no to it. > This reveals a quirk of this function, which is that "a + b" is > list-specific behavior. We can accept a wider variety of types if we > avoid that behavior. Using Collection allows us to accept things like > lists, tuples, sets, and so on. > > (Iterable would also have worked, but Iterable also includes things like > generator expressions which are consumed upon iteration, which would > require a rewrite to make sure that each input was only traversed once.) Totally not worth it now :) > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/expr.py | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 2b96bec722f..0b841f292d7 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -33,6 +33,7 @@ > > import re > from typing import ( > + Collection, > Iterable, > List, > MutableMapping, > @@ -133,8 +134,8 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None: > def check_keys(value: _JSObject, > info: QAPISourceInfo, > source: str, > - required: List[str], > - optional: List[str]) -> None: > + required: Collection[str] = (), > + optional: Collection[str] = ()) -> None: > """ > Ensures an object has a specific set of keys. [Const] > Any particular reason not to squash this part into PATCH 08? Oh, I figure it's because it requires the next hunk, and squashing that one would kill PATCH 08's "This commit *only* adds annotations." What about putting the next hunk before 08, and squash the remainder? > @@ -155,7 +156,7 @@ def pprint(elems: Iterable[str]) -> str: > "%s misses key%s %s" > % (source, 's' if len(missing) > 1 else '', > pprint(missing))) > - allowed = set(required + optional) > + allowed = set(required) | set(optional) > unknown = set(value) - allowed > if unknown: > raise QAPISemError(
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 2b96bec722f..0b841f292d7 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -33,6 +33,7 @@ import re from typing import ( + Collection, Iterable, List, MutableMapping, @@ -133,8 +134,8 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None: def check_keys(value: _JSObject, info: QAPISourceInfo, source: str, - required: List[str], - optional: List[str]) -> None: + required: Collection[str] = (), + optional: Collection[str] = ()) -> None: """ Ensures an object has a specific set of keys. [Const] @@ -155,7 +156,7 @@ def pprint(elems: Iterable[str]) -> str: "%s misses key%s %s" % (source, 's' if len(missing) > 1 else '', pprint(missing))) - allowed = set(required + optional) + allowed = set(required) | set(optional) unknown = set(value) - allowed if unknown: raise QAPISemError(
This is a minor adjustment that allows the 'required' and 'optional' keys fields to take a default value of an empty, immutable sequence (the empty tuple). This reveals a quirk of this function, which is that "a + b" is list-specific behavior. We can accept a wider variety of types if we avoid that behavior. Using Collection allows us to accept things like lists, tuples, sets, and so on. (Iterable would also have worked, but Iterable also includes things like generator expressions which are consumed upon iteration, which would require a rewrite to make sure that each input was only traversed once.) Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/expr.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)