1953305 - JSON Data Structure Parsing with JSONParseHandler

Status: NEW

(Core :: XPCOM, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

SpiderMonkey added the JSONParseHandler API (https://searchfox.org/mozilla-central/rev/3a3965bc78cf76f23ac0569c7f3c98b15f1dca5c/js/public/JSON.h#123-173) in bug 1858803. This so far has only been used by one component, other C++ components are still using the jsoncpp library.

This is a WIP patch which introduces a basic AST data structure which could be used for JSON values from C++ code using the JSONParseHandler API for parsing.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: WIP: Bug 1953305 - WIP JSON Data Structure Parsing with JSONParseHandler

Comment 2

[Henri Sivonen (:hsivonen)]

Nika, do you have advice on when Gecko code should favor parsing JSON using the code to be introduced here vs. parsing to serde_json::Value?

AFAICT, the C++ ergonomics of JSONValue here are worse than the Rust ergonomics of serde_json::Value (not a complaint about the code here; just the situation with C++ not having an ergonomic counterpart to Rust's match), so it seems to me that even when code in proximity of JSON parsing is C++, it might make sense to use serde_json::Value and make the result of walking the tree go over FFI.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Henri Sivonen (:hsivonen) from comment #2)

Nika, do you have advice on when Gecko code should favor parsing JSON using the code to be introduced here vs. parsing to serde_json::Value?

AFAICT, the C++ ergonomics of JSONValue here are worse than the Rust ergonomics of serde_json::Value (not a complaint about the code here; just the situation with C++ not having an ergonomic counterpart to Rust's match), so it seems to me that even when code in proximity of JSON parsing is C++, it might make sense to use serde_json::Value and make the result of walking the tree go over FFI.

I expect that if your code is in Rust serde_json is a nice interface for it. This is specifically using the SpiderMonkey JSON parser, and would be used by C++ code which wants to parse and handle JSON code without involving any new Rust code. For simple parsers, adding the complexity of Rust FFI would not be worth it. We have some existing users of the jsoncpp library in tree which are the type of consumers which this would potentially be preferable to.

That being said, this patch hasn't landed, in part because it doesn't have an immediate use-case, and in part because it has it's own problems which I would want to fix before landing (specifically that the SpiderMonkey API doesn't handle UTF-8 right now, so callers of this API might end up using the Latin1 callbacks when trying to pass their nsCString in, which may misbehave when fed UTF-8 text (specifically \uXXXX escapes will be broken, which means the problem likely wouldn't be noticed locally)).

Comment 4

[Andrew McCreight [:mccr8]]

FYI, I took up Nika's idea and implemented a basic AST wrapper thing in bug 1885221, in part 3. See JSIPCJSON. I guess the name isn't great if it is used for something else but it could be renamed.