Skip to content

fix signed 8- and 16-bit loads in wit-dylib bindgen#2455

Merged
dicej merged 2 commits intobytecodealliance:mainfrom
dicej:fix-wit-dylib-signed-loads
Mar 5, 2026
Merged

fix signed 8- and 16-bit loads in wit-dylib bindgen#2455
dicej merged 2 commits intobytecodealliance:mainfrom
dicej:fix-wit-dylib-signed-loads

Conversation

@dicej
Copy link
Collaborator

@dicej dicej commented Mar 5, 2026

No description provided.

@dicej dicej requested a review from a team as a code owner March 5, 2026 21:00
@dicej dicej requested review from fitzgen and removed request for a team March 5, 2026 21:00
@dicej
Copy link
Collaborator Author

dicej commented Mar 5, 2026

I couldn't find any existing wit-dylib tests besides wit-dylib-smoke.wit, so I'm not sure how to test this; open to suggestions.

@dicej dicej requested review from alexcrichton and removed request for fitzgen March 5, 2026 21:02
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Most tests live here, but those wouldn't work to test this actually given how this is purely ABI-related. Given that I think copying wit-dylib-smoke.wat is fine and it'd just be a manual inspection that *_s instructions show up

@dicej
Copy link
Collaborator Author

dicej commented Mar 5, 2026

Most tests live here, but those wouldn't work to test this actually given how this is purely ABI-related. Given that I think copying wit-dylib-smoke.wat is fine and it'd just be a manual inspection that *_s instructions show up

Oof, sorry I missed the tests and test-programs directories; I should have seen those.

Ideally, we'd model this as a runtime test which uses WAT to avoid
compiler-optimization-induced unpredictability, but that would be a lot of work,
so we settle for checking that the `wit-dylib` output does in fact use signed
loads when the WIT world requires them.  I've verified that these tests fail
without my earlier fix.
@dicej dicej enabled auto-merge March 5, 2026 22:14
@dicej dicej added this pull request to the merge queue Mar 5, 2026
Merged via the queue into bytecodealliance:main with commit 2b5c44d Mar 5, 2026
36 checks passed
@dicej dicej deleted the fix-wit-dylib-signed-loads branch March 5, 2026 22:31
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