Skip to content

make std::intrinsic functions actually be intrinsics #139916

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 16, 2025

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
Copy link
Collaborator

rustbot commented Apr 16, 2025

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 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
@rustbot
Copy link
Collaborator

rustbot commented Apr 16, 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

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

jhpratt commented Apr 21, 2025

(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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants