Re: [Qemu-devel] [PATCH v5 12/17] qapi: rename QAPIExprError/QAPILineError

[Marc-André Lureau]

----- Original Message -----
> Make the summary line "qapi: Rename QAPIExprError to QAPILineError".
> 

ok

> Marc-André Lureau <address@hidden> writes:
> 
> > There is nothing specific about expressions in this exception,
> > the following patch will use it without expressions.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  scripts/qapi.py | 146
> >  ++++++++++++++++++++++++++++----------------------------
> >  1 file changed, 73 insertions(+), 73 deletions(-)
> >
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index 21bc32f..4d1b0e4 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -110,11 +110,11 @@ class QAPISchemaError(Exception):
> >              "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
> >  
> >  
> > -class QAPIExprError(Exception):
> > -    def __init__(self, expr_info, msg):
> > +class QAPILineError(Exception):
> > +    def __init__(self, info, msg):
> >          Exception.__init__(self)
> > -        assert expr_info
> > -        self.info = expr_info
> > +        assert info
> > +        self.info = info
> >          self.msg = msg
> >  
> >      def __str__(self):
> 
> Since we're talking about misnamed / awkward error stuff:
> 
> * QAPISchemaError is really a parse error.  __init__()'s schema argument
>   isn't a QAPISchema, it's a QAPISchemaParser.
> 
> * Method __str__() is mostly duplicated.
> 
> How do you like the following untested sketch?

I like it, though I would consider it separately from this series. Here I 
systematically renamed the existing class ("line" since it's about location in 
file), your proposed change has more implications I don't really want to get 
into here.

> 
>     class QAPISchemaError(Exception):
>         def __init__(self, fname, line, col, incl_info, msg):
>             Exception.__init__(self)
>             self.fname = fname
>             self.line = line
>             self.col = col
>             self.info = incl_info
>             self.msg = msg
> 
>         def __str__(self):
>             loc = "%s:%d" % (self.fname, self.line)
>             if self.col is not None:
>                 loc += ":%s" % self.col
>             return error_path(self.info) + \
>                 "%s: %s" % (loc, self.msg)
> 
> 
>     class QAPISchemaParseError(QAPISchemaError):
>         def __init__(self, parser, msg):
>             col = 1
>             for ch in parser.src[parser.line_pos:parser.pos]:
>                 if ch == '\t':
>                     col = (col + 7) % 8 + 1
>                 else:
>                     col += 1
>             QAPISchemaError.__init__(self, parser.fname, parser.line,
>             parser.col,
>                                      parser.incl_info, msg)
> 
> 
>     class QAPISchemaSemanticError(Exception):
>         def __init__(self, info, msg):
>             QAPISchemaError(self, info['file'], info['line'], None,
>                             info['parent'], msg)
> 
> The class names are a bit long.  If they result in awkward line breaks,
> let's shorten them some.
> 
> The "except (QAPISchemaError, QAPIExprError)" become just "except
> QAPISchemaError".
> 
> Could QAPISchemaParser.__init__() raise QAPISchemaParseError instead?
> 
> [...]
>