| Message ID | 20190624173935.25747-2-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block: Try to create well-typed json:{} filenames | expand |
Max Reitz <mreitz@redhat.com> writes: > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qapi-schema/bad-type-int.json | 1 - > tests/qapi-schema/enum-int-member.json | 1 - > scripts/qapi/common.py | 25 ++++++++++++++++++++---- > scripts/qapi/introspect.py | 2 ++ > tests/qapi-schema/bad-type-int.err | 2 +- > tests/qapi-schema/enum-int-member.err | 2 +- > tests/qapi-schema/leading-comma-list.err | 2 +- > 7 files changed, 26 insertions(+), 9 deletions(-) > > diff --git a/tests/qapi-schema/bad-type-int.json b/tests/qapi-schema/bad-type-int.json > index 56fc6f8126..81355eb196 100644 > --- a/tests/qapi-schema/bad-type-int.json > +++ b/tests/qapi-schema/bad-type-int.json > @@ -1,3 +1,2 @@ > # we reject an expression with a metatype that is not a string > -# FIXME: once the parser understands integer inputs, improve the error message > { 'struct': 1, 'data': { } } > diff --git a/tests/qapi-schema/enum-int-member.json b/tests/qapi-schema/enum-int-member.json > index 6c9c32e149..6958440c6d 100644 > --- a/tests/qapi-schema/enum-int-member.json > +++ b/tests/qapi-schema/enum-int-member.json > @@ -1,3 +1,2 @@ > # we reject any enum member that is not a string > -# FIXME: once the parser understands integer inputs, improve the error message > { 'enum': 'MyEnum', 'data': [ 1 ] } > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index d61bfdc526..3396ea4a09 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py > @@ -498,6 +498,8 @@ class QAPISchemaParser(object): > raise QAPISemError(info, "Unknown pragma '%s'" % name) > > def accept(self, skip_comment=True): > + num_match = re.compile(r'([-+]?inf|nan|[-+0-9.][0-9a-f.ex]*)') > + > while True: > self.tok = self.src[self.cursor] > self.pos = self.cursor This is yet another extension over plain JSON. RFC 8259: number = [ minus ] int [ frac ] [ exp ] decimal-point = %x2E ; . digit1-9 = %x31-39 ; 1-9 e = %x65 / %x45 ; e E exp = e [ minus / plus ] 1*DIGIT frac = decimal-point 1*DIGIT int = zero / ( digit1-9 *DIGIT ) minus = %x2D ; - plus = %x2B ; + zero = %x30 ; 0 Extensions are acceptable when we have an actual use for it, and we document them properly. Isn't the parenthesis in your regular expression redundant? What use do you have in mind for 'inf' and 'nan'? Why accept leading '+' as in '+123'? Why accept empty integral part as in '.123'? Why accept '.xe.'? Kidding you, that must be a bug in your regexp. Please decide what number syntax you'd like to accept, then specify it in docs/devel/qapi-code-gen.txt, so we can first discuss the specification, and then check the regexp implements it. docs/devel/qapi-code-gen.txt update goes here: === Schema syntax === Syntax is loosely based on JSON (http://www.ietf.org/rfc/rfc8259.txt). Differences: * Comments: start with a hash character (#) that is not part of a string, and extend to the end of the line. * Strings are enclosed in 'single quotes', not "double quotes". * Strings are restricted to printable ASCII, and escape sequences to just '\\'. --> * Numbers and null are not supported. Hrmm, commit 9d55380b5a "qapi: Remove null from schema language" left two instances in error messages behind. I'll fix them. > @@ -584,7 +586,22 @@ class QAPISchemaParser(object): > return > self.line += 1 > self.line_pos = self.cursor > - elif not self.tok.isspace(): > + elif self.tok.isspace(): > + pass > + elif num_match.match(self.src[self.pos:]): > + match = num_match.match(self.src[self.pos:]).group(0) Sadly, the walrus operator is Python 3.8. > + try: > + self.val = int(match, 0) > + except ValueError: > + try: > + self.val = float(match) > + except ValueError: > + raise QAPIParseError(self, > + '"%s" is not a valid integer or float' % match) > + > + self.cursor += len(match) - 1 > + return > + else: > raise QAPIParseError(self, 'Stray "%s"' % self.tok) Any particular reason for putting the number case last? > > def get_members(self): > @@ -617,9 +634,9 @@ class QAPISchemaParser(object): > if self.tok == ']': > self.accept() > return expr > - if self.tok not in "{['tfn": > + if self.tok not in "{['tfn-+0123456789.i": This is getting a bit ugly. Let's not worry about it now. > raise QAPIParseError(self, 'Expected "{", "[", "]", string, ' > - 'boolean or "null"') > + 'boolean, number or "null"') > while True: > expr.append(self.get_expr(True)) > if self.tok == ']': > @@ -638,7 +655,7 @@ class QAPISchemaParser(object): > elif self.tok == '[': > self.accept() > expr = self.get_values() > - elif self.tok in "'tfn": > + elif self.tok in "'tfn-+0123456789.i": > expr = self.val > self.accept() > else: > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index f62cf0a2e1..6a61dd831f 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -57,6 +57,8 @@ def to_qlit(obj, level=0, suppress_first_indent=False): > ret += indent(level) + '}))' > elif isinstance(obj, bool): > ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false') > + elif isinstance(obj, int) and obj >= -(2 ** 63) and obj < 2 ** 63: > + ret += 'QLIT_QNUM(%i)' % obj Please explain the range check. > else: > assert False # not implemented > if level > 0: > diff --git a/tests/qapi-schema/bad-type-int.err b/tests/qapi-schema/bad-type-int.err > index da89895404..e22fb4f655 100644 > --- a/tests/qapi-schema/bad-type-int.err > +++ b/tests/qapi-schema/bad-type-int.err > @@ -1 +1 @@ > -tests/qapi-schema/bad-type-int.json:3:13: Stray "1" > +tests/qapi-schema/bad-type-int.json:2: 'struct' key must have a string value Test needs a rename, assuming it's not redundant now. > diff --git a/tests/qapi-schema/enum-int-member.err b/tests/qapi-schema/enum-int-member.err > index 071c5213d8..112175f79d 100644 > --- a/tests/qapi-schema/enum-int-member.err > +++ b/tests/qapi-schema/enum-int-member.err > @@ -1 +1 @@ > -tests/qapi-schema/enum-int-member.json:3:31: Stray "1" > +tests/qapi-schema/enum-int-member.json:2: Member of enum 'MyEnum' requires a string name This one's name is still good. > diff --git a/tests/qapi-schema/leading-comma-list.err b/tests/qapi-schema/leading-comma-list.err > index f5c870bb9c..fa9c80aa57 100644 > --- a/tests/qapi-schema/leading-comma-list.err > +++ b/tests/qapi-schema/leading-comma-list.err > @@ -1 +1 @@ > -tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean or "null" > +tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean, number or "null"
On 14.11.19 10:15, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qapi-schema/bad-type-int.json | 1 - >> tests/qapi-schema/enum-int-member.json | 1 - >> scripts/qapi/common.py | 25 ++++++++++++++++++++---- >> scripts/qapi/introspect.py | 2 ++ >> tests/qapi-schema/bad-type-int.err | 2 +- >> tests/qapi-schema/enum-int-member.err | 2 +- >> tests/qapi-schema/leading-comma-list.err | 2 +- >> 7 files changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/tests/qapi-schema/bad-type-int.json b/tests/qapi-schema/bad-type-int.json >> index 56fc6f8126..81355eb196 100644 >> --- a/tests/qapi-schema/bad-type-int.json >> +++ b/tests/qapi-schema/bad-type-int.json >> @@ -1,3 +1,2 @@ >> # we reject an expression with a metatype that is not a string >> -# FIXME: once the parser understands integer inputs, improve the error message >> { 'struct': 1, 'data': { } } >> diff --git a/tests/qapi-schema/enum-int-member.json b/tests/qapi-schema/enum-int-member.json >> index 6c9c32e149..6958440c6d 100644 >> --- a/tests/qapi-schema/enum-int-member.json >> +++ b/tests/qapi-schema/enum-int-member.json >> @@ -1,3 +1,2 @@ >> # we reject any enum member that is not a string >> -# FIXME: once the parser understands integer inputs, improve the error message >> { 'enum': 'MyEnum', 'data': [ 1 ] } >> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py >> index d61bfdc526..3396ea4a09 100644 >> --- a/scripts/qapi/common.py >> +++ b/scripts/qapi/common.py >> @@ -498,6 +498,8 @@ class QAPISchemaParser(object): >> raise QAPISemError(info, "Unknown pragma '%s'" % name) >> >> def accept(self, skip_comment=True): >> + num_match = re.compile(r'([-+]?inf|nan|[-+0-9.][0-9a-f.ex]*)') >> + >> while True: >> self.tok = self.src[self.cursor] >> self.pos = self.cursor > > This is yet another extension over plain JSON. RFC 8259: > > number = [ minus ] int [ frac ] [ exp ] > decimal-point = %x2E ; . > digit1-9 = %x31-39 ; 1-9 > e = %x65 / %x45 ; e E > exp = e [ minus / plus ] 1*DIGIT > frac = decimal-point 1*DIGIT > int = zero / ( digit1-9 *DIGIT ) > minus = %x2D ; - > plus = %x2B ; + > zero = %x30 ; 0 > > Extensions are acceptable when we have an actual use for it, and we > document them properly. Well, it isn’t really an extension, because this isn’t a JSON parser but just something that accepts anything that looks like a number and then lets Python try a conversion on it. > Isn't the parenthesis in your regular expression redundant? You’re right, but on second thought, maybe I should surround it by \< and \>. > What use do you have in mind for 'inf' and 'nan'? I could imagine inf being a useful default value, actually. nan, probably not so much. > Why accept leading '+' as in '+123'? > > Why accept empty integral part as in '.123'? > > Why accept '.xe.'? Kidding you, that must be a bug in your regexp. Well, kind of. I wanted to accept anything that looks in any way like a number and then let Python try to convert it. That’s also the reason why the case comes last. For that reason, I decided to keep the regex as simple as possible, because the attempted conversions would reject anything that isn’t (to Python) a valid number later. It was my impression that the QAPI schema isn’t really JSON anyway and that our QAPI schema parser isn’t a JSON parser. Under that assumption it simply seemed useful to me to accept anything that could potentially be a number to Python and convert it. Now, honestly, I still don’t see the point of having a strict JSON “parser” here, but if you insist. Seems possible to do in a regex. Though I do think it makes sense to support hex integers as an extension. > Please decide what number syntax you'd like to accept, then specify it > in docs/devel/qapi-code-gen.txt, so we can first discuss the > specification, and then check the regexp implements it. > > docs/devel/qapi-code-gen.txt update goes here: > > === Schema syntax === > > Syntax is loosely based on JSON (http://www.ietf.org/rfc/rfc8259.txt). > Differences: > > * Comments: start with a hash character (#) that is not part of a > string, and extend to the end of the line. > > * Strings are enclosed in 'single quotes', not "double quotes". > > * Strings are restricted to printable ASCII, and escape sequences to > just '\\'. > > --> * Numbers and null are not supported. OK. > Hrmm, commit 9d55380b5a "qapi: Remove null from schema language" left > two instances in error messages behind. I'll fix them. > >> @@ -584,7 +586,22 @@ class QAPISchemaParser(object): >> return >> self.line += 1 >> self.line_pos = self.cursor >> - elif not self.tok.isspace(): >> + elif self.tok.isspace(): >> + pass >> + elif num_match.match(self.src[self.pos:]): >> + match = num_match.match(self.src[self.pos:]).group(0) > > Sadly, the walrus operator is Python 3.8. > >> + try: >> + self.val = int(match, 0) >> + except ValueError: >> + try: >> + self.val = float(match) >> + except ValueError: >> + raise QAPIParseError(self, >> + '"%s" is not a valid integer or float' % match) >> + >> + self.cursor += len(match) - 1 >> + return >> + else: >> raise QAPIParseError(self, 'Stray "%s"' % self.tok) > > Any particular reason for putting the number case last? Because the match is so broad. >> >> def get_members(self): >> @@ -617,9 +634,9 @@ class QAPISchemaParser(object): >> if self.tok == ']': >> self.accept() >> return expr >> - if self.tok not in "{['tfn": >> + if self.tok not in "{['tfn-+0123456789.i": > > This is getting a bit ugly. Let's not worry about it now. > >> raise QAPIParseError(self, 'Expected "{", "[", "]", string, ' >> - 'boolean or "null"') >> + 'boolean, number or "null"') >> while True: >> expr.append(self.get_expr(True)) >> if self.tok == ']': >> @@ -638,7 +655,7 @@ class QAPISchemaParser(object): >> elif self.tok == '[': >> self.accept() >> expr = self.get_values() >> - elif self.tok in "'tfn": >> + elif self.tok in "'tfn-+0123456789.i": >> expr = self.val >> self.accept() >> else: >> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py >> index f62cf0a2e1..6a61dd831f 100644 >> --- a/scripts/qapi/introspect.py >> +++ b/scripts/qapi/introspect.py >> @@ -57,6 +57,8 @@ def to_qlit(obj, level=0, suppress_first_indent=False): >> ret += indent(level) + '}))' >> elif isinstance(obj, bool): >> ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false') >> + elif isinstance(obj, int) and obj >= -(2 ** 63) and obj < 2 ** 63: >> + ret += 'QLIT_QNUM(%i)' % obj > > Please explain the range check. Will do. >> else: >> assert False # not implemented >> if level > 0: >> diff --git a/tests/qapi-schema/bad-type-int.err b/tests/qapi-schema/bad-type-int.err >> index da89895404..e22fb4f655 100644 >> --- a/tests/qapi-schema/bad-type-int.err >> +++ b/tests/qapi-schema/bad-type-int.err >> @@ -1 +1 @@ >> -tests/qapi-schema/bad-type-int.json:3:13: Stray "1" >> +tests/qapi-schema/bad-type-int.json:2: 'struct' key must have a string value > > Test needs a rename, assuming it's not redundant now. I’m not adding a test here, it’s just the value has changed in 4d42815587063d. Thanks for reviewing! Max >> diff --git a/tests/qapi-schema/enum-int-member.err b/tests/qapi-schema/enum-int-member.err >> index 071c5213d8..112175f79d 100644 >> --- a/tests/qapi-schema/enum-int-member.err >> +++ b/tests/qapi-schema/enum-int-member.err >> @@ -1 +1 @@ >> -tests/qapi-schema/enum-int-member.json:3:31: Stray "1" >> +tests/qapi-schema/enum-int-member.json:2: Member of enum 'MyEnum' requires a string name > > This one's name is still good. > >> diff --git a/tests/qapi-schema/leading-comma-list.err b/tests/qapi-schema/leading-comma-list.err >> index f5c870bb9c..fa9c80aa57 100644 >> --- a/tests/qapi-schema/leading-comma-list.err >> +++ b/tests/qapi-schema/leading-comma-list.err >> @@ -1 +1 @@ >> -tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean or "null" >> +tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean, number or "null"
Max Reitz <mreitz@redhat.com> writes: > On 14.11.19 10:15, Markus Armbruster wrote: >> Max Reitz <mreitz@redhat.com> writes: >> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> --- >>> tests/qapi-schema/bad-type-int.json | 1 - >>> tests/qapi-schema/enum-int-member.json | 1 - >>> scripts/qapi/common.py | 25 ++++++++++++++++++++---- >>> scripts/qapi/introspect.py | 2 ++ >>> tests/qapi-schema/bad-type-int.err | 2 +- >>> tests/qapi-schema/enum-int-member.err | 2 +- >>> tests/qapi-schema/leading-comma-list.err | 2 +- >>> 7 files changed, 26 insertions(+), 9 deletions(-) >>> >>> diff --git a/tests/qapi-schema/bad-type-int.json b/tests/qapi-schema/bad-type-int.json >>> index 56fc6f8126..81355eb196 100644 >>> --- a/tests/qapi-schema/bad-type-int.json >>> +++ b/tests/qapi-schema/bad-type-int.json >>> @@ -1,3 +1,2 @@ >>> # we reject an expression with a metatype that is not a string >>> -# FIXME: once the parser understands integer inputs, improve the error message >>> { 'struct': 1, 'data': { } } >>> diff --git a/tests/qapi-schema/enum-int-member.json b/tests/qapi-schema/enum-int-member.json >>> index 6c9c32e149..6958440c6d 100644 >>> --- a/tests/qapi-schema/enum-int-member.json >>> +++ b/tests/qapi-schema/enum-int-member.json >>> @@ -1,3 +1,2 @@ >>> # we reject any enum member that is not a string >>> -# FIXME: once the parser understands integer inputs, improve the error message >>> { 'enum': 'MyEnum', 'data': [ 1 ] } >>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py >>> index d61bfdc526..3396ea4a09 100644 >>> --- a/scripts/qapi/common.py >>> +++ b/scripts/qapi/common.py >>> @@ -498,6 +498,8 @@ class QAPISchemaParser(object): >>> raise QAPISemError(info, "Unknown pragma '%s'" % name) >>> >>> def accept(self, skip_comment=True): >>> + num_match = re.compile(r'([-+]?inf|nan|[-+0-9.][0-9a-f.ex]*)') >>> + >>> while True: >>> self.tok = self.src[self.cursor] >>> self.pos = self.cursor >> >> This is yet another extension over plain JSON. RFC 8259: >> >> number = [ minus ] int [ frac ] [ exp ] >> decimal-point = %x2E ; . >> digit1-9 = %x31-39 ; 1-9 >> e = %x65 / %x45 ; e E >> exp = e [ minus / plus ] 1*DIGIT >> frac = decimal-point 1*DIGIT >> int = zero / ( digit1-9 *DIGIT ) >> minus = %x2D ; - >> plus = %x2B ; + >> zero = %x30 ; 0 >> >> Extensions are acceptable when we have an actual use for it, and we >> document them properly. > > Well, it isn’t really an extension, because this isn’t a JSON parser but > just something that accepts anything that looks like a number and then > lets Python try a conversion on it. I'm totally cool with deviating from JSON, all I really care about is proper schema language documentation. If we stick to JSON form number syntax, this is easy: replace "Numbers and null are not supported" by "null is not supported", done. Implementation should also be easy enough: convert RFC 8259's EBNF to a regexp, feed the matched string to Python's number parser, done. Fancier syntax we'd need to document ourselves. I'm willing to deal with that if we have a sufficiently compelling use for them. >> Isn't the parenthesis in your regular expression redundant? > > You’re right, but on second thought, maybe I should surround it by \< > and \>. > >> What use do you have in mind for 'inf' and 'nan'? > > I could imagine inf being a useful default value, actually. nan, > probably not so much. > >> Why accept leading '+' as in '+123'? >> >> Why accept empty integral part as in '.123'? >> >> Why accept '.xe.'? Kidding you, that must be a bug in your regexp. > > Well, kind of. > > I wanted to accept anything that looks in any way like a number and then > let Python try to convert it. That’s also the reason why the case comes > last. > > For that reason, I decided to keep the regex as simple as possible, > because the attempted conversions would reject anything that isn’t (to > Python) a valid number later. Ah, now I see. > It was my impression that the QAPI schema isn’t really JSON anyway and > that our QAPI schema parser isn’t a JSON parser. Under that assumption > it simply seemed useful to me to accept anything that could potentially > be a number to Python and convert it. > > Now, honestly, I still don’t see the point of having a strict JSON > “parser” here, but if you insist. Seems possible to do in a regex. > > Though I do think it makes sense to support hex integers as an extension. Let's start simple & stupid. Extensions can be added on top. Hexadecimal integers may well be compelling enough to justify an extension. >> Please decide what number syntax you'd like to accept, then specify it >> in docs/devel/qapi-code-gen.txt, so we can first discuss the >> specification, and then check the regexp implements it. >> >> docs/devel/qapi-code-gen.txt update goes here: >> >> === Schema syntax === >> >> Syntax is loosely based on JSON (http://www.ietf.org/rfc/rfc8259.txt). >> Differences: >> >> * Comments: start with a hash character (#) that is not part of a >> string, and extend to the end of the line. >> >> * Strings are enclosed in 'single quotes', not "double quotes". >> >> * Strings are restricted to printable ASCII, and escape sequences to >> just '\\'. >> >> --> * Numbers and null are not supported. > > OK. > >> Hrmm, commit 9d55380b5a "qapi: Remove null from schema language" left >> two instances in error messages behind. I'll fix them. >> >>> @@ -584,7 +586,22 @@ class QAPISchemaParser(object): >>> return >>> self.line += 1 >>> self.line_pos = self.cursor >>> - elif not self.tok.isspace(): >>> + elif self.tok.isspace(): >>> + pass >>> + elif num_match.match(self.src[self.pos:]): >>> + match = num_match.match(self.src[self.pos:]).group(0) >> >> Sadly, the walrus operator is Python 3.8. >> >>> + try: >>> + self.val = int(match, 0) >>> + except ValueError: >>> + try: >>> + self.val = float(match) >>> + except ValueError: >>> + raise QAPIParseError(self, >>> + '"%s" is not a valid integer or float' % match) >>> + >>> + self.cursor += len(match) - 1 >>> + return >>> + else: >>> raise QAPIParseError(self, 'Stray "%s"' % self.tok) >> >> Any particular reason for putting the number case last? > > Because the match is so broad. > >>> >>> def get_members(self): >>> @@ -617,9 +634,9 @@ class QAPISchemaParser(object): >>> if self.tok == ']': >>> self.accept() >>> return expr >>> - if self.tok not in "{['tfn": >>> + if self.tok not in "{['tfn-+0123456789.i": >> >> This is getting a bit ugly. Let's not worry about it now. >> >>> raise QAPIParseError(self, 'Expected "{", "[", "]", string, ' >>> - 'boolean or "null"') >>> + 'boolean, number or "null"') >>> while True: >>> expr.append(self.get_expr(True)) >>> if self.tok == ']': >>> @@ -638,7 +655,7 @@ class QAPISchemaParser(object): >>> elif self.tok == '[': >>> self.accept() >>> expr = self.get_values() >>> - elif self.tok in "'tfn": >>> + elif self.tok in "'tfn-+0123456789.i": >>> expr = self.val >>> self.accept() >>> else: >>> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py >>> index f62cf0a2e1..6a61dd831f 100644 >>> --- a/scripts/qapi/introspect.py >>> +++ b/scripts/qapi/introspect.py >>> @@ -57,6 +57,8 @@ def to_qlit(obj, level=0, suppress_first_indent=False): >>> ret += indent(level) + '}))' >>> elif isinstance(obj, bool): >>> ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false') >>> + elif isinstance(obj, int) and obj >= -(2 ** 63) and obj < 2 ** 63: >>> + ret += 'QLIT_QNUM(%i)' % obj >> >> Please explain the range check. > > Will do. > >>> else: >>> assert False # not implemented >>> if level > 0: >>> diff --git a/tests/qapi-schema/bad-type-int.err b/tests/qapi-schema/bad-type-int.err >>> index da89895404..e22fb4f655 100644 >>> --- a/tests/qapi-schema/bad-type-int.err >>> +++ b/tests/qapi-schema/bad-type-int.err >>> @@ -1 +1 @@ >>> -tests/qapi-schema/bad-type-int.json:3:13: Stray "1" >>> +tests/qapi-schema/bad-type-int.json:2: 'struct' key must have a string value >> >> Test needs a rename, assuming it's not redundant now. > > I’m not adding a test here, it’s just the value has changed in > 4d42815587063d. The test name 'bad-type-int' is now bad, because the int in it is no longer bad (pardon my lame puns). > Thanks for reviewing! Better late than never, I guess... You're welcome! > > Max > >>> diff --git a/tests/qapi-schema/enum-int-member.err b/tests/qapi-schema/enum-int-member.err >>> index 071c5213d8..112175f79d 100644 >>> --- a/tests/qapi-schema/enum-int-member.err >>> +++ b/tests/qapi-schema/enum-int-member.err >>> @@ -1 +1 @@ >>> -tests/qapi-schema/enum-int-member.json:3:31: Stray "1" >>> +tests/qapi-schema/enum-int-member.json:2: Member of enum 'MyEnum' requires a string name >> >> This one's name is still good. >> >>> diff --git a/tests/qapi-schema/leading-comma-list.err b/tests/qapi-schema/leading-comma-list.err >>> index f5c870bb9c..fa9c80aa57 100644 >>> --- a/tests/qapi-schema/leading-comma-list.err >>> +++ b/tests/qapi-schema/leading-comma-list.err >>> @@ -1 +1 @@ >>> -tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean or "null" >>> +tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean, number or "null"
diff --git a/tests/qapi-schema/bad-type-int.json b/tests/qapi-schema/bad-type-int.json index 56fc6f8126..81355eb196 100644 --- a/tests/qapi-schema/bad-type-int.json +++ b/tests/qapi-schema/bad-type-int.json @@ -1,3 +1,2 @@ # we reject an expression with a metatype that is not a string -# FIXME: once the parser understands integer inputs, improve the error message { 'struct': 1, 'data': { } } diff --git a/tests/qapi-schema/enum-int-member.json b/tests/qapi-schema/enum-int-member.json index 6c9c32e149..6958440c6d 100644 --- a/tests/qapi-schema/enum-int-member.json +++ b/tests/qapi-schema/enum-int-member.json @@ -1,3 +1,2 @@ # we reject any enum member that is not a string -# FIXME: once the parser understands integer inputs, improve the error message { 'enum': 'MyEnum', 'data': [ 1 ] } diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index d61bfdc526..3396ea4a09 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -498,6 +498,8 @@ class QAPISchemaParser(object): raise QAPISemError(info, "Unknown pragma '%s'" % name) def accept(self, skip_comment=True): + num_match = re.compile(r'([-+]?inf|nan|[-+0-9.][0-9a-f.ex]*)') + while True: self.tok = self.src[self.cursor] self.pos = self.cursor @@ -584,7 +586,22 @@ class QAPISchemaParser(object): return self.line += 1 self.line_pos = self.cursor - elif not self.tok.isspace(): + elif self.tok.isspace(): + pass + elif num_match.match(self.src[self.pos:]): + match = num_match.match(self.src[self.pos:]).group(0) + try: + self.val = int(match, 0) + except ValueError: + try: + self.val = float(match) + except ValueError: + raise QAPIParseError(self, + '"%s" is not a valid integer or float' % match) + + self.cursor += len(match) - 1 + return + else: raise QAPIParseError(self, 'Stray "%s"' % self.tok) def get_members(self): @@ -617,9 +634,9 @@ class QAPISchemaParser(object): if self.tok == ']': self.accept() return expr - if self.tok not in "{['tfn": + if self.tok not in "{['tfn-+0123456789.i": raise QAPIParseError(self, 'Expected "{", "[", "]", string, ' - 'boolean or "null"') + 'boolean, number or "null"') while True: expr.append(self.get_expr(True)) if self.tok == ']': @@ -638,7 +655,7 @@ class QAPISchemaParser(object): elif self.tok == '[': self.accept() expr = self.get_values() - elif self.tok in "'tfn": + elif self.tok in "'tfn-+0123456789.i": expr = self.val self.accept() else: diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index f62cf0a2e1..6a61dd831f 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -57,6 +57,8 @@ def to_qlit(obj, level=0, suppress_first_indent=False): ret += indent(level) + '}))' elif isinstance(obj, bool): ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false') + elif isinstance(obj, int) and obj >= -(2 ** 63) and obj < 2 ** 63: + ret += 'QLIT_QNUM(%i)' % obj else: assert False # not implemented if level > 0: diff --git a/tests/qapi-schema/bad-type-int.err b/tests/qapi-schema/bad-type-int.err index da89895404..e22fb4f655 100644 --- a/tests/qapi-schema/bad-type-int.err +++ b/tests/qapi-schema/bad-type-int.err @@ -1 +1 @@ -tests/qapi-schema/bad-type-int.json:3:13: Stray "1" +tests/qapi-schema/bad-type-int.json:2: 'struct' key must have a string value diff --git a/tests/qapi-schema/enum-int-member.err b/tests/qapi-schema/enum-int-member.err index 071c5213d8..112175f79d 100644 --- a/tests/qapi-schema/enum-int-member.err +++ b/tests/qapi-schema/enum-int-member.err @@ -1 +1 @@ -tests/qapi-schema/enum-int-member.json:3:31: Stray "1" +tests/qapi-schema/enum-int-member.json:2: Member of enum 'MyEnum' requires a string name diff --git a/tests/qapi-schema/leading-comma-list.err b/tests/qapi-schema/leading-comma-list.err index f5c870bb9c..fa9c80aa57 100644 --- a/tests/qapi-schema/leading-comma-list.err +++ b/tests/qapi-schema/leading-comma-list.err @@ -1 +1 @@ -tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean or "null" +tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean, number or "null"
Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qapi-schema/bad-type-int.json | 1 - tests/qapi-schema/enum-int-member.json | 1 - scripts/qapi/common.py | 25 ++++++++++++++++++++---- scripts/qapi/introspect.py | 2 ++ tests/qapi-schema/bad-type-int.err | 2 +- tests/qapi-schema/enum-int-member.err | 2 +- tests/qapi-schema/leading-comma-list.err | 2 +- 7 files changed, 26 insertions(+), 9 deletions(-)