On 14.11.19 10:15, Markus Armbruster wrote:
> Max Reitz <mre...@redhat.com> writes:
>
>> Signed-off-by: Max Reitz <mre...@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"
signature.asc
Description: OpenPGP digital signature
