Paolo Bonzini <[email protected]> writes:
> Il ven 30 gen 2026, 14:00 Markus Armbruster <[email protected]> ha scritto:
>
>> > Another benefit is that QEMU can report the first parsing error
>> > immediately, without waiting for delimiters to be balanced.
>>
>> Sounds promising! Let's see...
>>
>> Before the series:
>>
>> $ socat "READLINE,prompt=QMP> " UNIX-CONNECT:$HOME/work/images/test-qmp
>> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 10},
>> "package": "v10.2.0-567-gfb6b66de43-dirty"}, "capabilities": ["oob"]}}
>> QMP> [{"a"]
>>
>> Parse error not diagnosed right away, but ...
>>
>> QMP> }
>> {"error": {"class": "GenericError", "desc": "JSON parse error, missing :
>> in object pair"}}
>>
>> .... only when the streamer decides the expression is complete.
>>
>> After the series:
>>
>> QMP> [{"a"]
>> {"error": {"class": "GenericError", "desc": "JSON parse error at line 1,
>> column 6, expecting ':'"}}
>>
>> Cool! However, if I do it again, things fall apart:
>>
>> QMP> [{"a"
>> QMP> }
>> QMP> }
>> QMP> }
>> QMP> ]
>> QMP> ]
>> {"error": {"class": "GenericError", "desc": "JSON parse error at line 7,
>> column 1, expecting value"}}
>>
>> Parse error recovery not quite right?
>
> Well, if you read the above very carefully :) the error is *reported*
> immediately, but recovery still waits for delimiters to be balanced.
Yes, but the recovery regressed a bit.
Having read the entire series now, I wonder whether it's related to
capping @brace_count and @bracket_count at zero [PATCH 3].
My example input [{"a"] puts the parser in error recovery state with
brace_count == 1, bracket_count == 0. Your parser will silently throw
away further input until brace_count drops to zero.
The old parser additionally snaps out of this unresponsive state when
bracked_count goes negative.
> In testing, when I got an error I just typed a long enough variant on
> "]}]}]}" and that is enough to recover—just like in the old parser.
With the old parser, I can park my finger on ']' or '}' for a moment,
hit return, and be good. With the new one, I have to park on both.
More seriously, the new parser seems to break a recovery technique
documented in docs/interop/qmp-spec.rst:
Forcing the JSON parser into known-good state
---------------------------------------------
Incomplete or invalid input can leave the server's JSON parser in a
state where it can't parse additional commands. To get it back into
known-good state, the client should provoke a lexical error.
The cleanest way to do that is sending an ASCII control character
other than ``\t`` (horizontal tab), ``\r`` (carriage return), or
``\n`` (new line).
Works with old parser:
QMP> [{"a"]
QMP> ^L
{"error": {"class": "GenericError", "desc": "JSON parse error, stray '\f'"}}
QMP> {}
{"error": {"class": "GenericError", "desc": "QMP input lacks member
'execute'"}}
Note: the ^L is a formfeed character.
New parser:
QMP> [{"a"]
{"error": {"class": "GenericError", "desc": "JSON parse error at line 1,
column 6, expecting ':'"}}
QMP> ^L
QMP> {}
Good: the parse error is reported immediately.
Bad: the formfeed no longer forces the parser into known-good state.
This needs fixing.
> Still
> the difference in error reporting matters, because it gives feedback that
> is immediately useful, rather than possibly delayed forever.
Reporting parse errors immediately is such a lovely quality of life
improvement. May I have that without regressing error recovery?
> The policy is easy to change, either in v2 or in subsequent work, because
> recovery is layered on top of json-parser and its code is nothing more than
> "if you believe it's a good time to recover, reset the parser".
>
> Paolo
>> > On top of the benefits intrinsic in the push architecture, it so happens
>> > that it's really easy to add a location to JSON parsing errors now, so
>> > do that as well.
>> >
>> > Paolo
>>
>>
