Always do a full roundtrip in run-roundtrip.py #1661
Conversation
|
@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 |
src/wat-writer.cc
Outdated
| // The first name we encounter here, pre-bulk-memory, was intended to refer to | ||
| // the table while segment names were not supported at all. For this reason | ||
| // we cannot emit a segment name here without bulk-memory enabled, otherwise | ||
| // the name will be assumed to be the ame of a table and parsing will fail. |
|
@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. |
keithw
force-pushed
the
roundtrip
branch
2 times, most recently
from
0c14b49
to
67e40ee
Compare
|
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 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? |
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: 1. We were not correctly parsing events with inline import/export. 2. 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
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:
was not enabled (See
wasm2wat --generate-namesproduces invalid names #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