Skip to content

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented Sep 23, 2025

So, in my defence, these were both separated out from the other intrinsics in the file and had a different safety comment in the stable versions, so, I didn't notice them before. But, in my offence, the entire reason I did the previous PR was because I was using them for SIMD intrinsic fallbacks, and fabs is needed for those too, so, I don't really have an excuse.

Extra follow-up to #146683.

r? @RalfJung who reviewed the previous one

These don't appear to be used anywhere outside of the standard locations, at least.

@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2025

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 23, 2025
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 23, 2025
/// [`f16::abs`](../../std/primitive.f16.html#method.abs)
#[rustc_nounwind]
#[rustc_intrinsic]
pub const unsafe fn fabsf16(x: f16) -> f16;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Side remark: fabsf16 and fabsf128 aren't used in their corresponding fn abs definitions, so, I'm assuming they're "not ready yet" in that sense. However, they can still be marked safe for consistency.

@RalfJung
Copy link
Member

r=me with that comment removed.

@clarfonthey
Copy link
Contributor Author

Comment is now fixed

@RalfJung
Copy link
Member

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Sep 24, 2025

📌 Commit e8a8e06 has been approved by RalfJung

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 Sep 24, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 24, 2025
…alfJung

Make missed precondition-free float intrinsics safe

So, in my defence, these were both separated out from the other intrinsics in the file *and* had a different safety comment in the stable versions, so, I didn't notice them before. But, in my offence, the entire reason I did the previous PR was because I was using them for SIMD intrinsic fallbacks, and `fabs` is needed for those too, so, I don't really have an excuse.

Extra follow-up to rust-lang#146683.

r? `@RalfJung` who reviewed the previous one

These don't appear to be used anywhere outside of the standard locations, at least.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 24, 2025
…alfJung

Make missed precondition-free float intrinsics safe

So, in my defence, these were both separated out from the other intrinsics in the file *and* had a different safety comment in the stable versions, so, I didn't notice them before. But, in my offence, the entire reason I did the previous PR was because I was using them for SIMD intrinsic fallbacks, and `fabs` is needed for those too, so, I don't really have an excuse.

Extra follow-up to rust-lang#146683.

r? ``@RalfJung`` who reviewed the previous one

These don't appear to be used anywhere outside of the standard locations, at least.
bors added a commit that referenced this pull request Sep 24, 2025
Rollup of 9 pull requests

Successful merges:

 - #146711 (fix 2 borrowck issues)
 - #146735 (unstably constify float mul_add methods)
 - #146857 (revert change removing `has_infer` check. Commit conservatively patch…)
 - #146897 (fix ICE in rustdoc::invalid_html_tags)
 - #146915 (Make missed precondition-free float intrinsics safe)
 - #146932 (Switch next-solver related rustc dependencies of r-a to crates.io ones)
 - #146959 (temporary-lifetime-extension-tuple-ctor.rs: make usable on all editions)
 - #146964 (library: std: sys: pal: uefi: Add some comments)
 - #146969 (const-eval: better wording for errors involving maybe-null pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 24, 2025
Rollup of 8 pull requests

Successful merges:

 - #146711 (fix 2 borrowck issues)
 - #146857 (revert change removing `has_infer` check. Commit conservatively patch…)
 - #146897 (fix ICE in rustdoc::invalid_html_tags)
 - #146915 (Make missed precondition-free float intrinsics safe)
 - #146932 (Switch next-solver related rustc dependencies of r-a to crates.io ones)
 - #146959 (temporary-lifetime-extension-tuple-ctor.rs: make usable on all editions)
 - #146964 (library: std: sys: pal: uefi: Add some comments)
 - #146969 (const-eval: better wording for errors involving maybe-null pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2320fc3 into rust-lang:master Sep 24, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 24, 2025
rust-timer added a commit that referenced this pull request Sep 24, 2025
Rollup merge of #146915 - clarfonthey:safe-intrinsics-2, r=RalfJung

Make missed precondition-free float intrinsics safe

So, in my defence, these were both separated out from the other intrinsics in the file *and* had a different safety comment in the stable versions, so, I didn't notice them before. But, in my offence, the entire reason I did the previous PR was because I was using them for SIMD intrinsic fallbacks, and `fabs` is needed for those too, so, I don't really have an excuse.

Extra follow-up to #146683.

r? ```@RalfJung``` who reviewed the previous one

These don't appear to be used anywhere outside of the standard locations, at least.
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Sep 25, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#146711 (fix 2 borrowck issues)
 - rust-lang/rust#146857 (revert change removing `has_infer` check. Commit conservatively patch…)
 - rust-lang/rust#146897 (fix ICE in rustdoc::invalid_html_tags)
 - rust-lang/rust#146915 (Make missed precondition-free float intrinsics safe)
 - rust-lang/rust#146932 (Switch next-solver related rustc dependencies of r-a to crates.io ones)
 - rust-lang/rust#146959 (temporary-lifetime-extension-tuple-ctor.rs: make usable on all editions)
 - rust-lang/rust#146964 (library: std: sys: pal: uefi: Add some comments)
 - rust-lang/rust#146969 (const-eval: better wording for errors involving maybe-null pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
@clarfonthey clarfonthey deleted the safe-intrinsics-2 branch September 26, 2025 00:08
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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