Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

Remove json-rust from benchmark? #21

Closed
ecton opened this issue Jan 18, 2023 · 2 comments · Fixed by #22
Closed

Remove json-rust from benchmark? #21

ecton opened this issue Jan 18, 2023 · 2 comments · Fixed by #22

Comments

@ecton
Copy link
Contributor

ecton commented Jan 18, 2023

In an attempt to try to find a good JSON DOM library that supports borrowing, I came across this benchmark and almost used the json crate. I found it odd that the crates.io listing has no README, yet it's such a popular crate and performs quite well!

Then I saw on github that it isn't maintained. Not a problem if it's done with development, as a JSON library certainly could be. But another issue points out a soundness issue that Miri also doesn't like.

Given that it's unmaintained and potentially unsound, I'm not sure how valid of a comparison it actually provides in today's JSON ecosystem. In fact, I worry that the mere presence of it on this list might be leading some developers to adopt the crate despite, in my opinion, it not being a safe crate to adopt given the open issues.

@dtolnay
Copy link
Member

dtolnay commented Jan 18, 2023

Oh that's too bad, I liked that crate a lot. :(

I agree a UB issue open for 2 years without response is concerning and we should avoid recommending the crate at this point. Could you send a PR to remove it from the readme and remove the relevant benchmark code from the implementation? I would love to put it back later if the crate becomes maintained again.

Thank you for raising this.

@ecton
Copy link
Contributor Author

ecton commented Jan 18, 2023

Absolutely will send in a PR. Thank you for the prompt reply (and your countless other contributions to the ecosystem)!

ecton added a commit to ecton/json-benchmark that referenced this issue Jan 18, 2023
The crate currently is unmaintained and has a longstanding open issue
regarding a soundness issue. Closes serde-rs#21.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants