qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 06/17] qapi: rework qapi Exception


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PATCH v6 06/17] qapi: rework qapi Exception
Date: Thu, 5 Jan 2017 13:26:48 -0500 (EST)

Hi

----- Original Message -----
> Marc-André Lureau <address@hidden> writes:
> 
> > Use a base class QAPIError, and QAPIParseError for parser errors and
> > QAPISemError for semantic errors, suggested by Markus Armbruster.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  scripts/qapi.py | 338
> >  ++++++++++++++++++++++++++------------------------------
> >  1 file changed, 158 insertions(+), 180 deletions(-)
> >
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index 21bc32fda3..5885c9e4ad 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -91,35 +91,38 @@ def error_path(parent):
> >      return res
> >  
> >  
> > -class QAPISchemaError(Exception):
> > -    def __init__(self, schema, msg):
> > +class QAPIError(Exception):
> > +    def __init__(self, fname, line, col, incl_info, msg):
> >          Exception.__init__(self)
> > -        self.fname = schema.fname
> > +        self.fname = fname
> > +        self.line = line
> > +        self.col = col
> > +        self.info = incl_info
> >          self.msg = msg
> > -        self.col = 1
> > -        self.line = schema.line
> > -        for ch in schema.src[schema.line_pos:schema.pos]:
> > -            if ch == '\t':
> > -                self.col = (self.col + 7) % 8 + 1
> > -            else:
> > -                self.col += 1
> > -        self.info = schema.incl_info
> >  
> >      def __str__(self):
> > -        return error_path(self.info) + \
> > -            "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
> > +        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 QAPIExprError(Exception):
> > -    def __init__(self, expr_info, msg):
> > -        Exception.__init__(self)
> > -        assert expr_info
> > -        self.info = expr_info
> > -        self.msg = msg
> > +class QAPIParseError(QAPIError):
> > +    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
> > +        QAPIError.__init__(self, parser.fname, parser.line,
> > +                           col, parser.incl_info, msg)
> 
> Let's break the line after col, so that line and columns stay together.
> Could be done on commit.
> 

ok

