make std::intrinsics functions actually be intrinsics #139916

RalfJung · 2025-04-16T13:03:14Z

Most of the functions in std::intrinsics are actually intrinsics, but some are not: for historical reasons, std::intrinsics::{copy,copy_nonoverlapping,write_bytes} are accessible on stable, and the versions in std::ptr are just re-exports. These functions are not intrinsics, but wrappers around the intrinsic, because they add extra debug assertions.

This PR makes the functions in std::intrinsics actually be intrinsics.

The advantage is that we can now use it in tests that need to directly call the intrinsic, thus removing a footgun for compiler development. We also remove the extended user-facing doc comments of these functions out of a file that should be largely internal documentation.
The downside is that if users are using those functions directly, they will not get the debug assertions any more. Note however that those users are already ignoring a deprecation warning, so I think this is fine. Furthermore, if someone imports the intrinsic name of this function and turns that into a function pointer, that will no longer work, since only the wrapper functions can be turned into a function pointer. I would be rather surprised if anyone did this, though... and again, they must have already ignored a deprecation warning. Still, seems worth a crater run, if there's general agreement that we want to go ahead with this change.

(intrinsics::drop_in_place also remains not-an-intrinsic, which bugs me, but oh well, not much we can do about it; we can't remove it from the module as the path is accidentally-stable.)

Cc @rust-lang/libs-api @saethlin

rustbot · 2025-04-16T13:03:19Z

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 · 2025-04-16T13:03:22Z

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

The Miri subtree was changed

cc @rust-lang/miri

jhpratt · 2025-04-21T01:18:16Z

(intrinsics::drop_in_place also remains not-an-intrinsic, which bugs me, but oh well, not much we can do about it; we can't remove it from the module as the path is accidentally-stable.)

I'd be interested in seeing a crater run on that, to be honest. We've pulled the rug on things that were accidentally stabilized before, even long after the fact.

Mark-Simulacrum

r=me with test confirmed to be adjusted as expected

Mark-Simulacrum · 2025-04-27T19:27:29Z

tests/mir-opt/lower_intrinsics.f_copy_nonoverlapping.LowerIntrinsics.panic-abort.diff

@@ -45,7 +45,7 @@
          _9 = copy _10;
          _8 = move _9 as *mut i32 (PtrToPtr);
          StorageDead(_9);
-         _3 = copy_nonoverlapping::<i32>(move _4, move _8, const 0_usize) -> [return: bb1, unwind unreachable];
+-         _3 = std::intrinsics::copy_nonoverlapping::<i32>(move _4, move _8, const 0_usize) -> [return: bb1, unwind unreachable];
 +         copy_nonoverlapping(dst = move _8, src = move _4, count = const 0_usize);


I'm confused why this changed - maybe I'm missing something but the test looks like it's supposed to already call the intrinsic? Is this change ok?

Note that only the "old" line changed -- it is now using std::intrinsics::copy_nonoverlapping rather than the local copy_nonoverlapping, which is expected as that is exactly what changed in the rs file.

The "new" line, in both cases, is the MIR statement form of copy_nonoverlapping.

Oh, right, this is a diff of a diff. Okay, makes sense.

RalfJung · 2025-04-28T06:48:43Z

Since this is a slight breaking change (of accidentally-stable API surface), does it need an FCP of some sort -- from @rust-lang/libs-api, I assume?

Amanieu · 2025-04-29T15:43:18Z

FCP since this is very slightly breaking, though unlikely to cause any issues in practice.

@rfcbot merge

rfcbot · 2025-04-29T15:43:20Z

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

RalfJung · 2025-04-29T15:44:48Z

@bors try

make std::intrinsic functions actually be intrinsics Most of the functions in `std::intrinsics` are actually intrinsics, but some are not: for historical reasons, `std::intrinsics::{copy,copy_nonoverlapping,write_bytes}` are accessible on stable, and the versions in `std::ptr` are just re-exports. These functions are not intrinsics, but wrappers around the intrinsic, because they add extra debug assertions. This PR makes the functions in `std::intrinsics` actually be intrinsics. - The advantage is that we can now use it in tests that need to directly call the intrinsic, thus removing a footgun for compiler development. We also remove the extended user-facing doc comments of these functions out of a file that should be largely internal documentation. - The downside is that if users are using those functions directly, they will not get the debug assertions any more. Note however that those users are already ignoring a deprecation warning, so I think this is fine. Furthermore, if someone imports the `intrinsic` name of this function and turns that into a function pointer, that will no longer work, since only the wrapper functions can be turned into a function pointer. I would be rather surprised if anyone did this, though... and again, they must have already ignored a deprecation warning. Still, seems worth a crater run, if there's general agreement that we want to go ahead with this change. (`intrinsics::drop_in_place` also remains not-an-intrinsic, which bugs me, but oh well, not much we can do about it; we can't remove it from the module as the path is accidentally-stable.) Cc `@rust-lang/libs-api` `@saethlin`

bors · 2025-04-29T15:46:03Z

⌛ Trying commit 5d60226 with merge b8c67c6...

bors · 2025-04-29T17:50:55Z

☀️ Try build successful - checks-actions
Build commit: b8c67c6 (b8c67c6e3233dac3bcd238e82e72152bf538d0fa)

RalfJung · 2025-04-29T18:05:04Z

@craterbot check

craterbot · 2025-04-29T18:05:18Z

👌 Experiment pr-139916 created and queued.
🤖 Automatically detected try build b8c67c6
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-04-30T07:30:44Z

🚧 Experiment pr-139916 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-05-01T10:16:05Z

🎉 Experiment pr-139916 is completed!
📊 6 regressed and 2 fixed (622899 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

RalfJung · 2025-05-01T12:25:30Z

These all look spurious :)

scottmcm · 2025-05-01T17:51:38Z

Big fan of letting tests just use the versions from core 👍

rustbot assigned Mark-Simulacrum Apr 16, 2025

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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 16, 2025

This comment has been minimized.

Sign in to view

RalfJung force-pushed the intrinsic-wrappers branch from 0497c07 to 82d5e7a Compare April 16, 2025 13:39

This comment has been minimized.

Sign in to view

RalfJung force-pushed the intrinsic-wrappers branch from 82d5e7a to 5a7d62b Compare April 16, 2025 14:26

This comment has been minimized.

Sign in to view

make std::intrinsic functions actually be intrinsics

5d60226

RalfJung force-pushed the intrinsic-wrappers branch from 5a7d62b to 5d60226 Compare April 16, 2025 15:01

Mark-Simulacrum reviewed Apr 27, 2025

View reviewed changes

RalfJung added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Apr 28, 2025

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 29, 2025

RalfJung added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 29, 2025

rustbot mentioned this pull request Apr 29, 2025

Tracking issue for release notes of #139916: make std::intrinsic functions actually be intrinsics #140472

Open

RalfJung changed the title ~~make std::intrinsic functions actually be intrinsics~~ make std::intrinsics functions actually be intrinsics Apr 29, 2025

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2025

RalfJung added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Apr 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make std::intrinsics functions actually be intrinsics #139916

make std::intrinsics functions actually be intrinsics #139916

RalfJung commented Apr 16, 2025 •

edited

Loading

rustbot commented Apr 16, 2025

rustbot commented Apr 16, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jhpratt commented Apr 21, 2025

Mark-Simulacrum left a comment

Mark-Simulacrum Apr 27, 2025

RalfJung Apr 28, 2025 •

edited

Loading

Mark-Simulacrum Apr 28, 2025

RalfJung commented Apr 28, 2025 •

edited

Loading

Amanieu commented Apr 29, 2025

rfcbot commented Apr 29, 2025 •

edited by dtolnay

Loading

RalfJung commented Apr 29, 2025

bors commented Apr 29, 2025

bors commented Apr 29, 2025

RalfJung commented Apr 29, 2025

craterbot commented Apr 29, 2025

craterbot commented Apr 30, 2025

craterbot commented May 1, 2025

RalfJung commented May 1, 2025

scottmcm commented May 1, 2025

make std::intrinsics functions actually be intrinsics #139916

Are you sure you want to change the base?

make std::intrinsics functions actually be intrinsics #139916

Conversation

RalfJung commented Apr 16, 2025 • edited Loading

rustbot commented Apr 16, 2025

rustbot commented Apr 16, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jhpratt commented Apr 21, 2025

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Mark-Simulacrum Apr 27, 2025

Choose a reason for hiding this comment

RalfJung Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

Mark-Simulacrum Apr 28, 2025

Choose a reason for hiding this comment

RalfJung commented Apr 28, 2025 • edited Loading

Amanieu commented Apr 29, 2025

rfcbot commented Apr 29, 2025 • edited by dtolnay Loading

RalfJung commented Apr 29, 2025

bors commented Apr 29, 2025

bors commented Apr 29, 2025

RalfJung commented Apr 29, 2025

craterbot commented Apr 29, 2025

craterbot commented Apr 30, 2025

craterbot commented May 1, 2025

RalfJung commented May 1, 2025

scottmcm commented May 1, 2025

RalfJung commented Apr 16, 2025 •

edited

Loading

RalfJung Apr 28, 2025 •

edited

Loading

RalfJung commented Apr 28, 2025 •

edited

Loading

rfcbot commented Apr 29, 2025 •

edited by dtolnay

Loading