Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Fix error in type of memory.init introduced in #56#57

Merged
sbc100 merged 1 commit intomainfrom
memory_instrs_update
May 3, 2024
Merged

Fix error in type of memory.init introduced in #56#57
sbc100 merged 1 commit intomainfrom
memory_instrs_update

Conversation

@sbc100
Copy link
Member

@sbc100 sbc100 commented May 2, 2024

Sorry for the churn. This time I'll double check the test in wabt before updating the testsuite repo.

Sorry for the chrun.  This time I'll double check the test in wabt
before updating the testsuite repo.
@sbc100 sbc100 requested a review from dschuff May 2, 2024 23:55
@dschuff
Copy link
Member

dschuff commented May 3, 2024

Should the size and offset fields of memory.init not be the same size as pointers, a la size_t?

@sbc100
Copy link
Member Author

sbc100 commented May 3, 2024

Those arguments are supposed to stay as i32.. at least according the overview and current implementations.

@bvisness
Copy link
Collaborator

bvisness commented May 3, 2024

I think it makes sense for these parameters to remain i32 since they are determined by the data segment, which still doesn't use 64-bit indices. (Nor should it, probably.)

@sbc100 sbc100 merged commit 04620c2 into main May 3, 2024
@sbc100 sbc100 deleted the memory_instrs_update branch May 3, 2024 05:29
@dschuff
Copy link
Member

dschuff commented May 3, 2024

Ah, right. I guess that logic would make more sense for memory.copy than memory.init.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants