Consider replacing jsoncpp

[ekpyron]

• The repo looks like jsoncpp only receives minimal maintenance.
• The workaround of manually downloading jsoncpp in cmake is evil (we could of course attempt to change that in isolation, but there don't seem to be package updates for jsoncpp e.g. in recent ubuntu either making this a continuing hassle).
• jsoncpp is neither the fastest nor the smallest nor the most conformant option.
• There are a number of pretty much drop-in replacements available.
• We can easily move to a header-only library which pretty much avoids all dependency issues in perpetuity.

I'd suggest to consider replacing jsoncpp with either http://rapidjson.org/ or https://github.com/nlohmann/json (or any other of the available candidates).

Is there any reason for staying with jsoncpp other than the required work a move would be (which I wouldn't expect to actually be too bad)?

Closes #3557.

[christianparpart]

wrt. nlohmann/json, they state Our whole code consists of a single header file json.hpp, which would be the main reason for me to choose them.

Also the examples he provides in the readme look very convincing to me. => 👍 for this one.

[chriseth]

I had a quick look over nlohmann and found the following two items that might be issues:

• https://github.com/nlohmann/json#comments-in-json (just remember we had some discussion about that at some point)
• https://github.com/nlohmann/json#order-of-object-keys - we have to check if that can lead to nondeterminism

[ekpyron]

I think while the insertion order of the keys is not preserved, they don't have an "undefined" order - they use a std::map, so in the end the keys will be ordered by string comparison and that's deterministic - but we need to recheck, yes.

Regarding comments I'm not sure - should we allow them even though they are actually not valid json? I know that json-cpp only supports them in a weird way as well (whether a comment is fine depends on whether it's key-comment-comma or key-comma-comment, if I recall correctly).

[nlohmann]

Hi, I'm the author for nlohmann/json. Please let me know if you need any assistance!

[chriseth]

@nlohmann great to have you here! Can you comment on the order of keys? Is it a defined order that may be different from the insertion order or can it depend on things like memory layout?

[nlohmann]

We use an std::map internally, so the order when iterating over an object is sorted alphabetically w.r.t. std::string::operator<.

[chriseth]

Wonderful, thanks!

[aarlt]

Ah nice, I just found this issue. @ekpyron I had the same impression and I think it would be nice to replace jsoncpp. Around a year ago I created a branch to replace jsoncpp but somehow I never created a PR (https://github.com/aarlt/solidity/tree/nlohmann-json). I don't remember exactly what the state was, but if you would like to replace jsoncpp with nlohmann/json, I could support you.

[nlohmann]

As I wrote earlier, just let me know if you need assistance!

[ekpyron]

I brought this up in our weekly call earlier and @chriseth said we should postpone this until after 0.6.0 is released.
However aarlt@1329c25 is a great reference for estimating how much effort this will be - it doesn't look too bad, so I think we can do this quickly after 0.6.0.

[axic]

nlohmann's seems to be better in every single benchmark compared to jsoncpp: https://github.com/miloyip/nativejson-benchmark

[chriseth]

Are benchmarks the benchmark? I would say it is more important that the code is easy to understand and does not create any ambiguities.

[axic]

I think the API of nlohman was quite good, even compared to jsoncpp.

The benchmark categories are:

• code size
• parsing speed
• serialisation speed

There is a huge difference between speeds, which can be an issue with large compilations (large JSON outputs or input). I think this may be the reason people were complaining on compilation speed.

[ekpyron]

By the way: @Marenz is having problems problems with the performance of the AST export and import tests, for which I think the json parsing and emitting part is a major bottleneck - so it's not like performance is entirely irrelevant... and regarding readability: just have a quick look at aarlt@1329c25 - the API is very similar to jsoncpp and closer to STL, which I'd consider an advantage.

[axic]

Regarding comments I'm not sure - should we allow them even though they are actually not valid json? I know that json-cpp only supports them in a weird way as well (whether a comment is fine depends on whether it's key-comment-comma or key-comma-comment, if I recall correctly).

I would say there is no point supporting comments when they are not a valid JSON feature. IIRC we have them disabled as much as jsoncpp allows disabling them.

Also apparently nlohman-json now supports CBOR integration: https://github.com/nlohmann/json#binary-formats-bson-cbor-messagepack-and-ubjson

This means we could in theory replace our code or at least replace it in the tests. However this is not a pressing issue as our small CBOR code works well and as intended.

[maximegmd]

Dropping another suggestion that favors performance: https://github.com/simdjson/simdjson

The API is also quite straight forward.

[Zachinquarantine]

just curious, what is https://github.com/ethereum/solidity/blob/develop/scripts/install_obsolete_jsoncpp_1_7_4.sh and why does it exist?

[cameel]

It used to be needed in CI on macOS and older Ubuntu versions: #4073 . For Ubuntu It was later replaced with a package (#6453). For macOS we still use it though. osx_install_dependencies.sh runs that script.

I'm not completely sure why macOS needs specifically 1.7.4 but there's a comment comment in one of these PRs saying that it's somehow related to how we download and build it on our own during cmake setup:

Would be nicer to get rid of the script altogether, but I'd suggest to keep it for the macos build for now and remove it, once we got rid of the whole build-json-cpp-on-our-own-mess in general (then we can just use 1.8.4 from homebrew instead - they in fact have it: https://formulae.brew.sh/formula/jsoncpp).

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[cameel]

I'm going to keep this one open. It may not be high on our priority list, but I think we should move away from jsoncpp eventually.

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

[cameel]

This was implemented by #14877.