Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay contract checks until after all functions are defined #1619

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

badboy
Copy link
Member

@badboy badboy commented Jun 21, 2023

Otherwise we might call it with the wrong ABI.
This so far wasn't an issue on platforms UniFFI tests on, but can be observed on Windows (at leats in Wine), where suddenly the checksums won't match, e.g.
39784 is expected, but the call gets 33135464 back. The former is 0x9b68 and fits well within a 16-bit unsigned integer. The latter is 0x1f99b68 which does not fit (but it has the same suffix, guess why...)

Otherwise we might call it with the wrong ABI.
This so far wasn't an issue on platforms UniFFI tests on,
but can be observed on Windows (at leats in Wine),
where suddenly the checksums won't match, e.g.
39784 is expected, but the call gets 33135464 back.
The former is 0x9b68 and fits well within a 16-bit unsigned integer.
The latter is 0x1f99b68 which does not fit (but it has the same suffix,
guess why...)
@badboy badboy added the bug Something isn't working label Jun 21, 2023
@badboy badboy requested a review from a team as a code owner June 21, 2023 13:23
@badboy badboy requested review from skhamis and removed request for a team June 21, 2023 13:23
Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

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

Sorry for this one, it's like the endian issue all over again. The fix looks good though.

@badboy badboy merged commit 6938c26 into main Jun 22, 2023
@badboy badboy deleted the delay-contract-checks branch June 22, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants