de-stabilize unsized raw ptr methods for Weak #80422
Merged
Conversation
This file contains 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
|
r? @kennytm (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ p=5 |
|
📌 Commit 8e0b7f9 has been approved by |
bors
added
S-waiting-on-bors
Mark-Simulacrum
added
beta-accepted
RalfJung
force-pushed
the
weak-no-unsized-raw
branch
2 times, most recently
from
fbf8b85
to
8e0b7f9
Compare
|
@bors r+ |
|
📌 Commit 8e0b7f9 has been approved by |
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 28, 2020
…Mark-Simulacrum de-stabilize unsized raw ptr methods for Weak `@Mark-Simulacrum` this is the beta branch version of rust-lang#80422.
Mark-Simulacrum
removed
the
beta-nominated
Merged
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 31, 2020
…ulacrum [beta] backports This backports accepted PRs and switches to bootstrapping from the released compiler: * de-stabilize unsized raw ptr methods for Weak rust-lang#80422 * Use package name for top-level directory in bare tarballs rust-lang#80397 * Prevent caching normalization results with a cycle rust-lang#80246 r? `@Mark-Simulacrum`
|
Does this need |
|
It landed just before the release, so it shouldn't need relnotes I think. |
m-ou-se
added a commit
to m-ou-se/rust
that referenced
this pull request
Jan 16, 2021
…RalfJung Re-stabilize Weak::as_ptr and friends for unsized T As per [T-lang consensus](https://hackmd.io/7r3_is6uTz-163fsOV8Vfg), this uses a branch to handle the dangling case. The discussed optimization of only doing the branch in the T: ?Sized case is left for a followup patch, as doing so is not trivial (as it requires specialization) and not _obviously_ better (as it requires using `wrapping_offset` rather than `offset` more). <details><summary>Basically said optimization</summary> Specialize on `T: Sized`: ```rust fn as_ptr(&self) -> *const T { if [ T is Sized ] || !is_dangling(ptr) { (ptr as *mut T).set_ptr_value( (ptr as *mut u8).wrapping_offset(data_offset) ) } else { ptr::null() } } fn from_raw(*const T) -> Self { if [ T is Sized ] || !ptr.is_null() { let ptr = (ptr as *mut RcBox).set_ptr_value( (ptr as *mut u8).wrapping_offset(-data_offset) ); Weak { ptr } } else { Weak::new() } } ``` (but with more `set_ptr_value` to avoid `Sized` restrictions and maintain metadata.) Written in this fashion, this is not a correctness-critical specialization (i.e. so long as `[ T is Sized ]` is false for unsized `T`, it can be `rand()` for sized `T` without breaking correctness), but it's still touchy, so I'd rather do it in another PR with separate review. --- </details> This effectively reverts rust-lang#80422 and re-establishes rust-lang#74160. T-libs [previously signed off](rust-lang#74160 (comment)) on this stable API change in rust-lang#74160.
m-ou-se
added a commit
to m-ou-se/rust
that referenced
this pull request
Jan 16, 2021
…RalfJung Re-stabilize Weak::as_ptr and friends for unsized T As per [T-lang consensus](https://hackmd.io/7r3_is6uTz-163fsOV8Vfg), this uses a branch to handle the dangling case. The discussed optimization of only doing the branch in the T: ?Sized case is left for a followup patch, as doing so is not trivial (as it requires specialization) and not _obviously_ better (as it requires using `wrapping_offset` rather than `offset` more). <details><summary>Basically said optimization</summary> Specialize on `T: Sized`: ```rust fn as_ptr(&self) -> *const T { if [ T is Sized ] || !is_dangling(ptr) { (ptr as *mut T).set_ptr_value( (ptr as *mut u8).wrapping_offset(data_offset) ) } else { ptr::null() } } fn from_raw(*const T) -> Self { if [ T is Sized ] || !ptr.is_null() { let ptr = (ptr as *mut RcBox).set_ptr_value( (ptr as *mut u8).wrapping_offset(-data_offset) ); Weak { ptr } } else { Weak::new() } } ``` (but with more `set_ptr_value` to avoid `Sized` restrictions and maintain metadata.) Written in this fashion, this is not a correctness-critical specialization (i.e. so long as `[ T is Sized ]` is false for unsized `T`, it can be `rand()` for sized `T` without breaking correctness), but it's still touchy, so I'd rather do it in another PR with separate review. --- </details> This effectively reverts rust-lang#80422 and re-establishes rust-lang#74160. T-libs [previously signed off](rust-lang#74160 (comment)) on this stable API change in rust-lang#74160.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@Mark-Simulacrum this is the patch re: #80407.
I couldn't figure out the branch it needs to go on though, stable is still the old stable but beta already the new beta...?