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.

Sign up for GitHub

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

Jump to bottom

Use arch::wasm::unreachable instead of arch::wasm32::unreachable #122878

Closed
wants to merge 1 commit into from
Closed

Use arch::wasm::unreachable instead of arch::wasm32::unreachable #122878

wants to merge 1 commit into from

Conversation

newpavlov
Copy link
Contributor

Closes #122877

@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2024
@kadiwa4
Copy link
Contributor

kadiwa4 commented Mar 22, 2024

Either this or #122797 can be closed. But this solution looks more elegant.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 23, 2024

📌 Commit 963844b has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2024
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Mar 23, 2024
Merged
taiki-e
taiki-e reviewed Mar 23, 2024
View reviewed changes
library/unwind/src/wasm.rs
@@ -59,7 +59,7 @@ pub unsafe fn _Unwind_RaiseException(exception: *mut _Unwind_Exception) -> _Unwi
wasm_throw(0, exception.cast())
} else {
let _ = exception;
core::arch::wasm32::unreachable()
core::arch::wasm::unreachable()
Copy link
Member

Choose a reason for hiding this comment

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

core::arch::wasm requires the simd_wasm64 feature, but AFAIK it isn't set in this crate.

rust/library/unwind/src/lib.rs

Lines 1 to 13 in 963844b

#![no_std]
#![unstable(feature = "panic_unwind", issue = "32837")]
#![feature(link_cfg)]
#![feature(staged_api)]
#![feature(c_unwind)]
#![feature(strict_provenance)]
#![cfg_attr(not(target_env = "msvc"), feature(libc))]
#![cfg_attr(
all(target_family = "wasm", not(target_os = "emscripten")),
feature(link_llvm_intrinsics)
)]
#![allow(internal_features)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I will close this PR in favor of #122797 then. Though IMO it's weird that unreachable on WASM64 requires simd_wasm64 feature.

Copy link
Member

Choose a reason for hiding this comment

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

Is that a mistake in core::arch or does that intrinsic really require SIMD, but only on wasm64 and not wasm32?

Copy link
Member

Choose a reason for hiding this comment

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

@RalfJung I assume simd_wasm64 really just means "special architecture intrinsics and features for wasm64" because... people just see core::arch and their brain shuts off kinda. I mean, all the architectural extension features were named stdsimd once.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 23, 2024
@matthiaskrgr
Rollup merge of rust-lang#122878 - newpavlov:patch-2, r=Mark-Simulacrum
6ef7a67 
Use `arch::wasm::unreachable` instead of `arch::wasm32::unreachable`

Closes rust-lang#122877
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 23, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 23, 2024
@bors
Auto merge of rust-lang#122925 - matthiaskrgr:rollup-2afe4wp, r=matth…
2594896 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#116016 (Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition)
 - rust-lang#122460 (Rework rmake support library API)
 - rust-lang#122658 (ci: Build gccjit from a git archive)
 - rust-lang#122698 (Cancel `cargo update` job if there's no updates)
 - rust-lang#122878 (Use `arch::wasm::unreachable` instead of `arch::wasm32::unreachable`)
 - rust-lang#122915 (Delay a bug if no RPITITs were found)
 - rust-lang#122916 (docs(sync): normalize dot in fn summaries)
 - rust-lang#122921 (Enable more mir-opt tests in debug builds)
 - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.)

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee
Copy link
Member

taiki is correct.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 23, 2024
@newpavlov newpavlov closed this Mar 23, 2024
@newpavlov newpavlov deleted the patch-2 branch March 23, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@workingjubilee workingjubilee workingjubilee left review comments

@taiki-e taiki-e taiki-e left review comments

@RalfJung RalfJung RalfJung left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WASM64 compiltion failure on core::arch::wasm32::unreachable() in the unwind library
8 participants
@newpavlov @rustbot @kadiwa4 @Mark-Simulacrum @bors @workingjubilee @RalfJung @taiki-e