Constify `copy` related functions by usbalbin · Pull Request #83091 · rust-lang/rust

usbalbin · 2021-03-13T18:49:19Z

Constify

*const T::copy_to[_nonoverlapping]
*mut T::copy_to[_nonoverlapping]
*mut T::copy_from[_nonoverlapping]
mem::transmute_copy
mem::swap
ptr::swap[_nonoverlapping]
mem::replace
ptr::replace

rust-highfive · 2021-03-13T18:49:22Z

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

usbalbin · 2021-03-13T18:57:02Z

What do we want to call the feature gate for copy_to and copy_from? Would it make sense to reuse const_intrinsic_copy since copy_[to/from] are not much more than wrappers for the functions behind that?

RalfJung · 2021-03-14T18:03:08Z

Cc @rust-lang/wg-const-eval

What do we want to call the feature gate for copy_to and copy_from? Would it make sense to reuse const_intrinsic_copy since copy_[to/from] are not much more than wrappers for the functions behind that?

Yeah that makes sense.

usbalbin · 2021-03-14T20:22:58Z

Do we want to keep these checks? The same checks were removed for ptr::copy_nonoverlapping when it was constified in #79684

For copy_nonoverlapping we ended up removing them entirely also for performance reasons. I am not sure if those apply here.

"Not sure" as in "Lets undo those changes for now so we can continue with the less controversial changes and maybe consider that for some other time"? Or just "Not sure" :)

"Not sure" as in "let's see what others think"... @oli-obk ?

I'm for removing the checks. I don't see why swap_nonoverlapping should have them while copy_nonoverlapping doesn't have them.

Yeah, that's fair.

usbalbin · 2021-03-15T16:28:24Z

I have removed the // FIXMEmentioned above and squashed some commits. Do you want me to squash all commits into one or keep it as it is, one per feature gate?

oli-obk · 2021-03-15T17:38:32Z

could you create and reference an issue for these?

oli-obk · 2021-03-15T17:39:48Z

Do you want me to squash all commits into one or keep it as it is, one per feature gate?

one commit per feature gate is great, thanks!

Once all unstable feature attributes have an issue instead of none, r=me

usbalbin · 2021-03-15T20:45:14Z

@oli-obk

Once all unstable feature attributes have an issue instead of none, r=me

Ok, so...

@bors r=oli-obk

...I hope that is right :)

bors · 2021-03-15T20:45:15Z

@usbalbin: 🔑 Insufficient privileges: Not in reviewers

joshtriplett · 2021-03-16T08:44:21Z

@bors r=oli-obk

bors · 2021-03-16T08:44:22Z

📌 Commit db9a53b has been approved by oli-obk

Rollup of 10 pull requests Successful merges: - rust-lang#81822 (Added `try_exists()` method to `std::path::Path`) - rust-lang#83072 (Update `Vec` docs) - rust-lang#83077 (rustdoc: reduce GC work during search) - rust-lang#83091 (Constify `copy` related functions) - rust-lang#83156 (Fall-back to sans-serif if Arial is not available) - rust-lang#83157 (No background for code in portability snippets) - rust-lang#83160 (Deprecate RustcEncodable and RustcDecodable.) - rust-lang#83162 (Specify *.woff2 files as binary) - rust-lang#83172 (More informative diagnotic from `x.py test` attempt atop beta checkout) - rust-lang#83196 (Use delay_span_bug instead of panic in layout_scalar_valid_range) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…Mark-Simulacrum Make copy/copy_nonoverlapping fn's again Make copy/copy_nonoverlapping fn's again, rather than intrinsics. This a short-term change to address issue rust-lang#84297. It effectively reverts PRs rust-lang#81167 rust-lang#81238 (and part of rust-lang#82967), rust-lang#83091, and parts of rust-lang#79684.

…r=jhpratt Fix missing const for inherent pointer `replace` methods `ptr::replace` (the free fn) is already const stable. However, there are inherent convenience methods on `*mut T` and `NonNull<T>`, allowing you to write eg. `unsafe { foo.replace(bar) }` where `foo` is `*mut T` or `NonNull<T>`. It seems const was never added to the inherent method (likely oversight), so this PR adds it. I don't believe this needs another[^1] FCP as the inherent methods are already stable and `ptr::replace` is already const stable, so this adds no new API. Original tracking issue: rust-lang#83164 `ptr::replace` constified in rust-lang#83091 `ptr::replace` const stabilized in rust-lang#130954 [^1]: `const_replace` FCP completed: rust-lang#83164 (comment)

