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.

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

remove support for extern-block const intrinsics #132492

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 2, 2024

This converts all const-callable intrinsics into the "new" form of a regular fn with #[rustc_intrinsic] attribute. That simplifies some of the logic since those functions can be marked const fn like regular functions, so intrinsics no longer need a special case to be considered const-callable at all.

I also added a new attribute #[rustc_const_stable_intrinsic] to mark an intrinsic as being ready to be exposed on stable. Previously we used the #[rustc_const_stable_indirect] attribute for that, but that attribute had a dual role -- when used on a regular function, it is an entirely safe marker to make this function part of recursive const stability, but on an intrinsic it is a trusted marker requiring special care. It's not great for the same attribute to be sometimes fully checked and safe, and sometimes trusted and requiring special care, so I split this into two attributes.

This also fixes #122652 by accepting intrinsics as const-stable if they have a fallback body that is recursively const-stable.

The library changes are best reviewed with whitespace hidden.

r? @compiler-errors

@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 Nov 2, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the const-intrinsics branch 2 times, most recently from 223e7bb to 57ed415 Compare November 2, 2024 08:05
@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the const-intrinsics branch 2 times, most recently from 4565d04 to 7231200 Compare November 2, 2024 08:25
@rust-log-analyzer

This comment has been minimized.

});
}
Some(ConstStability { level: StabilityLevel::Stable { .. }, .. }) => {
// All good.
// All good. Note that a `#[rustc_const_stable]` intrinsic (meaning it
Copy link
Member

Choose a reason for hiding this comment

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

When would an intrinsic be marked as rustc_const_stable but not rustc_const_stable_intrinsic? I cannot seem to find any example in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

transmute is such an intrinsic. It seems silly to require both attributes.

@bors
Copy link
Contributor

bors commented Nov 4, 2024

☔ The latest upstream changes (presumably #132586) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Nov 4, 2024

☔ The latest upstream changes (presumably #132603) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors
Copy link
Member

r=me after rebase, sorry for taking a bit to review this lol

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2024
@RalfJung
Copy link
Member Author

RalfJung commented Nov 4, 2024

Review within 2.5 days, that's pretty fast I would say. :) Thanks!

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Nov 4, 2024

📌 Commit a741b33 has been approved by compiler-errors

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2024
…kingjubilee

Rollup of 11 pull requests

Successful merges:

 - rust-lang#131153 (Improve duplicate derive Copy/Clone diagnostics)
 - rust-lang#132025 (fix suggestion for diagnostic error E0027)
 - rust-lang#132303 (More tests for non-exhaustive C-like enums in FFI)
 - rust-lang#132492 (remove support for extern-block const intrinsics)
 - rust-lang#132587 (Revert "Avoid nested replacement ranges" from rust-lang#129346.)
 - rust-lang#132596 ([rustdoc] Fix `--show-coverage` when JSON output format is used)
 - rust-lang#132598 (Clippy: Move some attribute lints to be early pass (post expansion))
 - rust-lang#132601 (Update books)
 - rust-lang#132606 (Improve example of `impl Pattern for &[char]`)
 - rust-lang#132608 (document `type_implements_trait`)
 - rust-lang#132609 (docs: fix grammar in doc comment at unix/process.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 23ef001 into rust-lang:master Nov 5, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2024
Rollup merge of rust-lang#132492 - RalfJung:const-intrinsics, r=compiler-errors

remove support for extern-block const intrinsics

This converts all const-callable intrinsics into the "new" form of a regular `fn` with `#[rustc_intrinsic]` attribute. That simplifies some of the logic since those functions can be marked `const fn` like regular functions, so intrinsics no longer need a special case to be considered const-callable at all.

I also added a new attribute `#[rustc_const_stable_intrinsic]` to mark an intrinsic as being ready to be exposed on stable. Previously we used the `#[rustc_const_stable_indirect]` attribute for that, but that attribute had a dual role -- when used on a regular function, it is an entirely safe marker to make this function part of recursive const stability, but on an intrinsic it is a trusted marker requiring special care. It's not great for the same attribute to be sometimes fully checked and safe, and sometimes trusted and requiring special care, so I split this into two attributes.

This also fixes rust-lang#122652 by accepting intrinsics as const-stable if they have a fallback body that is recursively const-stable.

The library changes are best reviewed with whitespace hidden.

r? `@compiler-errors`
@RalfJung RalfJung deleted the const-intrinsics branch November 5, 2024 08:50
@scottmcm
Copy link
Member

scottmcm commented Nov 5, 2024

Very nice! Thanks, Ralf.

This'll make refactor-to-intrinsic so much nicer, not needing to add a scary "looks like it's making a stability promise" attribute to stuff that used to just be normal code.

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.

Disable const-stability checking for intrinsics with fallback MIR?
7 participants