wabt
wabt copied to clipboard
Always do a full roundtrip in run-roundtrip.py
Even when the result is to be printed rather than compared byte for byte with the first version its still good to process the resulting wat output file so that we know we can parse what we generate.
Case in point, this changed caused me to fix two latent bugs:
- We were not correctly parsing events with inline import/export.
- We were output element segment names even when bulk memory was not enabled (See #1651)
The fix for (2) is a little more involved that we might like since for the first time the wat writer needs to know what features are enabled.
Fixes: #1651
@binji .. do you think there is any way we can avoid having to pass features to the wat writer here?
@binji .. do you think there is any way we can avoid having to pass features to the wat writer here?
I guess the alternative would be instead to pass Features to GenerateNames and in that case avoid generating segment names at all?
@binji any objections to this approach?
I took the liberty of rebasing this on top of the current main branch, which caused it to start crashing on a bug that is fixed by #1889, which I also rebased on top of the current main branch and then rebased this PR on top of it.
@takikawa @sbc100 any objections to merging both of these in their present form?
Longer-term, it could be nice to try to enforcing roundtripping on every (valid?) module in the spec tests.
It doesn't look like #1889 will be merged soon, but it seems like it would still be nice to get this PR in after almost two years and benefit from the enhanced testing. How about if we disable the roundtrip/fold-function-references.txt test for now? (Thanks to this PR's full roundtripping, that test file starts exposing a bug that #1889 would fix.)
I just rebased this on the current main branch (and added a SKIP to that test file). @sbc100 Any objections to merging at this point?