Re: [PATCH 10/22] qapi/parser: Fix typing of token membership tests

[John Snow]

On 4/25/21 3:59 AM, Markus Armbruster wrote:
> John Snow <jsnow@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 <jsnow@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

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.)

--js