Refactor set_ptr_value as with_metadata_of #95249
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
By reversing the arguments we achieve several clarifications: - The function closely resembles `cast` but with an argument to initialized the metadata. This is easier to teach and answers an long outstanding question that had restricted cast to `Sized` targets initially. See multiples reviews of <rust-lang#47631> - The 'object identity', in the form or provenance, is now preserved from the call receiver to the result. This helps explain the method as a builder-style, instead of some kind of setter that would modify something in-place. Ensuring that the result has the identity of the `self` argument is also beneficial for an intuition of effects. - An outstanding concern, 'Correct argument type', is avoided by not committing to any specific argument type. This is consistent with cast which does not require its receiver to be a raw address.
|
r? @dtolnay (rust-highfive has picked a reviewer for you, use r? to override) |
dtolnay
approved these changes
Mar 28, 2022
|
@bors r+ |
|
📌 Commit d489ea7 has been approved by |
|
⌛ Testing commit d489ea7 with merge c3df12a0160c390d2af772844007b3cafd79b9b4... |
|
💔 Test failed - checks-actions |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
I could use some help interpreting the error:
Spurios or related to any doc-comments of this PR? |
|
Unclear, but let's find out if spurious. @bors retry |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (c1230e1): comparison url. Summary: This benchmark run did not return any relevant results. 9 results were found to be statistically significant but too small to be relevant. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
yvt
added a commit
to r3-os/r3
that referenced
this pull request
Mar 30, 2022
<rust-lang/rust#95249> This method is a part of the `set_ptr_value` unstable feature.
lopopolo
added a commit
to artichoke/cactusref
that referenced
this pull request
Apr 2, 2022
rust-lang/rust#95249 reworked the `ptr::set_ptr_value` API. The new API is called `ptr::with_metadata_of`. This PR syncs this crate's `Rc` impl with upstream. This nightly breakage means a new semver incompatible version bump is due.
8 tasks
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Oct 22, 2022
…nter_argument, r=dtolnay Adjust argument type for mutable with_metadata_of (rust-lang#75091) The method takes two pointer arguments: one `self` supplying the pointer value, and a second pointer supplying the metadata. The new parameter type more clearly reflects the actual requirements. The provenance of the metadata parameter is disregarded completely. Using a mutable pointer in the call site can be coerced to a const pointer while the reverse is not true. In some cases, the current parameter type can thus lead to a very slightly confusing additional cast. [Example](HeroicKatora/static-alloc@cad9377). ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = val as *const _ as *mut T; let ptr = uninit.as_ptr().with_metadata_of(ptr); ``` This could then instead be simplified to: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = uninit.as_ptr().with_metadata_of(&**val); ``` Tracking issue: rust-lang#75091 `@dtolnay` you're reviewed rust-lang#95249, would you mind chiming in?
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Oct 22, 2022
…nter_argument, r=dtolnay Adjust argument type for mutable with_metadata_of (rust-lang#75091) The method takes two pointer arguments: one `self` supplying the pointer value, and a second pointer supplying the metadata. The new parameter type more clearly reflects the actual requirements. The provenance of the metadata parameter is disregarded completely. Using a mutable pointer in the call site can be coerced to a const pointer while the reverse is not true. In some cases, the current parameter type can thus lead to a very slightly confusing additional cast. [Example](HeroicKatora/static-alloc@cad9377). ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = val as *const _ as *mut T; let ptr = uninit.as_ptr().with_metadata_of(ptr); ``` This could then instead be simplified to: ```rust // Manually taking an unsized object from a `ManuallyDrop` into another allocation. let val: &core::mem::ManuallyDrop<T> = …; let ptr = uninit.as_ptr().with_metadata_of(&**val); ``` Tracking issue: rust-lang#75091 ``@dtolnay`` you're reviewed rust-lang#95249, would you mind chiming in?
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.
Replaces
set_ptr_value(#75091) with methods of reversed argument order:By reversing the arguments we achieve several clarifications:
castwith an argument toinitialize the metadata. This is easier to teach and answers a long
outstanding question that had restricted cast to
Sizedpointeetargets. See multiples reviews of
Add some APIs to ptr::NonNull and fix
sinceattributes #47631from the receiver argument to the result. This helps explain the method as
a builder-style, instead of some kind of setter that would modify
something in-place. Ensuring that the result has the identity of the
selfargument is also beneficial for an intuition of effects.committing to any specific argument type. This is consistent with cast
which does not require its receiver to be a 'raw address'.
Hopefully the usage examples in
sync/rc.rsserve as sufficient examples of the style to convince the reader of the readability improvements of this style, when compared to the previous order of arguments.I want to take the opportunity to motivate inclusion of this method separate from metadata API, separate from
feature(ptr_metadata). It does not involve thePointeetrait in any form. This may be regarded as a very, very light form that does not commit to any details of the pointee trait, or its associated metadata. There are several use cases for which this is already sufficient and no further inspection of metadata is necessary.Storing the coercion of
*mut Tinto*mut dyn Traitas a way to dynamically cast some an arbitrary instance of the same type to a dyn trait instance. In particular, one can have a field of typeOption<*mut dyn io::Seek>to memorize if a particular writer is seekable. Then a methodfn(self: &T) -> Option<&dyn Seek>can be provided, which does not involve the static trait boundT: Seek. This makes it possible to create an API that is capable of utilizing seekable streams and non-seekable streams (instead of a possible less efficient manner such as more buffering) through the same entry-point.Enabling more generic forms of unsizing for no-
stdsmart pointers. Using the stable APIs only few concrete cases are available. One can unsize arrays to[T]byptr::slice_from_raw_partsbut unsizing a custom smart pointer to, e.g.,dyn Iterator,dyn Future,dyn Debug, can't easily be done generically. Exposingwith_metadata_ofwould allow smart pointers to offer their ownunsafeescape hatch with similar parameters where the caller provides the unsized metadata. This is particularly interesting for embedded wheredyn-trait usage can drastically reduce code size.