Skip to content

remove redundant unsafe - #163918

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
yilin0518:fix_arm_hint
Oct 11, 2026
Merged

rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
yilin0518:fix_arm_hint

Conversation

@yilin0518

@yilin0518 yilin0518 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Related Zulipchat topic link: Safety requirements of ARM hint APIs

This PR tries to remove the unsafe in core_arch/arm_shared/hint.rs. As the Zulip topic discussed, these APIs have #[cfg(any(...))] attribute to restrict that these can only be compiled when specific architecture is met. So there seems no need to mark these APIs unsafe.

For __nop, I found that in riscv_shared there exist the same nop instruction, but it is marked safe. The concrete path is core_arch/src/riscv_shared/mod.rs

/// Generates the `NOP` instruction
///
/// The NOP instruction does not change any architecturally visible state, except for
/// advancing the `pc` and incrementing any applicable performance counters.
#[inline]
#[unstable(feature = "riscv_ext_intrinsics", issue = "114544")]
pub fn nop() {
    unsafe {
        asm!("nop", options(nomem, nostack, preserves_flags));
    }
}

But there is a remaining question: For wfe and wfi, they can be configurable to trap. So I think if needed, I can complement some discription about trap. Hoping you can give me some suggestion.

@rustbot

rustbot commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 7, 2026
@rustbot

rustbot commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

r? @JohnTitor

rustbot has assigned @JohnTitor.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from JohnTitor, Mark-Simulacrum, clarfonthey

@rust-log-analyzer

This comment has been minimized.

@JohnTitor JohnTitor left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rust-bors

rust-bors Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 75b0be8 has been approved by JohnTitor

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Oct 11, 2026
rust-bors Bot pushed a commit that referenced this pull request Oct 11, 2026
remove redundant unsafe



Related Zulipchat topic link: [Safety requirements of ARM hint APIs](https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Safety.20requirements.20of.20ARM.20hint.20APIs/with/629609101)

This PR tries to remove the unsafe in `core_arch/arm_shared/hint.rs`. As the Zulip topic discussed, these APIs have `#[cfg(any(...))]` attribute to restrict that these can only be compiled when specific architecture is met. So there seems no need to mark these APIs unsafe.

For `__nop`, I found that in `riscv_shared` there exist the same `nop` instruction, but it is marked safe. The concrete path is `core_arch/src/riscv_shared/mod.rs`

```rust
/// Generates the `NOP` instruction
///
/// The NOP instruction does not change any architecturally visible state, except for
/// advancing the `pc` and incrementing any applicable performance counters.
#[inline]
#[unstable(feature = "riscv_ext_intrinsics", issue = "114544")]
pub fn nop() {
    unsafe {
        asm!("nop", options(nomem, nostack, preserves_flags));
    }
}
```

But there is a remaining question: For `wfe` and `wfi`, they can be configurable to trap. So I think if needed, I can complement some discription about trap. Hoping you can give me some suggestion.
@rust-bors

rust-bors Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

⌛ Testing commit 75b0be8 with merge c833618...

Workflow: https://github.com/rust-lang/rust/actions/runs/38145486415

@JonathanBrouwer

Copy link
Copy Markdown
Member

@bors yield
Yielding to enclosing rollup

@rust-bors

rust-bors Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Auto build was cancelled. Cancelled workflows:

The next pull request likely to be tested is #164159.

rust-bors Bot pushed a commit that referenced this pull request Oct 11, 2026
…uwer

Rollup of 3 pull requests

Successful merges:

 - #163918 (remove redundant unsafe)
 - #164047 (Remove indirection for `Expr` in `Body`)
 - #164154 (rustc-dev-guide subtree update)
rust-bors Bot pushed a commit that referenced this pull request Oct 11, 2026
Rollup merge of #163918 - yilin0518:fix_arm_hint, r=JohnTitor

remove redundant unsafe

Related Zulipchat topic link: [Safety requirements of ARM hint APIs](https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Safety.20requirements.20of.20ARM.20hint.20APIs/with/629609101)

This PR tries to remove the unsafe in `core_arch/arm_shared/hint.rs`. As the Zulip topic discussed, these APIs have `#[cfg(any(...))]` attribute to restrict that these can only be compiled when specific architecture is met. So there seems no need to mark these APIs unsafe.

For `__nop`, I found that in `riscv_shared` there exist the same `nop` instruction, but it is marked safe. The concrete path is `core_arch/src/riscv_shared/mod.rs`

```rust
/// Generates the `NOP` instruction
///
/// The NOP instruction does not change any architecturally visible state, except for
/// advancing the `pc` and incrementing any applicable performance counters.
#[inline]
#[unstable(feature = "riscv_ext_intrinsics", issue = "114544")]
pub fn nop() {
    unsafe {
        asm!("nop", options(nomem, nostack, preserves_flags));
    }
}
```

But there is a remaining question: For `wfe` and `wfi`, they can be configurable to trap. So I think if needed, I can complement some discription about trap. Hoping you can give me some suggestion.
@rust-bors
rust-bors Bot merged commit f3983ba into rust-lang:main Oct 11, 2026
14 of 15 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Oct 11, 2026
github-actions Bot pushed a commit to rust-lang/stdarch that referenced this pull request Oct 12, 2026
…uwer

Rollup of 3 pull requests

Successful merges:

 - rust-lang/rust#163918 (remove redundant unsafe)
 - rust-lang/rust#164047 (Remove indirection for `Expr` in `Body`)
 - rust-lang/rust#164154 (rustc-dev-guide subtree update)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants