Skip to content

/rust

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for ptr::swap_nonoverlapping (library feature swap_nonoverlapping) #42818

Closed
scottmcm opened this Issue Jun 22, 2017 · 4 comments

Comments

Assignees
No one assigned
Labels
B-unstable C-tracking-issue T-libs final-comment-period
Projects
None yet
Milestone
No milestone
4 participants
@scottmcm @SimonSapin @rfcbot @Mark-Simulacrum
@scottmcm
Copy link
Member

scottmcm commented Jun 22, 2017 •
edited

Proposed in PR #42819.

This is semantically the same as a loop over ptr::swap, but with manual vectorization (from PR #40454) to make it faster for cases (like T == String) where LLVM doesn't.

@scottmcm scottmcm referenced this issue Jun 22, 2017

Merged

Reuse the mem::swap optimizations to speed up slice::rotate #42819

@Mark-Simulacrum Mark-Simulacrum added B-unstable T-libs labels Jun 22, 2017

bors added a commit that referenced this issue Jun 26, 2017

@bors
Auto merge of #42819 - scottmcm:swap-nonoverlapping, r=sfackler 
Reuse the mem::swap optimizations to speed up slice::rotate

This is most helpful for compound types where LLVM didn't vectorize the loop.  Highlight: bench slice::rotate_medium_by727_strings gets 38% faster.

Exposes the swapping logic from PR #40454 as `pub unsafe fn ptr::swap_nonoverlapping` under library feature `swap_nonoverlapping` #42818.

(The new method seemed plausible, and was the simplest way to share the logic.  I'm not attached to it, though, so let me know if a different way would be better.)
Loading status checks…
240dcc4

bors added a commit that referenced this issue Jun 28, 2017

@bors
Auto merge of #42819 - scottmcm:swap-nonoverlapping, r=sfackler 
Reuse the mem::swap optimizations to speed up slice::rotate

This is most helpful for compound types where LLVM didn't vectorize the loop.  Highlight: bench slice::rotate_medium_by727_strings gets 38% faster.

Exposes the swapping logic from PR #40454 as `pub unsafe fn ptr::swap_nonoverlapping` under library feature `swap_nonoverlapping` #42818.

(The new method seemed plausible, and was the simplest way to share the logic.  I'm not attached to it, though, so let me know if a different way would be better.)
Loading status checks…
47faf1d

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue label Jul 22, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Contributor

SimonSapin commented Mar 17, 2018

In Nightly for 8 months, let’s stabilize.

@rfcbot fcp merge
@rfcbot

This comment has been minimized.

Sign in to view
Copy link

rfcbot commented Mar 17, 2018 •
edited by alexcrichton

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period final-comment-period and removed proposed-final-comment-period labels Mar 17, 2018

@rfcbot

This comment has been minimized.

Sign in to view
Copy link

rfcbot commented Mar 19, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔
@rfcbot

This comment has been minimized.

Sign in to view
Copy link

rfcbot commented Mar 29, 2018

The final comment period is now complete.

@ecstatic-morse ecstatic-morse referenced this issue Apr 7, 2018

Merged

Rewrite docs for `std::ptr` #49767

4 of 9 tasks complete

@tinaun tinaun referenced this issue Apr 17, 2018

Merged

stabilize a bunch of minor api additions #50017

bors added a commit that referenced this issue Apr 18, 2018

@bors
Auto merge of #50017 - tinaun:stabilize-all-the-things, r=sfackler 
stabilize a bunch of minor api additions

besides `ptr::NonNull::cast` (which is 4 days away from end of FCP) all of these have been finished with FCP for a few weeks now with minimal issues raised

* Closes #41020
* Closes #42818
* Closes #44030
* Closes #44400
* Closes #46507
* Closes #47653
* Closes #46344

the following functions will be stabilized in 1.27:
* `[T]::rsplit`
* `[T]::rsplit_mut`
* `[T]::swap_with_slice`
* `ptr::swap_nonoverlapping`
* `NonNull::cast`
* `Duration::from_micros`
* `Duration::from_nanos`
* `Duration::subsec_millis`
* `Duration::subsec_micros`
* `HashMap::remove_entry`
Loading status checks…
ac3c228

@bors bors closed this in #50017 Apr 18, 2018

kennytm added a commit to kennytm/rust that referenced this issue May 14, 2018

@kennytm
Rollup merge of rust-lang#49767 - ecstatic-morse:ptr-docs, r=stevekla… 
…bnik

Rewrite docs for `std::ptr`

This PR attempts to resolve rust-lang#29371.

This is a fairly major rewrite of the `std::ptr` docs, and deserves a fair bit of scrutiny. It adds links to the GNU libc docs for various instrinsics, adds internal links to types and functions referenced in the docs, adds new, more complex examples for many functions, and introduces a common template for discussing unsafety of functions in `std::ptr`.

All functions in `std::ptr` (with the exception of `ptr::eq`) are unsafe because they either read from or write to a raw pointer. The "Safety" section now informs that the function is unsafe because it dereferences a raw pointer and requires that any pointer to be read by the function points to "a valid vaue of type `T`".

Additionally, each function imposes some subset of the following conditions on its arguments.

* The pointer points to valid memory.
* The pointer points to initialized memory.
* The pointer is properly aligned.

These requirements are discussed in the "Undefined Behavior" section along with the  consequences of using functions that perform bitwise copies without requiring `T: Copy`. I don't love my new descriptions of the consequences of making such copies. Perhaps the old ones were good enough?

Some issues which still need to be addressed before this is merged:
- [ ] The new docs assert that `drop_in_place` is equivalent to calling `read` and discarding the value. Is this correct?
- [ ] Do `write_bytes` and `swap_nonoverlapping` require properly aligned pointers?
- [ ] The new example for `drop_in_place` is a lackluster.
- [ ] Should these docs rigorously define what `valid` memory is? Or should is that the job of the reference? Should we link to the reference?
- [ ] Is it correct to require that pointers that will be read from refer to "valid values of type `T`"?
- [x] I can't imagine ever using `{read,write}_volatile` with non-`Copy` types.  Should I just link to {read,write} and say that the same issues with non-`Copy` types apply?
- [x] `write_volatile` doesn't link back to `read_volatile`.
- [ ] Update docs for the unstable [`swap_nonoverlapping`](rust-lang#42818)
- [ ] Update docs for the unstable [unsafe pointer methods RFC](rust-lang/rfcs#1966)

Looking forward to your feedback.

r? @steveklabnik
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
97e82a3

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 15, 2018

@GuillaumeGomez
Rollup merge of rust-lang#49767 - ecstatic-morse:ptr-docs, r=stevekla… 
…bnik

Rewrite docs for `std::ptr`

This PR attempts to resolve rust-lang#29371.

This is a fairly major rewrite of the `std::ptr` docs, and deserves a fair bit of scrutiny. It adds links to the GNU libc docs for various instrinsics, adds internal links to types and functions referenced in the docs, adds new, more complex examples for many functions, and introduces a common template for discussing unsafety of functions in `std::ptr`.

All functions in `std::ptr` (with the exception of `ptr::eq`) are unsafe because they either read from or write to a raw pointer. The "Safety" section now informs that the function is unsafe because it dereferences a raw pointer and requires that any pointer to be read by the function points to "a valid vaue of type `T`".

Additionally, each function imposes some subset of the following conditions on its arguments.

* The pointer points to valid memory.
* The pointer points to initialized memory.
* The pointer is properly aligned.

These requirements are discussed in the "Undefined Behavior" section along with the  consequences of using functions that perform bitwise copies without requiring `T: Copy`. I don't love my new descriptions of the consequences of making such copies. Perhaps the old ones were good enough?

Some issues which still need to be addressed before this is merged:
- [ ] The new docs assert that `drop_in_place` is equivalent to calling `read` and discarding the value. Is this correct?
- [ ] Do `write_bytes` and `swap_nonoverlapping` require properly aligned pointers?
- [ ] The new example for `drop_in_place` is a lackluster.
- [ ] Should these docs rigorously define what `valid` memory is? Or should is that the job of the reference? Should we link to the reference?
- [ ] Is it correct to require that pointers that will be read from refer to "valid values of type `T`"?
- [x] I can't imagine ever using `{read,write}_volatile` with non-`Copy` types.  Should I just link to {read,write} and say that the same issues with non-`Copy` types apply?
- [x] `write_volatile` doesn't link back to `read_volatile`.
- [ ] Update docs for the unstable [`swap_nonoverlapping`](rust-lang#42818)
- [ ] Update docs for the unstable [unsafe pointer methods RFC](rust-lang/rfcs#1966)

Looking forward to your feedback.

r? @steveklabnik
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
7a9eb83

@ecstatic-morse ecstatic-morse referenced this issue May 24, 2018

Closed

Rewrite docs for `std::ptr` (Take #2) #51016

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.