search

rust-lang / stdarch

Rust's standard library vendor-specific APIs and run-time feature detection
https://doc.rust-lang.org/stable/core/arch/
Apache License 2.0
601 stars 267 forks source link

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

Closed coolreader18 closed 6 months ago

coolreader18 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago 
   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 commented 7 months ago

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

coolreader18 commented 7 months ago

I guess I missed that testing locally, somehow.

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

coolreader18 commented 6 months ago

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

Amanieu commented 6 months ago

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

Amanieu commented 6 months ago

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

coolreader18 commented 6 months ago

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 commented 6 months ago

Eh whatever, I'll make an issue.