Support binaryen version_121

[SleeplessOne1917]

Closes #166.

Besides updating the project to be compatible with binaryen version_121, I made a few other changes:

• Use derive feature for strum instead of separate dependency on strum_macros.
• Use serialize-all for enums instead of manually adding a serialize attribute to every variant
• Remove Pass::name and instead use IntoStaticStr to generate static strings for enum variants.
• Update third party dependencies of each component crate.
• Add fields and functions related to PassOptions generateStackIR and optimizeStackIR fields (these are present in binaryen version_121 but did not exist yet in version_116).
• Add binding for containsPass (present in binaryen version_121 but did not exist yet in version_116).
• Apply changes suggested by running cargo clippy.

I'm opening this as a draft since I'm not really familiar with binaryen or the WASM spec more generally. I made this PR by referring to previous commits in this repository adding support for newer binaryen versions.

@brson @Aimeedeer pinging you two to make sure you see this since this repo hasn't received any new commits on the master branch for close to a year and you are the primary contributors.

[SleeplessOne1917]

This is needed to compile. Not sure if I should move this up a few lines so it's next to where src, tools, and output are added to the exports.

[SleeplessOne1917]

I made it so all of the cpp files from binaryen are added in the build, but I'm not sure if every single one is needed.

[SleeplessOne1917]

I saw a comment on the issue about issues with Windows paths saying this is fixed in binaryen now, but I'm not sure if any more work is needed here or if this repo gets it "for free".

[SleeplessOne1917]

I figured this warning would be more annoying than helpful since the new method on the types it was complaining about don't take args.

[SleeplessOne1917]

As stated in the TODO, this is gated behind conditional compilation in the C++ code and I'm not sure how to handle that on the Rust end. Would that just be adding a new feature and gating this enum variant behind a cfg attribute?

[SleeplessOne1917]

The result being tested in the assert no longer errors. I don't know if this is because I made a mistake doing the upgrade, or if it's due to a change in the unerlying C++ code in the new version.

Comparing PassRunner::run between version_116 and version_121, there is only one difference: the former does an assert at the start of the function and the latter doesn't.

[tysen]

Thanks a lot for this work @SleeplessOne1917 - here's hoping we can get this merged.

[tysen]

I see that a test failed during the CI build, but I ran it locally and it was all green.

[brson]

Thanks for doing all this legwork @SleeplessOne1917.

Pass arguments

The most difficult failure here looks to be related to binaryen "pass arguments". This causes the failures in pass_arg works and wasm_to_wasm_pass_arg. The latter is in the conformance tests, which can be run with

cargo test --manifest-path=components/conformance-tests/Cargo.toml

Some of the problems here are in the integration module, which is used to model the wasm-opt CLI on top of the wasm-opt-rs API. The integration module is perhaps not very important and could be left broken, except that it is used by conformance-tests, which needs to keep working.

The pass args are kind of an obscure and underdocumented feature of binaryen. We could just break them, but I think this can be fixed.

In the broken tests, this problem arises when using the extract-function pass, which has a required argument naming the function to extract.

On the command line, this used to look like

wasm-opt --extract-function --pass-arg extract-function@rust_begin_unwind

Now it appears to only work as

wasm-opt --extract-function=rust_begin_unwind

Internally to binaryen the Pass object has an optional string argument, which the bindings never set.

It looks like to me like the fix is:

• adjust the PassRunner shims to add a addWithArgument method that accepts the argument as the second string, calls the inner.add method with it.
• add the appropirate binding
• adjust the Passes::more_passes vec to be (I guess) Vec<(Pass, Option<String>)>. Kinda ugly.
• adjust create_and_run_pass_runner to handle the new pass-with-argument case
• modify the integration::parse_command_args parser to handle the --pass-name=value syntax, then apply it to the new api. there's an existing todo for this syntax and I guess it previously could be worked around. the existing parser for the extract-function@rust_begin_unwind syntax is probably irrelevant now and may need to be updated for current usage of the --pass-arg flag, which looks like it may no longer accept the k@v syntax.
• fix the two tests

I will hack on this myself shortly unless someone else really wants to tackle it, but I don't expect anything more after waiting so long here.

---

I haven't looked at the failure in optimization_read_module_error_works but this test seems like it should still be valid.

---

Almost all the conformance tests are failing. This indicates that the API is not configuring binaryen the same way as wasm-opt is. I have not looked into it yet.

These are run with

cargo test --manifest-path=components/conformance-tests/Cargo.toml

The initial run takes a long time because it is rebuilding the c++ wasm-opt to compare against.

[SleeplessOne1917]

@brson I made your suggested changes except for fixing the tests. The conformance tests are now failing with a different issue and I'm not sure how to advance. Most of the conformance tests are failing when the API outfile is compared with the Rust out file. All the test output says is that one massive byte vector was produced when it should have been another massive byte vector and I can't see anything that indicates what the actual issue is.

[HerrMuellerluedenscheid]

Is this ready to be merged? binaryen 116 does not provide a binary for linux arm64 which was added with 117 :(
So, would be awesome to have this merged

[SleeplessOne1917]

Besdies the CI build failing, we'll need to address the points I made in the comments I haven't marked as resolved already before merging. I'm hoping this can be merged sooner rather than later since cargo-leptos uses this library and I use Leptos.