Complicate entropy rng

[pitdicker]

This is (in my opinion) the proper fix for #503: make OsRng unavailable if the platform is not supported.

This also makes JitterRng::new unavailable on Wasm.

EntropyRng already had tricky logic. Making it work nicely when OsRng and/or JitterRng are not available at compile time complicates it further. And I didn't want to think how it was going to look when we want to add some configurable third entropy source.

I have tried to rewrite EntropyRng to have hopefully simpler logic. Because we need wrappers for OsRng and JitterRng to make it compile on all platforms, I went all the way and added a little trait to ease implementation.

[dhardy]

Hmm, those cfgs are horrible. Can we simplify them to cfg(any(feature="std", feature="stdweb"))?

I've thought about enabling stdweb by default, but it doesn't affect that cfg. We could make std imply stdweb which would fix that, but I think some users would complain in that case.

Of course it's possible that we get compile errors on other platforms not directly supported, but I don't think we have to care about that (if someone needs support for another platform, they can make a PR to fix).

[dhardy]

Brilliantly simple fix! 👍

[pitdicker]

Hmm, those cfgs are horrible. Can we simplify them to cfg(any(feature="std", feature="stdweb"))?

Yes they are 😄. In the end we should have 5 of them, 2 in rngs::mod.rs, and 3 in entropy.rs (could be reduced to 2 with the module trick). The 6 in lib.rs and deprecated.rs should be only temporary.

I've thought about enabling stdweb by default, but it doesn't affect that cfg. We could make std imply stdweb which would fix that, but I think some users would complain in that case.

Yes, the ones that use wasm-bindgen, or some other solution.

Of course it's possible that we get compile errors on other platforms not directly supported, but I don't think we have to care about that (if someone needs support for another platform, they can make a PR to fix).

Doing it exactly with the whole cfg-list as in this PR looks heavy, but seems like the right solution to me.

[dhardy]

Ah, cfg(any(feature="std", feature="stdweb")) is in any case wrong; I guess it would need to be cfg(any(all(target_arch = "wasm32", features = "stdweb"), all(not(target_arch = "wasm32"), features = "std"))). So not so worth bothering with.

Okay, then I think we're ready to merge?

[pitdicker]

I'd like to try it on the 0.5 branch first instead.

Closing in favour of #512.