Skip to content

Conversation

@squk
Copy link
Contributor

@squk squk commented Oct 18, 2024

The default wasm-rt implementation defines wasm_rt_memcpy as memcpy which expects void*

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Does the old output code even compile?

@keithw
Copy link
Member

keithw commented Oct 18, 2024

I'm a little confused -- we pass the tail-call tests, so... this suggests we need some additional testing here. How is this code-path not hit by the current tests?

@keithw
Copy link
Member

keithw commented Nov 7, 2024

I guess there are no spec tests for tail-calls that use multi-value? If so that seems like an omission we should fix. @squk , are you up for either:

  • adding a test to this repo (of using tail-call with multi-value) that this PR will make green, and/or
  • submitting one upstream to the tail-call proposal repo?

If you don't have cycles for this or don't want to, I think we could probably take it from here too, but I'd love to have a test somewhere.

@squk
Copy link
Contributor Author

squk commented Feb 24, 2025

@keithw I don't think I have the cycles for these tests unfortunately. We're not enabling this optimization for a while it seems

@squk squk closed this Feb 24, 2025
@keithw
Copy link
Member

keithw commented Feb 24, 2025

We definitely want this fix and thank you for finding and fixing the bug! If you don't have the cycles to improve the testing, as I said we can probably take it from here.

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.

4 participants