Rollup merge of rust-lang#136877 - Sky9x:const-inherent-ptr-replace, r=jhpratt Fix missing const for inherent pointer `replace` methods `ptr::replace` (the free fn) is already const stable. However, there are inherent convenience methods on `*mut T` and `NonNull<T>`, allowing you to write eg. `unsafe { foo.replace(bar) }` where `foo` is `*mut T` or `NonNull<T>`. It seems const was never added to the inherent method (likely oversight), so this PR adds it. I don't believe this needs another[^1] FCP as the inherent methods are already stable and `ptr::replace` is already const stable, so this adds no new API. Original tracking issue: rust-lang#83164 `ptr::replace` constified in rust-lang#83091 `ptr::replace` const stabilized in rust-lang#130954 [^1]: `const_replace` FCP completed: rust-lang#83164 (comment)

…r=jhpratt Fix missing const for inherent pointer `replace` methods `ptr::replace` (the free fn) is already const stable. However, there are inherent convenience methods on `*mut T` and `NonNull<T>`, allowing you to write eg. `unsafe { foo.replace(bar) }` where `foo` is `*mut T` or `NonNull<T>`. It seems const was never added to the inherent method (likely oversight), so this PR adds it. I don't believe this needs another[^1] FCP as the inherent methods are already stable and `ptr::replace` is already const stable, so this adds no new API. Original tracking issue: rust-lang#83164 `ptr::replace` constified in rust-lang#83091 `ptr::replace` const stabilized in rust-lang#130954 [^1]: `const_replace` FCP completed: rust-lang#83164 (comment)

rust-highfive assigned KodrAus Mar 13, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 13, 2021

usbalbin mentioned this pull request Mar 13, 2021
Tracking Issue for const_ptr_read #80377
Closed
3 tasks

usbalbin commented Mar 14, 2021
View reviewed changes

oli-obk approved these changes Mar 15, 2021
View reviewed changes

usbalbin force-pushed the const_copy branch from 4835d4c to c29ed77Compare March 15, 2021 16:17

usbalbin marked this pull request as ready for review March 15, 2021 17:27

oli-obk reviewed Mar 15, 2021
View reviewed changes

Comment threadlibrary/core/src/ptr/mod.rs Outdated
oli-obkMar 15, 2021
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you create and reference an issue for these?

This was referenced Mar 15, 2021
Tracking Issue for const_swap #83163
Closed
Tracking Issue for const_replace #83164
Closed
Tracking Issue for const_transmute_copy #83165
Closed

usbalbin added 4 commits March 15, 2021 20:45

Constify copy_to and copy_from
62cf244

Constify mem::swap and ptr::swap[_nonoverlapping]
64e2248

Constify mem::replace and ptr::replace
45988ee

Constify mem::transmute_copy
db9a53b

usbalbin force-pushed the const_copy branch from c29ed77 to db9a53bCompare March 15, 2021 19:48

usbalbin mentioned this pull request Mar 15, 2021
Tracking Issue for const_intrinsic_copy #80697
Closed
7 tasks

usbalbin changed the title ~~constify copy_to and copy_from~~Constify copy related functionsMar 15, 2021

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 Mar 16, 2021

JohnTitor mentioned this pull request Mar 16, 2021
Rollup of 10 pull requests #83199
Merged

bors merged commit d3460cd into rust-lang:masterMar 16, 2021

rustbot added this to the 1.52.0 milestone Mar 16, 2021

pnkfelix mentioned this pull request Jun 4, 2021
Make copy/copy_nonoverlapping fn's again #86003
Merged

Sky9x mentioned this pull request Feb 11, 2025
Fix missing const for inherent pointer replace methods #136877
Merged

Uh oh!

Uh oh!

Conversation

usbalbin commented Mar 13, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Mar 13, 2021

Uh oh!

usbalbin commented Mar 13, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Mar 14, 2021

Uh oh!

usbalbinMar 14, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJungMar 14, 2021

Choose a reason for hiding this comment

Uh oh!

usbalbinMar 15, 2021

Choose a reason for hiding this comment

Uh oh!

RalfJungMar 15, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obkMar 15, 2021

Choose a reason for hiding this comment

Uh oh!

RalfJungMar 15, 2021

Choose a reason for hiding this comment

Uh oh!

usbalbin commented Mar 15, 2021

Uh oh!

oli-obkMar 15, 2021

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Mar 15, 2021

Uh oh!

usbalbin commented Mar 15, 2021

Uh oh!

bors commented Mar 15, 2021

Uh oh!

joshtriplett commented Mar 16, 2021

Uh oh!

bors commented Mar 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

usbalbin commented Mar 13, 2021•
edited
Loading

usbalbin commented Mar 13, 2021•
edited
Loading

usbalbinMar 14, 2021•
edited
Loading

RalfJungMar 15, 2021•
edited
Loading