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

[wasm32] Add an intrinsic for the throw instruction #1542

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

coolreader18
Copy link
Contributor

For use in rust-lang/rust#121438. I was conservative in the wording of the docs cause I feel like this is the right place to put the intrinsic, but it likely won't be stabilized (at least in its current state).

@rustbot
Copy link
Collaborator

rustbot commented Feb 28, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks.

@coolreader18
Copy link
Contributor Author

coolreader18 commented Feb 28, 2024

   Compiling stdarch-test v0.1.0 (/checkout/crates/stdarch-test)
LLVM ERROR: undefined tag symbol cannot be weak
error: could not compile `core_arch` (lib test)

Ohh. Fair. I guess the other PR has to come first.

@coolreader18
Copy link
Contributor Author

Ah. The test is failing because wasmtime doesn't have exception support yet :/

@coolreader18
Copy link
Contributor Author

coolreader18 commented Mar 12, 2024

I guess I missed that testing locally, somehow.

Could run it in nodejs, I guess? Not really sure.

@coolreader18
Copy link
Contributor Author

coolreader18 commented Mar 13, 2024

@Amanieu what are your thoughts on just not having a assert_instr test for this?

@Amanieu
Copy link
Member

Amanieu commented Mar 13, 2024

That's probably fine for now, but should be addressed as soon as it can be tested in CI.

@Amanieu
Copy link
Member

Amanieu commented Mar 13, 2024

LGTM, now you just need to create a tracking issue in rust-lang/rust for this feature.

@coolreader18
Copy link
Contributor Author

coolreader18 commented Mar 14, 2024

I was thinking of having it just be perma-unstable for the foreseeable future - the llvm intrinsic is likely to change, and I don't see it being super useful at least until panic=unwind via exception-handling is stable (+ llvm support for non-cpp exception tags). Atm it's more of just an implementation detail for libpanic_unwind to use. Maybe that doesn't discount that it should have a tracking issue, but idk.

@coolreader18
Copy link
Contributor Author

Eh whatever, I'll make an issue.

@Amanieu Amanieu merged commit 81b846a into rust-lang:master Mar 19, 2024
27 checks passed
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.

None yet

3 participants