> >  
> > -    def __str__(self):
> > -        return error_path(self.info['parent']) + \
> > -            "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg)
> > +
> > +class QAPISemError(QAPIError):
> > +    def __init__(self, info, msg):
> > +        QAPIError.__init__(self, info['file'], info['line'], None,
> > +                           info['parent'], msg)
> >  
> >  
> >  class QAPISchemaParser(object):
> > @@ -140,25 +143,24 @@ class QAPISchemaParser(object):
> >          self.accept()
> >  
> >          while self.tok is not None:
> > -            expr_info = {'file': fname, 'line': self.line,
> > -                         'parent': self.incl_info}
> > +            info = {'file': fname, 'line': self.line,
> > +                    'parent': self.incl_info}
> >              expr = self.get_expr(False)
> >              if isinstance(expr, dict) and "include" in expr:
> >                  if len(expr) != 1:
> > -                    raise QAPIExprError(expr_info,
> > -                                        "Invalid 'include' directive")
> > +                    raise QAPISemError(info, "Invalid 'include'
> > directive")
> >                  include = expr["include"]
> >                  if not isinstance(include, str):
> > -                    raise QAPIExprError(expr_info,
> > -                                        "Value of 'include' must be a
> > string")
> > +                    raise QAPISemError(info,
> > +                                       "Value of 'include' must be a
> > string")
> >                  incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
> >                                                include)
> >                  # catch inclusion cycle
> > -                inf = expr_info
> > +                inf = info
> >                  while inf:
> >                      if incl_abs_fname == os.path.abspath(inf['file']):
> > -                        raise QAPIExprError(expr_info, "Inclusion loop for
> > %s"
> > -                                            % include)
> > +                        raise QAPISemError(info, "Inclusion loop for %s"
> > +                                           % include)
> >                      inf = inf['parent']
> >                  # skip multiple include of the same file
> >                  if incl_abs_fname in previously_included:
> > @@ -166,14 +168,13 @@ class QAPISchemaParser(object):
> >                  try:
> >                      fobj = open(incl_abs_fname, 'r')
> >                  except IOError as e:
> > -                    raise QAPIExprError(expr_info,
> > -                                        '%s: %s' % (e.strerror, include))
> > +                    raise QAPISemError(info, '%s: %s' % (e.strerror,
> > include))
> >                  exprs_include = QAPISchemaParser(fobj,
> >                  previously_included,
> > -                                                 expr_info)
> > +                                                 info)
> >                  self.exprs.extend(exprs_include.exprs)
> >              else:
> >                  expr_elem = {'expr': expr,
> > -                             'info': expr_info}
> > +                             'info': info}
> >                  self.exprs.append(expr_elem)
> >  
> >      def accept(self):
> > @@ -194,8 +195,7 @@ class QAPISchemaParser(object):
> >                      ch = self.src[self.cursor]
> >                      self.cursor += 1
> >                      if ch == '\n':
> > -                        raise QAPISchemaError(self,
> > -                                              'Missing terminating "\'"')
> > +                        raise QAPIParseError(self, 'Missing terminating
> > "\'"')
> >                      if esc:
> >                          if ch == 'b':
> >                              string += '\b'
> > @@ -213,25 +213,25 @@ class QAPISchemaParser(object):
> >                                  ch = self.src[self.cursor]
> >                                  self.cursor += 1
> >                                  if ch not in "0123456789abcdefABCDEF":
> > -                                    raise QAPISchemaError(self,
> > -                                                          '\\u escape
> > needs 4 '
> > -                                                          'hex digits')
> > +                                    raise QAPIParseError(self,
> > +                                                         '\\u escape needs
> > 4 '
> > +                                                         'hex digits')
> >                                  value = (value << 4) + int(ch, 16)
> >                              # If Python 2 and 3 didn't disagree so much on
> >                              # how to handle Unicode, then we could allow
> >                              # Unicode string defaults.  But most of QAPI
> >                              is
> >                              # ASCII-only, so we aren't losing much for
> >                              now.
> >                              if not value or value > 0x7f:
> > -                                raise QAPISchemaError(self,
> > -                                                      'For now, \\u escape
> > '
> > -                                                      'only supports
> > non-zero '
> > -                                                      'values up to
> > \\u007f')
> > +                                raise QAPIParseError(self,
> > +                                                     'For now, \\u escape
> > '
> > +                                                     'only supports
> > non-zero '
> > +                                                     'values up to
> > \\u007f')
> >                              string += chr(value)
> >                          elif ch in "\\/'\"":
> >                              string += ch
> >                          else:
> > -                            raise QAPISchemaError(self,
> > -                                                  "Unknown escape \\%s" %
> > ch)
> > +                            raise QAPIParseError(self,
> > +                                                 "Unknown escape \\%s" %
> > ch)
> >                          esc = False
> >                      elif ch == "\\":
> >                          esc = True
> > @@ -259,7 +259,7 @@ class QAPISchemaParser(object):
> >                  self.line += 1
> >                  self.line_pos = self.cursor
> >              elif not self.tok.isspace():
> > -                raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
> > +                raise QAPIParseError(self, 'Stray "%s"' % self.tok)
> >  
> >      def get_members(self):
> >          expr = OrderedDict()
> > @@ -267,24 +267,24 @@ class QAPISchemaParser(object):
> >              self.accept()
> >              return expr
> >          if self.tok != "'":
> > -            raise QAPISchemaError(self, 'Expected string or "}"')
> > +            raise QAPIParseError(self, 'Expected string or "}"')
> >          while True:
> >              key = self.val
> >              self.accept()
> >              if self.tok != ':':
> > -                raise QAPISchemaError(self, 'Expected ":"')
> > +                raise QAPIParseError(self, 'Expected ":"')
> >              self.accept()
> >              if key in expr:
> > -                raise QAPISchemaError(self, 'Duplicate key "%s"' % key)
> > +                raise QAPIParseError(self, 'Duplicate key "%s"' % key)
> >              expr[key] = self.get_expr(True)
> >              if self.tok == '}':
> >                  self.accept()
> >                  return expr
> >              if self.tok != ',':
> > -                raise QAPISchemaError(self, 'Expected "," or "}"')
> > +                raise QAPIParseError(self, 'Expected "," or "}"')
> >              self.accept()
> >              if self.tok != "'":
> > -                raise QAPISchemaError(self, 'Expected string')
> > +                raise QAPIParseError(self, 'Expected string')
> >  
> >      def get_values(self):
> >          expr = []
> > @@ -292,20 +292,20 @@ class QAPISchemaParser(object):
> >              self.accept()
> >              return expr
> >          if self.tok not in "{['tfn":
> > -            raise QAPISchemaError(self, 'Expected "{", "[", "]", string, '
> > -                                  'boolean or "null"')
> > +            raise QAPIParseError(self, 'Expected "{", "[", "]", string, '
> > +                                 'boolean or "null"')
> >          while True:
> >              expr.append(self.get_expr(True))
> >              if self.tok == ']':
> >                  self.accept()
> >                  return expr
> >              if self.tok != ',':
> > -                raise QAPISchemaError(self, 'Expected "," or "]"')
> > +                raise QAPIParseError(self, 'Expected "," or "]"')
> >              self.accept()
> >  
> >      def get_expr(self, nested):
> >          if self.tok != '{' and not nested:
> > -            raise QAPISchemaError(self, 'Expected "{"')
> > +            raise QAPIParseError(self, 'Expected "{"')
> >          if self.tok == '{':
> >              self.accept()
> >              expr = self.get_members()
> > @@ -316,7 +316,7 @@ class QAPISchemaParser(object):
> >              expr = self.val
> >              self.accept()
> >          else:
> > -            raise QAPISchemaError(self, 'Expected "{", "[" or string')
> > +            raise QAPIParseError(self, 'Expected "{", "[" or string')
> >          return expr
> >  
> >  #
> > @@ -375,20 +375,18 @@ valid_name = re.compile('^(__[a-zA-Z0-9.-]+_)?'
> >                          '[a-zA-Z][a-zA-Z0-9_-]*$')
> >  
> >  
> > -def check_name(expr_info, source, name, allow_optional=False,
> > +def check_name(info, source, name, allow_optional=False,
> >                 enum_member=False):
> >      global valid_name
> >      membername = name
> >  
> >      if not isinstance(name, str):
> > -        raise QAPIExprError(expr_info,
> > -                            "%s requires a string name" % source)
> > +        raise QAPISemError(info, "%s requires a string name" % source)
> >      if name.startswith('*'):
> >          membername = name[1:]
> >          if not allow_optional:
> > -            raise QAPIExprError(expr_info,
> > -                                "%s does not allow optional name '%s'"
> > -                                % (source, name))
> > +            raise QAPISemError(info, "%s does not allow optional name
> > '%s'"
> > +                               % (source, name))
> >      # Enum members can start with a digit, because the generated C
> >      # code always prefixes it with the enum name
> >      if enum_member and membername[0].isdigit():
> > @@ -397,8 +395,7 @@ def check_name(expr_info, source, name,
> > allow_optional=False,
> >      # and 'q_obj_*' implicit type names.
> >      if not valid_name.match(membername) or \
> >         c_name(membername, False).startswith('q_'):
> > -        raise QAPIExprError(expr_info,
> > -                            "%s uses invalid name '%s'" % (source, name))
> > +        raise QAPISemError(info, "%s uses invalid name '%s'" % (source,
> > name))
> >  
> >  
> >  def add_name(name, info, meta, implicit=False):
> > @@ -407,13 +404,11 @@ def add_name(name, info, meta, implicit=False):
> >      # FIXME should reject names that differ only in '_' vs. '.'
> >      # vs. '-', because they're liable to clash in generated C.
> >      if name in all_names:
> > -        raise QAPIExprError(info,
> > -                            "%s '%s' is already defined"
> > -                            % (all_names[name], name))
> > +        raise QAPISemError(info, "%s '%s' is already defined"
> > +                           % (all_names[name], name))
> >      if not implicit and (name.endswith('Kind') or name.endswith('List')):
> > -        raise QAPIExprError(info,
> > -                            "%s '%s' should not end in '%s'"
> > -                            % (meta, name, name[-4:]))
> > +        raise QAPISemError(info, "%s '%s' should not end in '%s'"
> > +                           % (meta, name, name[-4:]))
> >      all_names[name] = meta
> >  
> >  
> > @@ -465,7 +460,7 @@ def is_enum(name):
> >      return find_enum(name) is not None
> >  
> >  
> > -def check_type(expr_info, source, value, allow_array=False,
> > +def check_type(info, source, value, allow_array=False,
> >                 allow_dict=False, allow_optional=False,
> >                 allow_metas=[]):
> >      global all_names
> > @@ -476,69 +471,64 @@ def check_type(expr_info, source, value,
> > allow_array=False,
> >      # Check if array type for value is okay
> >      if isinstance(value, list):
> >          if not allow_array:
> > -            raise QAPIExprError(expr_info,
> > -                                "%s cannot be an array" % source)
> > +            raise QAPISemError(info, "%s cannot be an array" % source)
> >          if len(value) != 1 or not isinstance(value[0], str):
> > -            raise QAPIExprError(expr_info,
> > -                                "%s: array type must contain single type
> > name"
> > -                                % source)
> > +            raise QAPISemError(info,
> > +                               "%s: array type must contain single type
> > name" %
> > +                               source)
> >          value = value[0]
> >  
> >      # Check if type name for value is okay
> >      if isinstance(value, str):
> >          if value not in all_names:
> > -            raise QAPIExprError(expr_info,
> > -                                "%s uses unknown type '%s'"
> > -                                % (source, value))
> > +            raise QAPISemError(info, "%s uses unknown type '%s'"
> > +                               % (source, value))
> >          if not all_names[value] in allow_metas:
> > -            raise QAPIExprError(expr_info,
> > -                                "%s cannot use %s type '%s'"
> > -                                % (source, all_names[value], value))
> > +            raise QAPISemError(info, "%s cannot use %s type '%s'" %
> > +                               (source, all_names[value], value))
> >          return
> >  
> >      if not allow_dict:
> > -        raise QAPIExprError(expr_info,
> > -                            "%s should be a type name" % source)
> > +        raise QAPISemError(info, "%s should be a type name" % source)
> >  
> >      if not isinstance(value, OrderedDict):
> > -        raise QAPIExprError(expr_info,
> > -                            "%s should be a dictionary or type name" %
> > source)
> > +        raise QAPISemError(info,
> > +                           "%s should be a dictionary or type name" %
> > source)
> >  
> >      # value is a dictionary, check that each member is okay
> >      for (key, arg) in value.items():
> > -        check_name(expr_info, "Member of %s" % source, key,
> > +        check_name(info, "Member of %s" % source, key,
> >                     allow_optional=allow_optional)
> >          if c_name(key, False) == 'u' or c_name(key,
> >          False).startswith('has_'):
> > -            raise QAPIExprError(expr_info,
> > -                                "Member of %s uses reserved name '%s'"
> > -                                % (source, key))
> > +            raise QAPISemError(info, "Member of %s uses reserved name
> > '%s'"
> > +                               % (source, key))
> >          # Todo: allow dictionaries to represent default values of
> >          # an optional argument.
> > -        check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
> > +        check_type(info, "Member '%s' of %s" % (key, source), arg,
> >                     allow_array=True,
> >                     allow_metas=['built-in', 'union', 'alternate',
> >                     'struct',
> >                                  'enum'])
> >  
> >  
> > -def check_command(expr, expr_info):
> > +def check_command(expr, info):
> >      name = expr['command']
> >      boxed = expr.get('boxed', False)
> >  
> >      args_meta = ['struct']
> >      if boxed:
> >          args_meta += ['union', 'alternate']
> > -    check_type(expr_info, "'data' for command '%s'" % name,
> > +    check_type(info, "'data' for command '%s'" % name,
> >                 expr.get('data'), allow_dict=not boxed,
> >                 allow_optional=True,
> >                 allow_metas=args_meta)
> >      returns_meta = ['union', 'struct']
> >      if name in returns_whitelist:
> >          returns_meta += ['built-in', 'alternate', 'enum']
> > -    check_type(expr_info, "'returns' for command '%s'" % name,
> > +    check_type(info, "'returns' for command '%s'" % name,
> >                 expr.get('returns'), allow_array=True,
> >                 allow_optional=True, allow_metas=returns_meta)
> >  
> >  
> > -def check_event(expr, expr_info):
> > +def check_event(expr, info):
> >      global events
> >      name = expr['event']
> >      boxed = expr.get('boxed', False)
> > @@ -547,12 +537,12 @@ def check_event(expr, expr_info):
> >      if boxed:
> >          meta += ['union', 'alternate']
> >      events.append(name)
> > -    check_type(expr_info, "'data' for event '%s'" % name,
> > +    check_type(info, "'data' for event '%s'" % name,
> >                 expr.get('data'), allow_dict=not boxed,
> >                 allow_optional=True,
> >                 allow_metas=meta)
> >  
> >  
> > -def check_union(expr, expr_info):
> > +def check_union(expr, info):
> >      name = expr['union']
> >      base = expr.get('base')
> >      discriminator = expr.get('discriminator')
> > @@ -565,123 +555,117 @@ def check_union(expr, expr_info):
> >          enum_define = None
> >          allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
> >          if base is not None:
> > -            raise QAPIExprError(expr_info,
> > -                                "Simple union '%s' must not have a base"
> > -                                % name)
> > +            raise QAPISemError(info, "Simple union '%s' must not have a
> > base" %
> > +                               name)
> >  
> >      # Else, it's a flat union.
> >      else:
> >          # The object must have a string or dictionary 'base'.
> > -        check_type(expr_info, "'base' for union '%s'" % name,
> > +        check_type(info, "'base' for union '%s'" % name,
> >                     base, allow_dict=True, allow_optional=True,
> >                     allow_metas=['struct'])
> >          if not base:
> > -            raise QAPIExprError(expr_info,
> > -                                "Flat union '%s' must have a base"
> > -                                % name)
> > +            raise QAPISemError(info, "Flat union '%s' must have a base"
> > +                               % name)
> >          base_members = find_base_members(base)
> >          assert base_members
> >  
> >          # The value of member 'discriminator' must name a non-optional
> >          # member of the base struct.
> > -        check_name(expr_info, "Discriminator of flat union '%s'" % name,
> > +        check_name(info, "Discriminator of flat union '%s'" % name,
> >                     discriminator)
> >          discriminator_type = base_members.get(discriminator)
> >          if not discriminator_type:
> > -            raise QAPIExprError(expr_info,
> > -                                "Discriminator '%s' is not a member of
> > base "
> > -                                "struct '%s'"
> > -                                % (discriminator, base))
> > +            raise QAPISemError(info,
> > +                               "Discriminator '%s' is not a member of base
> > "
> > +                               "struct '%s'"
> > +                               % (discriminator, base))
> >          enum_define = find_enum(discriminator_type)
> >          allow_metas = ['struct']
> >          # Do not allow string discriminator
> >          if not enum_define:
> > -            raise QAPIExprError(expr_info,
> > -                                "Discriminator '%s' must be of enumeration
> > "
> > -                                "type" % discriminator)
> > +            raise QAPISemError(info,
> > +                               "Discriminator '%s' must be of enumeration
> > "
> > +                               "type" % discriminator)
> >  
> >      # Check every branch; don't allow an empty union
> >      if len(members) == 0:
> > -        raise QAPIExprError(expr_info,
> > -                            "Union '%s' cannot have empty 'data'" % name)
> > +        raise QAPISemError(info, "Union '%s' cannot have empty 'data'" %
> > name)
> >      for (key, value) in members.items():
> > -        check_name(expr_info, "Member of union '%s'" % name, key)
> > +        check_name(info, "Member of union '%s'" % name, key)
> >  
> >          # Each value must name a known type
> > -        check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
> > +        check_type(info, "Member '%s' of union '%s'" % (key, name),
> >                     value, allow_array=not base, allow_metas=allow_metas)
> >  
> >          # If the discriminator names an enum type, then all members
> >          # of 'data' must also be members of the enum type.
> >          if enum_define:
> >              if key not in enum_define['enum_values']:
> > -                raise QAPIExprError(expr_info,
> > -                                    "Discriminator value '%s' is not found
> > in "
> > -                                    "enum '%s'" %
> > -                                    (key, enum_define["enum_name"]))
> > +                raise QAPISemError(info,
> > +                                   "Discriminator value '%s' is not found
> > in "
> > +                                   "enum '%s'"
> > +                                   % (key, enum_define["enum_name"]))
> >  
> >      # If discriminator is user-defined, ensure all values are covered
> >      if enum_define:
> >          for value in enum_define['enum_values']:
> >              if value not in members.keys():
> > -                raise QAPIExprError(expr_info,
> > -                                    "Union '%s' data missing '%s' branch"
> > -                                    % (name, value))
> > +                raise QAPISemError(info, "Union '%s' data missing '%s'
> > branch"
> > +                                   % (name, value))
> >  
> >  
> > -def check_alternate(expr, expr_info):
> > +def check_alternate(expr, info):
> >      name = expr['alternate']
> >      members = expr['data']
> >      types_seen = {}
> >  
> >      # Check every branch; require at least two branches
> >      if len(members) < 2:
> > -        raise QAPIExprError(expr_info,
> > -                            "Alternate '%s' should have at least two
> > branches "
> > -                            "in 'data'" % name)
> > +        raise QAPISemError(info,
> > +                           "Alternate '%s' should have at least two
> > branches "
> > +                           "in 'data'" % name)
> >      for (key, value) in members.items():
> > -        check_name(expr_info, "Member of alternate '%s'" % name, key)
> > +        check_name(info, "Member of alternate '%s'" % name, key)
> >  
> >          # Ensure alternates have no type conflicts.
> > -        check_type(expr_info, "Member '%s' of alternate '%s'" % (key,
> > name),
> > +        check_type(info, "Member '%s' of alternate '%s'" % (key, name),
> >                     value,
> >                     allow_metas=['built-in', 'union', 'struct', 'enum'])
> >          qtype = find_alternate_member_qtype(value)
> >          if not qtype:
> > -            raise QAPIExprError(expr_info,
> > -                                "Alternate '%s' member '%s' cannot use "
> > -                                "type '%s'" % (name, key, value))
> > +            raise QAPISemError(info, "Alternate '%s' member '%s' cannot
> > use "
> > +                               "type '%s'" % (name, key, value))
> >          if qtype in types_seen:
> > -            raise QAPIExprError(expr_info,
> > -                                "Alternate '%s' member '%s' can't "
> > -                                "be distinguished from member '%s'"
> > -                                % (name, key, types_seen[qtype]))
> > +            raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> > +                               "be distinguished from member '%s'"
> > +                               % (name, key, types_seen[qtype]))
> >          types_seen[qtype] = key
> >  
> >  
> > -def check_enum(expr, expr_info):
> > +def check_enum(expr, info):
> >      name = expr['enum']
> >      members = expr.get('data')
> >      prefix = expr.get('prefix')
> >  
> >      if not isinstance(members, list):
> > -        raise QAPIExprError(expr_info,
> > -                            "Enum '%s' requires an array for 'data'" %
> > name)
> > +        raise QAPISemError(info,
> > +                           "Enum '%s' requires an array for 'data'" %
> > name)
> >      if prefix is not None and not isinstance(prefix, str):
> > -        raise QAPIExprError(expr_info,
> > -                            "Enum '%s' requires a string for 'prefix'" %
> > name)
> > +        raise QAPISemError(info,
> > +                           "Enum '%s' requires a string for 'prefix'" %
> > name)
> >      for member in members:
> > -        check_name(expr_info, "Member of enum '%s'" % name, member,
> > +        check_name(info, "Member of enum '%s'" % name, member,
> >                     enum_member=True)
> >  
> >  
> > -def check_struct(expr, expr_info):
> > +def check_struct(expr, info):
> >      name = expr['struct']
> >      members = expr['data']
> >  
> > -    check_type(expr_info, "'data' for struct '%s'" % name, members,
> > +    check_type(info, "'data' for struct '%s'" % name, members,
> >                 allow_dict=True, allow_optional=True)
> > -    check_type(expr_info, "'base' for struct '%s'" % name,
> > expr.get('base'),
> > +    check_type(info, "'base' for struct '%s'" % name, expr.get('base'),
> >                 allow_metas=['struct'])
> >  
> >  
> > @@ -690,27 +674,24 @@ def check_keys(expr_elem, meta, required,
> > optional=[]):
> >      info = expr_elem['info']
> >      name = expr[meta]
> >      if not isinstance(name, str):
> > -        raise QAPIExprError(info,
> > -                            "'%s' key must have a string value" % meta)
> > +        raise QAPISemError(info, "'%s' key must have a string value" %
> > meta)
> >      required = required + [meta]
> >      for (key, value) in expr.items():
> >          if key not in required and key not in optional:
> > -            raise QAPIExprError(info,
> > -                                "Unknown key '%s' in %s '%s'"
> > -                                % (key, meta, name))
> > +            raise QAPISemError(info, "Unknown key '%s' in %s '%s'"
> > +                               % (key, meta, name))
> >          if (key == 'gen' or key == 'success-response') and value is not
> >          False:
> > -            raise QAPIExprError(info,
> > -                                "'%s' of %s '%s' should only use false
> > value"
> > -                                % (key, meta, name))
> > +            raise QAPISemError(info,
> > +                               "'%s' of %s '%s' should only use false
> > value"
> > +                               % (key, meta, name))
> >          if key == 'boxed' and value is not True:
> > -            raise QAPIExprError(info,
> > -                                "'%s' of %s '%s' should only use true
> > value"
> > -                                % (key, meta, name))
> > +            raise QAPISemError(info,
> > +                               "'%s' of %s '%s' should only use true
> > value"
> > +                               % (key, meta, name))
> >      for key in required:
> >          if key not in expr:
> > -            raise QAPIExprError(info,
> > -                                "Key '%s' is missing from %s '%s'"
> > -                                % (key, meta, name))
> > +            raise QAPISemError(info, "Key '%s' is missing from %s '%s'"
> > +                               % (key, meta, name))
> >  
> >  
> >  def check_exprs(exprs):
> > @@ -743,8 +724,8 @@ def check_exprs(exprs):
> >              check_keys(expr_elem, 'event', [], ['data', 'boxed'])
> >              add_name(expr['event'], info, 'event')
> >          else:
> > -            raise QAPIExprError(expr_elem['info'],
> > -                                "Expression is missing metatype")
> > +            raise QAPISemError(expr_elem['info'],
> > +                               "Expression is missing metatype")
> >  
> >      # Try again for hidden UnionKind enum
> >      for expr_elem in exprs:
> > @@ -978,8 +959,8 @@ class QAPISchemaObjectType(QAPISchemaType):
> >  
> >      def check(self, schema):
> >          if self.members is False:               # check for cycles
> > -            raise QAPIExprError(self.info,
> > -                                "Object %s contains itself" % self.name)
> > +            raise QAPISemError(self.info,
> > +                               "Object %s contains itself" % self.name)
> >          if self.members:
> >              return
> >          self.members = False                    # mark as being checked
> > @@ -1051,12 +1032,11 @@ class QAPISchemaMember(object):
> >      def check_clash(self, info, seen):
> >          cname = c_name(self.name)
> >          if cname.lower() != cname and self.owner not in case_whitelist:
> > -            raise QAPIExprError(info,
> > -                                "%s should not use uppercase" %
> > self.describe())
> > +            raise QAPISemError(info,
> > +                               "%s should not use uppercase" %
> > self.describe())
> >          if cname in seen:
> > -            raise QAPIExprError(info,
> > -                                "%s collides with %s"
> > -                                % (self.describe(),
> > seen[cname].describe()))
> > +            raise QAPISemError(info, "%s collides with %s" %
> > +                               (self.describe(), seen[cname].describe()))
> >          seen[cname] = self
> >  
> >      def _pretty_owner(self):
> > @@ -1201,14 +1181,13 @@ class QAPISchemaCommand(QAPISchemaEntity):
> >              self.arg_type.check(schema)
> >              if self.boxed:
> >                  if self.arg_type.is_empty():
> > -                    raise QAPIExprError(self.info,
> > -                                        "Cannot use 'boxed' with empty
> > type")
> > +                    raise QAPISemError(self.info,
> > +                                       "Cannot use 'boxed' with empty
> > type")
> >              else:
> >                  assert not isinstance(self.arg_type,
> >                  QAPISchemaAlternateType)
> >                  assert not self.arg_type.variants
> >          elif self.boxed:
> > -            raise QAPIExprError(self.info,
> > -                                "Use of 'boxed' requires 'data'")
> > +            raise QAPISemError(self.info, "Use of 'boxed' requires
> > 'data'")
> >          if self._ret_type_name:
> >              self.ret_type = schema.lookup_type(self._ret_type_name)
> >              assert isinstance(self.ret_type, QAPISchemaType)
> > @@ -1235,14 +1214,13 @@ class QAPISchemaEvent(QAPISchemaEntity):
> >              self.arg_type.check(schema)
> >              if self.boxed:
> >                  if self.arg_type.is_empty():
> > -                    raise QAPIExprError(self.info,
> > -                                        "Cannot use 'boxed' with empty
> > type")
> > +                    raise QAPISemError(self.info,
> > +                                       "Cannot use 'boxed' with empty
> > type")
> >              else:
> >                  assert not isinstance(self.arg_type,
> >                  QAPISchemaAlternateType)
> >                  assert not self.arg_type.variants
> >          elif self.boxed:
> > -            raise QAPIExprError(self.info,
> > -                                "Use of 'boxed' requires 'data'")
> > +            raise QAPISemError(self.info, "Use of 'boxed' requires
> > 'data'")
> >  
> >      def visit(self, visitor):
> >          visitor.visit_event(self.name, self.info, self.arg_type,
> >          self.boxed)
> > @@ -1258,7 +1236,7 @@ class QAPISchema(object):
> >              self._predefining = False
> >              self._def_exprs()
> >              self.check()
> > -        except (QAPISchemaError, QAPIExprError) as err:
> > +        except QAPIError as err:
> >              print >>sys.stderr, err
> >              exit(1)
> >  
> > @@ -1557,8 +1535,8 @@ def c_name(name, protect=True):
> >      # namespace pollution:
> >      polluted_words = set(['unix', 'errno', 'mips', 'sparc'])
> >      name = name.translate(c_name_trans)
> > -    if protect and (name in c89_words | c99_words | c11_words | gcc_words
> > -                    | cpp_words | polluted_words):
> > +    if protect and (name in c89_words | c99_words | c11_words | gcc_words
> > |
> > +                    cpp_words | polluted_words):
> >          return "q_" + name
> >      return name
> 
> Spurious hunk.  Could drop on commit.

I think I did it to fix
scripts/qapi.py:1539:21: W503 line break before binary operator

I'll split it

> 
> The change is mostly mechanical.  Would be easier to review if the
> rename from expr_info to info was a separate patch, but I guess it's not
> worth splitting off now.  I'm not sure the rename is an improvement, but
> backing it out seems not worth it now, either.
> 
> With the two touch-ups:
> Reviewed-by: Markus Armbruster <address@hidden>
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]