Add a no_std/alloc feature

[Xanewok]

This builds up on #588 (thanks @Freax13!).

With this, we support no_std with alloc crate, starting from Rust 1.36 (this was done so that we can use it in WebAssembly environment but any such environment will work).

To minimize the noise, I tried to use the facade approach already used in Serde to provide some consistency between the two and improved on the io facade to work around it missing in core.

Thanks in advance for taking your time to review this and let me know if I can improve anything to help this land!

Closes #588. Closes #516.

[dtolnay]

Thanks! This is so much easier to review than the earlier no_std attempts. I intend to dedicate time toward getting this reviewed and merged.

I have not looked at most of this yet but here are some initial comments:

[dtolnay]

Please find a way to produce a reasonable error message for this case. The current error if you do serde_json = { default-features = false } is >1500 lines which is insane.

https://github.com/dtolnay/syn/blob/1.0.14/tests/features/mod.rs + https://github.com/dtolnay/syn/blob/1.0.14/tests/features/error.rs shows one good way.

[Xanewok]

Oh, that's neat! I wanted to do so but couldn't think of a reliable way to only show one compilation error.

As written here, maybe it'd be good to just support std and no-std (implying using alloc)? This would mean we wouldn't have to care about detecting no std and alloc anymore.

EDIT: Nevermind, we still need the Cargo alloc feature for serde/alloc and thus we still must check for either std or alloc to be enabled.

[Xanewok]

Done in 7852d2f.

Is there a way to run trybuild with custom feature flags, like compiletest? It'd be quite handy to have a UI test for --no-default-features so that we can catch whenever the error message regresses.

[cuviper]

The current error if you do serde_json = { default-features = false } [...]

Shouldn't this new error be considered a breaking change?

[dtolnay]

This can be simplified to not(feature = "std") because we know alloc must be enabled in that case. Same thing below.

In general feature = "alloc" should not need to appear anywhere.

[Xanewok]

I did that to introduce distinction between std, alloc and no feature at all, but maybe it'd be good only introduce two: default std and no-std (which implies alloc and is supported from 1.36 onwards)...

I'll adapt the code accordingly, thanks!

[dtolnay]

From cargo doc --no-default-features --features alloc:

This seems bad because the following would be accepted in alloc mode and break if a different part of the dependency graph enables std mode.

struct S;
impl serde_json::ser::Formatter for S {
    fn write_null<W: ?Sized>(&mut self, _: &mut W) -> Result<(), &'static str> {
        Err("")
    }
}

Either Formatter needs to be removed from the public API in alloc mode or the error type needs to be changed to an opaque type with no more than a subset of io::Error's API.

[Xanewok]

Oops, I didn't notice that, thanks!

Since std::io::Result is part of the default, public API I don't think we can be backwards compatible (methods are one thing but sth may depend on it being explicitly this type from std IIUC).

Because of this, I think the only thing we can do now, until std::io somehow lands in core, is to hide this from public API in no-std case.

EDIT: Again, sorry for the noise. You're right, that's a concern about code written for core not being compatible with std (not being a subset of real io) rather than the concrete public API... I'm on it.

[Xanewok]

Reimplemented a small subset of std::io::Error and friends so that accidentally opting into std should not break stuff

[Xanewok]

I addressed the feedback and thus also managed to unify the io facade as well (which now lives in io/mod.rs, while the small no_std-specific mplementation lives in io/core.rs). With this, the only place which specifies std-related conditional compilation is:

1. lib.rs - for the prelude/facade used across the library
2. io module - to reexport either std::io or the no_std reimplemented bits
3. features_check - to provide user-friendly compilation error for missing both std/alloc features

Overall, I'm pretty happy with how the io improvements turned out to be and with the overall patch. Thanks for reviewing this and let me know if I should change anything else!

[dtolnay]

Thanks, this looks great to me. I am making some tweaks in later commits but this is ready to land.

Question: for your use case do you require to_writer and/or Serializer, or only to_string? I am removing to_writer and Serializer from the public API in no-std mode for now. These rely on the caller to pass a Write impl and the only impl this PR provides in serde_json is for Vec<u8>. It is going to require more design work to figure out how best to expose that. With the io facade as implemented here, the caller is unable to write Write impls, and I also don't want to be in the business of copying many more Write impls from std into serde_json.

[Xanewok]

Awesome, thank you!

for your use case do you require to_writer and/or Serializer, or only to_string?

I think to_string suffices for now, yeah.

(...) and I also don't want to be in the business of copying many more Write impls from std into serde_json.

That's completely understandable! It's worth noting that for the second iteration of io I designed it so that we don't need to do any std-specific conditional compilation in files other than the io or lib facades (but couldn't make it proper pub(crate) due to io::Result leaking in the public API).

I am making some tweaks in later commits (...)

Thanks for not blocking on this but it'd be completely fine to ask me to follow up with other tweaks related to this PR!

[dtolnay]

Published in 1.0.45.