8000 Add `assert_zst` intrinsic for use in `mem::conjure_zst` function by clarfonthey · Pull Request #147287 · rust-lang/rust · GitHub
[go: up one dir, main page]
Skip to content

Add assert_zst intrinsic for use in mem::conjure_zst function #147287

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add assert_zst intrinsic for use in mem::conjure_zst function #147287

wants to merge 1 commit into from

Conversation

clarfonthey
Copy link
Contributor
@clarfonthey clarfonthey commented Oct 3, 2025 •
edited
Loading

Implementation as I described in this comment: #146479 (comment)

Which states that this should be a hard error for concrete types, but not for generic types. Since the compiler already has multiple assertions like this, this should be reasonable.

Note that mem::conjure_zst is being written concurrently in #146479, but since this is just adding an intrinsic, it should be fine to merge independently. The tracking issue for the function is #95383.

Notes for review:

  1. This technically has overlap with assert_inhabited since this does that as well, although it feels better to just do both the inhabitedness and size check under the same intrinsic rather than requiring both to be called, since the codegen will be a bit easier that way. (only one layout query, instead of two)
  2. I specifically searched for assert_inhabited in the code to determine what would need to be modified. I'm not sure if the string-based checks under miri a 8000 nd rust-analyzer need to be modified right now, but since this PR will ping both those teams, I'm sure they will tell me whether I should keep it or remove it if it matters.
  3. Even though these intrinsics just assert things, the errors returned are based upon what they're going to be used for, so, with that in mind, I chose to use the same language as conjure_zst in the error message (attempted to conjure type {} when it is not zero-sized). If the name gets changed before stabilisation, we probably want to change the error message here too.
  4. This still does not fully satisfy the libs-team consensus in Add an unsafe fn for creating a ZST libs-team#292 (comment) because this just compiles into a panic, but the lint works differently. So, this will require an additional follow-up PR modifying the invalid_value lint to also trigger in this case.

cc @scottmcm who proposed the original conjure_zst intrinsic; you can decide whether you want to actually do the review for this or not.

@rustbot
Copy link
Collaborator
rustbot commented Oct 3, 2025

⚠️ #[rustc_intrinsic_const_stable_indirect] controls whether intrinsics can be exposed to stable const
code; adding it needs t-lang approval.

cc @rust-lang/wg-const-eval

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

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

cc @rust-lang/rust-analyzer

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

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@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 Oct 3, 2025
@rustbot rustbot added the T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. label Oct 3, 2025
@rustbot
Copy link
Collaborator
rustbot commented Oct 3, 2025

r? @fee1-dead

rust 8000 bot has assigned @fee1-dead.
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

@clarfonthey
Copy link
Contributor Author
clarfonthey commented Oct 3, 2025 •
edited
Loading
This is me reading correctly, then failing to read, then reading correctly again. I think we can just modify the lint and use `assert_inhabited` though.

So, after going through this entire route and pinging everyone, I'm not actually sure this satisfies the requirements set out by the libs team!

And I guess I'm going to close this, but include my analysis here:

  1. (update: apparently, I can't read, this is being used)It appears that at least one of these assertion intrinsics, `assert_uninit_valid`, is no longer used, despite still being in the standard library. `assert_zero_valid` still *is* used, but only in `mem::zeroed`, not anywhere in `MaybeUninit`. So, really, `assert_inhabited` by itself is the only one of these that's being used.
  2. Most of this seems to be covered by the invalid_value lint instead, which specifically checks the type passed to MaybeUninit::assume_init and offers a warn-by-default lint there. Since mem::zeroed also calls this, all this does is add an additional unconditional panic to mem::zeroed which is not present in assume_init.
  3. As I found in the code I modified, a lot of this code seems to have this notion of "lax" versus "strict" validation which applies to all the checks except… you guessed it, assert_inhabited. So, there's a lot of cruft here that appears like it can just be removed.

For now, I'm going to close this and keep more discussion in the conjure_zst tracking issue.

@clarfonthey clarfonthey closed this Oct 3, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2025
@clarfonthey clarfonthey reopened this Oct 3, 2025
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2025
@clarfonthey clarfonthey closed this Oct 3, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2025
@clarfonthey clarfonthey mentioned this pull request Oct 3, 2025
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@fee1-dead fee1-dead

Labels

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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@clarfonthey @rustbot @fee1-dead
0