bytecodealliance / wasm-tools

CLI and Rust libraries for low-level manipulation of WebAssembly modules
Apache License 2.0
1.3k stars 234 forks source link

JsonFromWast issue with the latest testsuite #1771

Closed CharlieTap closed 2 weeks ago

CharlieTap commented 3 weeks ago

If you run jsonfromwast on the latest testsuite comments.wast it will generate:

and an entry in command.json

        {
            "type": "module",
            "line": 83,
            "filename": "comments.4.wat"
        },

I'm migrating over from WABT and I found there was never a module command previously that can take a wat file as input. WABTs output for this same scenario is the following:

         {
            "type": "module",
            "line": 0,
            "filename": "comments.4.wasm"
        },
alexcrichton commented 3 weeks ago

Thanks for the report! The test case in question here is this one which is using the module quote form of module definition. In general these tests are just tests for text parsers which you can safely ignore as a consumer of json-from-wast because you're not testing a text parser in that case.

The difference here I suspect is because the module is valid and wabt probably goes ahead and emits it as binary where wasm-tools is structured a bit differently and preserves the text form.

Is it easy enough to ignore the *.wat tests? Or would it be best to have a more closely matching output to wabt in this regard?

CharlieTap commented 3 weeks ago

Hey @alexcrichton, thanks for getting back so quickly! I did try that (ignoring files that end in .wat) but it fails because subsequent commands depend on that module which is failing to be instantiated. For example:

  {
            "type": "module",
            "line": 0,
            "filename": "comments.4.wasm"
        },
        {
            "type": "assert_return",
            "line": 104,
            "action": {
                "type": "invoke",
                "field": "f1",
                "args": []
            },
            "expected": [
                {
                    "type": "i32",
                    "value": "2"
                }
            ]
        },

For now I'm just ignoring comments.wast because despite this one test failing wasm-tools overall is better for my runtime/usecase

alexcrichton commented 3 weeks ago

Ah that's an excellent point! I'll try to work on generating a *.wasm when there's a *.wat as well. That way the JSON message can list both and the parser can pick whichever is appropriate.