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

Make core::mem::copy const #100006

Merged
merged 1 commit into from
Oct 30, 2022
Merged

Make core::mem::copy const #100006

merged 1 commit into from
Oct 30, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Aug 1, 2022 •
edited
Loading

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Aug 1, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 1, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

r? @kennytm

(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 Aug 1, 2022
@jyn514 jyn514 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 1, 2022
@thomcc
Copy link
Member

thomcc commented Aug 1, 2022

Reassigning to someone on t-libs-api -- @jyn514 asked me to review via discord, but it's an API change, so someone on libs-api should sign off on it.

I'm not sure if this needs ACP or not. On the one hand, it's very small, the change is small. On the other hand, it does repaint a bikeshed -- in that it changes the name (and to be totally honest, I'm not 100% convinced about the name change -- the rationale of distinguishing it from core::ptr::copy seems solid enough, but I'm not sure the past tense in core::mem::copied makes sense to me).

r? @joshtriplett

@rust-highfive rust-highfive assigned joshtriplett and unassigned kennytm Aug 1, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514 jyn514 mentioned this pull request Aug 1, 2022
Closed
@scottmcm
Copy link
Member

scottmcm commented Aug 1, 2022

Also, historically the -d naming was used for Iterator::cloned as distinct from Clone::clone, and thus also things like https://doc.rust-lang.org/std/option/enum.Option.html#method.copied. So if this is the "primary" copy method, it's not obvious to me that it should use past tense like this.

(TBH, copy feels to me like it wants to be a #[final] method in the Copy trait, not a standalone, but that's a whole different conversation.)

@thomcc
Copy link
Member

thomcc commented Aug 1, 2022

Oh, yeah, Copy::copy as a #[final] method seems exactly right (I had forgotten about these, since I don't think they exist yet).

@jyn514
Copy link
Member Author

jyn514 commented Aug 24, 2022

I am not sure the status of this PR. Should I wait for feedback from @joshtriplett that we want to make these changes?

@joshtriplett
Copy link
Member

I personally agree: I don't think that this should use the past-tense copied. I don't know what other libs-api folks think.

I'd be happy to merge a PR making the const change in the interim.

@jyn514
Copy link
Member Author

jyn514 commented Sep 14, 2022

Ok, I changed this so it only makes the function const.

@jyn514 jyn514 changed the title Update core::mem::copy Make core::mem::copy const Sep 14, 2022
@jyn514
Copy link
Member Author

jyn514 commented Oct 29, 2022

@joshtriplett do you know when you'll have a chance to look at this?

@dtolnay
Copy link
Member

dtolnay commented Oct 29, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 29, 2022

📌 Commit b5d5682 has been approved by dtolnay

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2022
@dtolnay dtolnay assigned dtolnay and unassigned joshtriplett Oct 29, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 29, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 30, 2022
@bors
Auto merge of rust-lang#103745 - matthiaskrgr:rollup-hipjva8, r=matth…
4827cee 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#100006 (Make `core::mem::copy` const)
 - rust-lang#102659 (1.65.0 release notes)
 - rust-lang#103124 (Add tests for autoderef on block tail)
 - rust-lang#103253 (rustdoc: add test case for masked blanket impl)
 - rust-lang#103715 (use consistent terminology)
 - rust-lang#103722 (Fix z-indexes of code example feature and cleanup its CSS)
 - rust-lang#103726 (Avoid unnecessary `&str` to `String` conversions)
 - rust-lang#103737 (rustdoc: use CSS margin/padding shorthand when all are being set)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 22e320b into rust-lang:master Oct 30, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 30, 2022
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.66.0, 1.67.0 Oct 30, 2022
@jyn514 jyn514 deleted the update-copy branch February 25, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.67.0
Development

Successfully merging this pull request may close these issues.