John Snow <js...@redhat.com> writes:
> On 4/25/21 3:59 AM, Markus Armbruster wrote:
>> John Snow <js...@redhat.com> writes:
>>
>>> When the token can be None, we can't use 'x in "abc"' style membership
>>> tests to group types of tokens together, because 'None in "abc"' is a
>>> TypeError.
>>>
>>> Easy enough to fix, if not a little ugly.
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> ---
>>> scripts/qapi/parser.py | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>>> index 7f3c009f64b..16fd36f8391 100644
>>> --- a/scripts/qapi/parser.py
>>> +++ b/scripts/qapi/parser.py
>>> @@ -272,7 +272,7 @@ def get_values(self):
>>> if self.tok == ']':
>>> self.accept()
>>> return expr
>>> - if self.tok not in "{['tf":
>>> + if self.tok is None or self.tok not in "{['tf":
>>> raise QAPIParseError(
>>> self, "expected '{', '[', ']', string, or boolean")
>>> while True:
>>> @@ -294,7 +294,8 @@ def get_expr(self, nested):
>>> elif self.tok == '[':
>>> self.accept()
>>> expr = self.get_values()
>>> - elif self.tok in "'tf":
>>> + elif self.tok and self.tok in "'tf":
>>> + assert isinstance(self.val, (str, bool))
>>> expr = self.val
>>> self.accept()
>>> else:
>>
>> How can self.tok be None?
>>
>> I suspect this is an artifact of PATCH 04. Before, self.tok is
>> initialized to the first token, then set to subsequent tokens (all str)
>> in turn. After, it's initialized to None, then set to tokens in turn.
>>
>
> Actually, it's set to None to represent EOF. See here:
>
> elif self.tok == '\n':
> if self.cursor == len(self.src):
> self.tok = None
> return
Alright, then this is actually a bug fix:
$ echo -n "{'key': " | python3 scripts/qapi-gen.py /dev/stdin
Traceback (most recent call last):
File "scripts/qapi-gen.py", line 19, in <module>
sys.exit(main.main())
File "/work/armbru/qemu/scripts/qapi/main.py", line 93, in main
generate(args.schema,
File "/work/armbru/qemu/scripts/qapi/main.py", line 50, in generate
schema = QAPISchema(schema_file)
File "/work/armbru/qemu/scripts/qapi/schema.py", line 852, in __init__
parser = QAPISchemaParser(fname)
File "/work/armbru/qemu/scripts/qapi/parser.py", line 59, in __init__
self._parse()
File "/work/armbru/qemu/scripts/qapi/parser.py", line 81, in _parse
expr = self.get_expr(False)
File "/work/armbru/qemu/scripts/qapi/parser.py", line 293, in get_expr
expr = self.get_members()
File "/work/armbru/qemu/scripts/qapi/parser.py", line 260, in get_members
expr[key] = self.get_expr(True)
File "/work/armbru/qemu/scripts/qapi/parser.py", line 297, in get_expr
elif self.tok in "'tf":
TypeError: 'in <string>' requires string as left operand, not NoneType
Likewise, the other hunk:
$ echo -n "{'key': [" | python3 scripts/qapi-gen.py /dev/stdin
Traceback (most recent call last):
File "scripts/qapi-gen.py", line 19, in <module>
sys.exit(main.main())
File "/work/armbru/qemu/scripts/qapi/main.py", line 89, in main
generate(args.schema,
File "/work/armbru/qemu/scripts/qapi/main.py", line 51, in generate
schema = QAPISchema(schema_file)
File "/work/armbru/qemu/scripts/qapi/schema.py", line 860, in __init__
parser = QAPISchemaParser(fname)
File "/work/armbru/qemu/scripts/qapi/parser.py", line 71, in __init__
expr = self.get_expr(False)
File "/work/armbru/qemu/scripts/qapi/parser.py", line 270, in get_expr
expr = self.get_members()
File "/work/armbru/qemu/scripts/qapi/parser.py", line 238, in get_members
expr[key] = self.get_expr(True)
File "/work/armbru/qemu/scripts/qapi/parser.py", line 273, in get_expr
expr = self.get_values()
File "/work/armbru/qemu/scripts/qapi/parser.py", line 253, in get_values
if self.tok not in "{['tf":
TypeError: 'in <string>' requires string as left operand, not NoneType
Please add test cases. I recommend adding them in a separate patch, so
this one's diff shows clearly what's being fixed.
There's a similar one in accept(), but it's safe:
self.tok = self.src[self.cursor]
...
elif self.tok in '{}:,[]':
return
Regarding "if not a little ugly": instead of
self.tok is None or self.tok not in "{['tf"
we could use
self.tok not in tuple("{['tf")
> A more pythonic idiom would be to create a lexer class that behaves as
> an iterator, yielding Token class objects, and eventually, raising
> StopIteration.
>
> (Not suggesting I do that now. I have thought about it though, yes.)
Yes, let's resist the temptation to improve things into too many
directions at once.
Aside: using exceptions for perfectly unexceptional things like loop
termination is in questionable taste, but we gotta go with the Python
flow.
