Skip to content

Improve test coverage around data.drop#2176

Merged
rossberg merged 2 commits into
WebAssembly:mainfrom
bvisness:data-drop-bounds-checks
May 26, 2026
Merged

Improve test coverage around data.drop#2176
rossberg merged 2 commits into
WebAssembly:mainfrom
bvisness:data-drop-bounds-checks

Conversation

@bvisness
Copy link
Copy Markdown
Contributor

There were very few tests around data.drop, which can be strange to handle in engines due to bounds check behavior. In particular, this adds a test that would have caught a minor bounds check bug in SpiderMonkey.

There were very few tests around data.drop, which can be strange to
handle in engines due to bounds check behavior. In particular, this adds
a test that would have caught a minor bounds check bug in SpiderMonkey.
Copy link
Copy Markdown
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's good, thanks!

If you feel motivated, you could add the analogous tests for elem.drop. :)

@bvisness
Copy link
Copy Markdown
Contributor Author

Adding analogous tests for tables / element segments is a bit wacky because commit a21b0df ignored the big DO NOT EDIT at the top of table_init.wast. I had to rework the generator slightly to introduce that test there (which is fine; it's a good test).

As a warning, though, I have another bigger patch coming, because if you step back and look at a lot of what is being added in this patch, you will notice that it utterly fails to test table64, because almost nothing in that file actually touches table64 at all for some reason!! So this PR adds the correct shape of tests for "consistency" but that will be remedied in a much bigger and uglier follow-up.

@rossberg
Copy link
Copy Markdown
Member

Cool, thanks!

@rossberg rossberg merged commit a05043b into WebAssembly:main May 26, 2026
1 check passed
@bvisness bvisness deleted the data-drop-bounds-checks branch May 26, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants