From: "Herman, Géza" <geza.herman@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: "Géza Herman" <geza.herman@gmail.com>,
"Mattias Engdegård" <mattiase@acm.org>,
emacs-devel@gnu.org
Subject: Re: [PATCH] Implement fast verisons of json-parse functions
Date: Sat, 30 Mar 2024 19:36:57 +0100 [thread overview]
Message-ID: <87wmpjmsie.fsf@gmail.com> (raw)
In-Reply-To: <86plvbdgcx.fsf@gnu.org>
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Herman, Géza <geza.herman@gmail.com>
>> 3 test failures:
>> 1. Handling of utf-8 decode errors: the new parser emits
>> json-utf8-decode-error instead of json-parse-error (this is
>> what
>> the test expects). I can fix this by modifying the test
>
> OK, but we will need to mention this in NEWS as an incompatible
> change.
Yes. I'm just mentioning this as an alternative solution:
originally the parser emitted json-parse-error for this, it was
changed during the review. So if we prefer maintaining
compatibility, it's easy to revert this change.
>> 2. Handling of a single \0 byte
>
> Does JSON allow null bytes in its strings? If not, why
> wrong-type-argument is not TRT?
That's correct, null bytes are not allowed (anywhere, not just in
strings). But my point is that the old parser made a special
distinction here. It is not just null bytes which is not allowed
in JSON, but for example, \x01 isn't allowed either. But, for
null bytes, the old parser gives a different error message than
for \x01 bytes. But from the JSON spec perspective, both \x00 and
\x01 are forbidden in the same way. I don't know why null bytes
are handled specially in this regard, so I didn't follow this
behavior in my parser. Maybe this special error case was added
because libjansson couldn't parse strings with null bytes back
then (because the API only accepted zero-terminated strings)?
To me, wrong-type-argument means that the input argument to the
parser is incorrect. Like it's not a string, but an integer. But
here, the parser gets a string, it's just that the string has null
bytes in it somewhere. The type of the argument to json-parse-*
is fine, it's the value which has the problem. So the parser
should give some kind of json-error in my opinion, not
wrong-type-argument. But, of, course, if we consider
strings-with-null and strings-without-null as two different types,
then the wrong-type-argument error makes sense (though I don't
know why we'd want to do this).
>> 3. Handling objects with duplicate keys.
>
> I think we should modify the expected results of the test to
> match the
> new behavior, and leave the order as it is now.
OK.
> But please also compare with what the Lisp implementation does
> in
> these cases, as that could give us further ideas or make us
> reconsider.
I checked json-read, and it seems that it has the exact same
behavior that my parser has. I thought that json-read can only
produce one format, but it turned out it has json-object-type and
json-array-type variables, so it can produce the same variety of
output that the C-based parsers can do. I think that the doc of
json-read should mention this fact. Anyways, the doc says:
(defvar json-object-type 'alist
"Type to convert JSON objects to.
Must be one of `alist', `plist', or `hash-table'. Consider
let-binding
this around your call to `json-read' instead of `setq'ing it.
Ordering
is maintained for `alist' and `plist', but not for `hash-table'.")
I played with this a little bit, and it works as described (for
hash tables, it keeps the last key-value pair).
I think this behavior is important, because this is used when
pretty-formatting JSON. Pretty formatting shouldn't remove
duplicate entries, nor change the ordering of members. Because
the new parser also behaves like this, it can be used to speed up
pretty formatting as well (yeah, I know, half of it, as there is
no new to-JSON serializer implemented yet).
next prev parent reply other threads:[~2024-03-30 18:36 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-19 18:23 [PATCH] Implement fast verisons of json-parse functions Herman, Géza
2024-03-19 18:47 ` Eli Zaretskii
2024-03-19 18:50 ` Herman, Géza
2024-03-19 19:28 ` Eli Zaretskii
2024-03-19 19:50 ` Herman, Géza
2024-03-20 3:37 ` Eli Zaretskii
2024-03-23 10:40 ` Herman, Géza
2024-03-23 11:11 ` Eli Zaretskii
2024-03-30 7:43 ` Eli Zaretskii
2024-03-30 10:50 ` Herman, Géza
2024-03-30 13:00 ` Eli Zaretskii
2024-03-30 18:36 ` Herman, Géza [this message]
2024-03-30 20:18 ` [External] : " Drew Adams
2024-03-30 20:51 ` Herman, Géza
2024-03-30 23:28 ` Drew Adams
2024-03-31 5:08 ` Eli Zaretskii
2024-03-31 14:51 ` Mattias Engdegård
2024-03-31 15:44 ` Eli Zaretskii
2024-04-01 8:48 ` Mattias Engdegård
2024-04-01 16:54 ` Herman, Géza
2024-03-31 17:48 ` Adam Porter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wmpjmsie.fsf@gmail.com \
--to=geza.herman@gmail.com \
--cc=eliz@gnu.org \
--cc=emacs-devel@gnu.org \
--cc=mattiase@acm.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).