Add --trap-mode=allow/clamp/js argument to asm2wasm and s2wasm

[jgravelle-google]

Update command-line arguments to take trap mode itself as a string parameter.

Not sure how reasonable it is to add an Invalid mode to the TrapMode enum just for error handling. I basically wanted a way to separate the argument string parsing logic from what to do with an error, and this seemed like the least-bad way to do that. Maybe it's moot and I should just exit from inside trapModeFromString. Thoughts?

[dschuff]

I think if trapModeFromString were directly in the top-level tool file such as s2wasm.cpp then it would be best to have it directly exit but otherwise not so great. I'm not a huge fan of an invalid state in a general-purpose enum like TrapMode but I like it better than, say, alternative kinds of returns in trapModeFromString (at least in the absence of helpers like std::optional or ErrorOr/Expected etc).

[jgravelle-google]

Pretty much. It doesn't sit well with me because it's adding to domain-logic complexity to avoid implementational coupling.
Although one minor upside is any zero-initialized TrapMode fields will be Invalid unless initialized, which counts for something? calloc users rejoice?

But yeah, optional is what I really want here. Or something like Haskell's Either. Still this isn't too bad.

[dschuff]

Actually maybe the best option is just to throw an exception. Since we already use exceptions in Binaryen, which I seem to always forget.

[kripken]

oh, is this keeping the old flags for compatibility?

[jgravelle-google]

Yeah, should I not? And if so, should I remove --imprecise as well?
Actually if we keep it I should update --imprecise to say "old name for --trap-mode=allow" now, to keep that consistent.

[dschuff]

I think we should consider asm2wasm as an implementation detail of emscripten, and aggressively eliminate legacy compatibility things.

[kripken]

Yeah, I think these are internal implementation details, just between emcc and asm2wasm. So we can remove old stuff.

But we will need to land this and the emscripten PRs at the same time, and make sure the emscripten one updates the binaryen tag to a new tag on binaryen after that merge, so it uses the new code immediately.