Update simd-json to make serenity compile again

[kangalio]

And fix an unsoundness (lol)

I found out that serenity doesn't compile with the simd_json feature in poise https://github.com/serenity-rs/poise/actions/runs/6158123854/job/16710276804

[kangalio]

After reading #2474 (comment), I realize serenity actually re-exports the raw serialization and deserialization functions from the underlying JSON library in serenity::json::prelude. As such, any upgrade to serde-json or simd-json is a breaking change >:(

We could hack around this unfortunate decision by replacing the re-exported simd_json::from_str with a custom impl that keeps the old signature (at the cost of a soundness hole, that probably no one ever will encounter because this is such a niche serenity API). We should also maybe deprecate the re-exports from the underlying JSON libraries?

[mkrasnitski]

Is changing the trait bound to DeserializeOwned a breaking change? I'm not sure - since we're dealing with JSON, are there any types that don't implement DeserializeOwned?

[kangalio]

serenity::json::from_str isn't public API! Only serenity::json::prelude::from_str is. The two differ in that the public one is directly re-exported from serde_json or simd_json, whereas the private one is a wrapper function around serde_json or simd_json

---

I have another hack idea to fix the semver issue: depend on both simd_json 0.4 and 0.10 - use 0.10 internally, re-export 0.4. It's silly but it's a silly solution to a silly problem (namely, serenity publicly re-exporting implementation details)

[mkrasnitski]

Does from_slice work with multi-byte characters? I think that passing in a &mut s.to_string() would still be fine here, since even if the string becomes invalid UTF-8, it gets dropped immediately afterward.

[kangalio]

Does from_slice work with multi-byte characters?

Yes:

I think that passing in a &mut s.to_string() would still be fine here, since even if the string becomes invalid UTF-8, it gets dropped immediately afterward.

I originally thought the mere existence of a non UTF-8 str is instant UB but I looked it up and it seems you're right

Still, that would need an unsafe block. Using from_slice needs no unsafe block and has no performance penalty (I expect?

[mkrasnitski]

Ok, turns out this approach doesn't entirely work because having things like OwnedValue and Object imported from simd-json 0.10 is not compatible with methods like to_string which are imported from 0.4. What would need to happen is for two separate preludes to be created, one which is pub that re-exports everything from 0.4, and one which is internal containing everything from 0.10. Additionally, the pub types like Value, JsonMap, JsonError, etc. would need to be set to their versions from simd_json_old, which means that their new versions should therefore also be exported only in the "internal-only" prelude.

Considering that the json macro is broken on old simd-json versions, I'm really not sure if it's worth fixing this just to re-export a broken API, because the later rebases of next on top of that work would be pretty painful. Maybe we just bite the bullet and re-export the new versions of the functions, except for from_str for which the wrapper is already pub anyways? I don't think any of the other function signatures have been broken by simd-json.

The other fixes in this PR (like the feature flag typo fix) are fine and can be left in.

[mkrasnitski]

Any updates on this?

[kangalio]

I'm currently not motivated to implement that separate prelude thing, and pushing breaking changes into current feels wrong to me. Maybe it's ok if I just close this PR, let simd-json continue to be broken on current, and wait for next instead ™ ?

[mkrasnitski]

If you'd like, we can close this and split it into separate PRs.

[kangalio]

Split into what PRs? What would they do?

[mkrasnitski]

Replacing the uses of the incorrect simd-json feature with simd_json is relatively simple and worth incorporating.

[kangalio]

More trouble than its worth tbh. That replacing is a "pick up a piece of litter in your way" level of change