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.

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

CTFE: support assert_zero_valid and assert_uninit_valid #91323

Merged
merged 1 commit into from
Nov 30, 2021
Merged

CTFE: support assert_zero_valid and assert_uninit_valid #91323

merged 1 commit into from
Nov 30, 2021

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 28, 2021 •
edited
Loading

This ensures the implementation of all three type-based assert_ intrinsics remains consistent in Miri.

assert_inhabited recently got stabilized in #90896 (meaning stable const fn can call it), so do the same with these other intrinsics.

Cc @rust-lang/wg-const-eval

@rust-highfive
Copy link
Collaborator

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Nov 28, 2021

r? @oli-obk

r=me with the intrinsics made const-unstable.

If you want them to be stable immediately, then at least one caller should be stable, too and we should then nominate it for libs-api

@rust-highfive rust-highfive assigned oli-obk and unassigned jackh726 Nov 28, 2021
@RalfJung
Copy link
Member Author

If you want them to be stable immediately, then at least one caller should be stable, too

Oh, I did not know of this policy.
If they remain unstable they need a separate feature gate which seems kind of silly... const-"stability" of intrinsics is a somewhat strange concept anyway and I can't see a good reason for why assert_zero_valid should be unstable while assert_inhabited is stable. (I almost think of these 3 intrinsics as one intrinsic with a "mode" argument, except having them as separate intrinsics is a lot easier to implement.)

@oli-obk
Copy link
Contributor

oli-obk commented Nov 29, 2021

Just use none for the tracking issue an invent a feature gate name.

Const stabilizing any intrinsic needs lang team sign off as stated at the top of the doc. Even if it is trivial in this case, as we already have an intrinsic that is basically the same. Maybe someone from t-lang can just merge this without an fcp?

r? rust-lang/lang

@RalfJung
Copy link
Member Author

Well fair, I can also make them unstable.

(That "r?" does not seem to have done anything.)

@Mark-Simulacrum Mark-Simulacrum added the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 29, 2021
@Mark-Simulacrum
Copy link
Member

Nominating for T-lang discussion (which seems better than r?, regardless). But I'm not clear on the ask being presented, so it may be helpful for a more extensive summary to be put in the description.

It sounds like this is just intending to enable usage of these two intrinsics in const code, which on the surface seems pretty harmless, but if there are reasons to not to do this, it would be helpful to call those out explicitly.

@RalfJung
Copy link
Member Author

Ah this is not worth the bureaucracy.^^ I will just make them unstable. Having the implementations all in rustc (rather than spread across rustc and Miri) is still worth it.

@RalfJung RalfJung removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 29, 2021
@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Nov 29, 2021

📌 Commit f830abed669acd532c39c07ee2c002b3e45054d4 has been approved by oli-obk

@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 Nov 29, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fee1-dead
Copy link
Member

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 29, 2021
@fee1-dead
Copy link
Member

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Nov 29, 2021

📌 Commit 6c3c3e0 has been approved by oli-obk

@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 29, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 29, 2021
@matthiaskrgr
Rollup merge of rust-lang#91323 - RalfJung:assert-type, r=oli-obk
78041be 
CTFE: support assert_zero_valid and assert_uninit_valid

This ensures the implementation of all three type-based assert_ intrinsics remains consistent in Miri.

`assert_inhabited` recently got stabilized in rust-lang#90896 (meaning stable `const fn` can call it), so do the same with these other intrinsics.

Cc `@rust-lang/wg-const-eval`
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 29, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 30, 2021
@matthiaskrgr
Rollup merge of rust-lang#91323 - RalfJung:assert-type, r=oli-obk
ad898ec 
CTFE: support assert_zero_valid and assert_uninit_valid

This ensures the implementation of all three type-based assert_ intrinsics remains consistent in Miri.

`assert_inhabited` recently got stabilized in rust-lang#90896 (meaning stable `const fn` can call it), so do the same with these other intrinsics.

Cc ``@rust-lang/wg-const-eval``
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 30, 2021
Closed
@JohnTitor JohnTitor mentioned this pull request Nov 30, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2021
@bors
Auto merge of rust-lang#91388 - JohnTitor:rollup-640o1e5, r=JohnTitor
1c02878 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#91243 (Don't treat unnormalized function arguments as well-formed)
 - rust-lang#91250 (Refactor EmitterWriter::emit_suggestion_default )
 - rust-lang#91317 (tests: Ignore `test/debuginfo/rc_arc.rs` on windows-gnu)
 - rust-lang#91323 (CTFE: support assert_zero_valid and assert_uninit_valid)
 - rust-lang#91358 (Fix small typo)
 - rust-lang#91360 (:arrow_up: rust-analyzer)
 - rust-lang#91368 (Don't re-export `MirPass`)
 - rust-lang#91383 (Add `drop_while` as doc alias to `Iterator::skip_while`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a940c68 into rust-lang:master Nov 30, 2021
@rustbot rustbot added this to the 1.59.0 milestone Nov 30, 2021
@RalfJung RalfJung mentioned this pull request Nov 30, 2021
Merged
bors added a commit to rust-lang/miri that referenced this pull request Nov 30, 2021
@bors
Auto merge of #1929 - RalfJung:rustup, r=RalfJung
5e9cf62 
rustup

With rust-lang/rust#91323 landed, we can remove these intrinsics from Miri.
@RalfJung RalfJung deleted the assert-type branch December 2, 2021 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.
9 participants
@RalfJung @rust-highfive @oli-obk @Mark-Simulacrum @bors @rust-log-analyzer @fee1-dead @jackh726 @rustbot