Revert revert of constness in #86003 by usbalbin · Pull Request #86295 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation37 Commits11 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Re-constify mem::swap, mem::replace, ptr::write which were marked as not const in #86003
Once the checks pass, this should solve #86236
(rust-highfive has picked a reviewer for you, use r? to override)
This comment has been minimized.
Would it make sense to make a tracking issue for const_ptr_write?
@pnkfelix any objections to this? I am not sure why these were de-constified to begin with, so just making sure I am not missing anything here. :)
This comment has been minimized.
This comment has been minimized.
@usbalbin we also lost src/test/ui/consts/copy-intrinsic.rs in #86003; could you add that back as well?
LGTM -- r=me, except that it'd be good to hear back from @pnkfelix. Let's give them a few more days.
r? @RalfJung
This comment has been minimized.
Sorry but I am not quite sure what to do with the errors (or rather lack of errors)
Are those things that are no longer checked? Should I remove those tests or somehow make sure the errors are detected again?
Something is strange, it looks like none of the errors showed up -- but the exit status is still 1...?
Ah, I think I know what happens: the errors now have the wrong span; they are shown with spans in the standard library.
To fix this, you should call the intrinsics directly in this test. You will probably need to add a extern "rust-intrinsic" block to import them.
So, something like this. But at least on the playground this doesn't change the spans that are shown...
Thanks :)
What should I do about
error: module has missing stability attribute
--> src/main.rs:2:1
?
Same error in your playground
Ah... well we'll need to add a stability attribute. This should do it:
#![stable(feature = "dummy", since = "1.0.0")]
So if you write code which calls unstable code than you have to be explicit about the stabilitiy of your code?
Again thanks :)
Would you like me to squash the two latest commits (2faa57a and d1ae4e2) onto one?
A chain of things happens here:
- To call
copy_nonoverlapping, it needs to beconst fn - To make an intrinsic
const fn, we need to addrustc_const_unstable - To be allowed to use that attribute we need to enable the
staged_apifeature - Once we enable that feature we need to give stability attributes to everything; the easiest thing is to make is all stable so that we don't have to enable feature gates to call our own stuff. (Maybe we don't actually need that, and
#![unstable(...)]would also work.
Would you like me to squash the two latest commits (2faa57a and d1ae4e2) onto one?
Seems fine to me to keep this separate.
@pnkfelix confirmed that as long as we don't touch copy/copy_nonoverlapping, we should be good. So, let's (re-)land this. Thanks @usbalbin :)
@bors r+
This comment has been minimized.
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
Looks like #86194 changed the output of some tests added here.
…opy and copy_nonoverlapping' 5f6016f
This comment has been minimized.
@pnkfelix confirmed that as long as we don't touch
copy/copy_nonoverlapping, we should be good. So, let's (re-)land this. Thanks @usbalbin :)
@bors r+
Thank you :)
📌 Commit 4aa1267 has been approved by RalfJung
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
Labels
This PR was explicitly merged by bors.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.