Support wasm32-unknown-emscripten

[huangjj27]

this pr is trying to fix #442

[pitdicker]

First time someone opened a PR for me 😄.

It seems my fix didn't work because the combination of featurres and target-specific features is not yet supported by cargo rust-lang/cargo#1197. I'll try just duplicating the cargo.toml section next.

[CryZe]

This does not seem correct, as wasm32-unknown-emscripten is a Unix, and thus both the unix impl and the stdweb impl would get compiled in. I'm pretty sure the unix implementation works just fine, so I'm not entirely sure what this is trying to fix.

[pitdicker]

@CryZe Yes, that is what I am looking at at the moment. But compiling LLVM for emscripten takes a while...

Do you already have a fix?

[CryZe]

The chrono / time crates ran into the target feature problem as well. Nothing related to emscripten though, idk what emscripten has to do with this.

[CryZe]

Alright, I read a bit further about what is going on here and it seems to me like you should just bring in stdweb when the feature is activated (even though it's unused) but keep the unix implementation for emscripten.

[pitdicker]

I have fixed it up enough to compile with wasm32-unknown-emscripten. But I get a linker error when trying to run tests:

# after endless stream of warnings
error: unresolved symbol: llvm_ctpop_i8

I am not really jumping on finding the cause of that...

[CryZe]

Why would the emscripten target use the stdweb implementation though, it already has a working implementation :(

[CryZe]

This doesn't check the architecture, but the other impl checks the architecture. This leaves a hole where no impl is compiled in.

[CryZe]

This completely ignores the existence of the asmjs target or the experimental target.

[CryZe]

We still have no reason why the stdweb implementation is even used for the emscripten target, which was already working perfectly fine.

[pitdicker]

@CryZe I don't care about the WASM/js targets at all and don't know much about them either t.b.h., just trying to help out.

Can you help me out with a little background?

As I understand it, emscripten emulates a basic Unix environment. Because we have a generic Unix implementation that reads from /dev/urandom, it works all right. (Although I think the docs should note it may not be of crypto-quality depending on the browser etc., see https://github.com/kripken/emscripten/blob/incoming/src/library_fs.js#L1291).

Stdweb is an alternative for 'bare' webassembly, which lives behind the stdweb feature flag in rand. If someone decides to use stdweb, even when the emcripten environment is also available, is that wrong?

What is the idea of the different targets? I can keep wasm32-unknown-unknown and wasm32-unknown-emscripten apart. Is asmjs-unknown-emscripten still used, and used in combination with stdweb? And what is an experimental target?

Would this be right?

if architecture is wasm32 or asmjs
    if feature is stdweb
        use stdweb
    else if os is emscripten
        use /dev/urandom
    else
        use "not implemented" stub

[CryZe]

In my opinion it should be:

if architecture is unix (all emscripten targets)
    use /dev/urandom
else if wasm and not emscripten (so only wasm32-unknown-unknown atm)
    if stdweb
        use stdweb
    else
        use "not implemented" stub

You may be right that the stdweb implementation may be more cryptographically correct, but until we are sure about that I'd trust the emscripten implementation more.

The thing is that with the cargo bug, the stdweb feature accidentally gets carried over to the emscripten targets. So having the stdweb feature activated on those targets isn't really an indicator that you actually want the stdweb feature over the actual emscripten implementation.

Rough explanation of the individual targets:

• asmjs-unknown-emscripten: Uses emscripten and compiles to asm.js
• wasm32-unknown-emscripten: Uses emscripten and compiles to asm.js and then uses binaryen to compile to wasm
• wasm32-experimental-emscripten: Uses LLVM's wasm target in combination with emscripten
• wasm32-unknown-unknown: Uses the raw LLVM wasm target

[pitdicker]

Thank you! With that approach I see changing only the line you pointed out does the trick.

[CryZe]

This looks good now 👍

[huangjj27]

@CryZe @pitdicker Thanks for your fixes! I'll inform the yew's author to make a test for the game of life.|

Update:
I'm sorry that I haven't made it clear that what I want is not support stdweb feature in emscripten but make the feature not to influence the target, so it's ok not to compile with stdweb when using wasm32-unknown-emscripten target.

[dhardy]

I don't know much about this issue, but it looks like this PR is ready. @huangjj27 you can further discuss in #442 I guess.