Add `as_{,mut_}ptr` methods to `Option` by nvzqz · Pull Request #80308 · rust-lang/rust

nvzqz · 2020-12-22T21:22:31Z

These enable simpler conversion from Option<&{,mut} T> to *const T and Option<&mut T> to *mut T.

These operations are the opposite of <*const T>::as_ref and <*mut T>::as_mut.

In FFI, the types Option<&T> and Option<&mut T> are semantically equivalent to *const T and *mut T, and they can even be safely used in place of those types.

The self type in each method is a reference because Option<&mut T> does not implement Copy. These methods would otherwise move self, making them inconvenient or sometimes impossible to use.

These enable simpler conversion from `Option<&{,mut} T>` to `*const T` and `Option<&mut T>` to `*mut T`. These operations are the opposite of `<*const T>::as_ref` and `<*mut T>::as_mut`. In FFI, the types `Option<&T>` and `Option<&mut T>` are semantically equivalent to `*const T` and `*mut T`, and they can even be safely used in place of those types. The `self` type in each method is a reference because `Option<&mut T>` does not implement `Copy`. These methods would otherwise move `self`, making them inconvenient or sometimes impossible to use.

rust-highfive · 2020-12-22T21:22:34Z

r? @sfackler

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

nvzqz · 2020-12-22T21:37:52Z

@rustbot modify labels +T-libs

nvzqz · 2021-01-01T03:24:04Z

@rustbot modify labels +A-result-option

m-ou-se

Thanks! Looks useful.

Can you open a tracking issue for this?

m-ou-se · 2021-01-06T16:42:47Z

library/core/src/option.rs

+impl<T> Option<&T> {
+    /// Converts from `Option<&T>` (or `&Option<&T>`) to `*const T`.
+    ///
+    /// This is the opposite of `<*const T>::as_ref`.


Can you turn <*const T>::as_ref into a link to its documentation?

Sure thing. Do you happen to know how to intra-doc link pointer type methods? Otherwise I'll just use the full URL.

Paging @jyn514, who probably knows this. ^^

[..][pointer::as_ref] seems to work, but that doesn't give you control over which of the two as_refs it links to.

Is it guaranteed that #method.as_ref and #method.as_ref-1 will refer to the const and mut variants respectively? If not, it might be worth adding a custom id.

pointer was recently implemented and isn't in beta yet, so you can't use it until the beta bump in 6 weeks (or else doc --stage 0 will break). #80181 (comment)

There is currently no way to distinguish *const from *mut, I wouldn't consider impl-1 stable and would prefer not to use it since the link checker won't warn if *const and *mut get swapped.

m-ou-se · 2021-01-06T16:43:50Z

library/core/src/option.rs

 }

+impl<T> Option<&T> {
+    /// Converts from `Option<&T>` (or `&Option<&T>`) to `*const T`.


(or &Option<&T>)

~~Maybe this should take an Option<&T> by value instead? Or I suppose that might be problematic with the &mut versions?~~ Edit: You already mentioned that. Never mind. :)

Regardless, it's probably better to leave out the (or `&Option<&T>`) part from the documentation, as that just might add confusion.

I added that for consistency with the docs of Option::as_deref and Option::as_deref_mut. I agree it doesn't add much value.

m-ou-se · 2021-01-06T16:46:35Z

library/core/src/option.rs

+    /// assert_eq!(x, unsafe { *ptr });
+    /// ```
+    #[unstable(feature = "option_as_ptr", issue = "none")]
+    #[rustc_const_unstable(feature = "option_as_ptr", issue = "none")]


Is there a reason you put #[rustc_const_unstable] here, but not on the one above?

Because these are over &mut T, which is unstable in const fn. Whereas the one above is &T, which has no issues in const fn. Should they both be #[rustc_const_unstable] regardless?

I'm not sure what the state of that attribute is. In the rustc source code, it says:

rust/compiler/rustc_mir/src/transform/check_consts/mod.rs

Lines 113 to 116 in 8fec6c7

// Functions with `#[unstable]` are const-unstable.

//

// FIXME(ecstaticmorse): We should keep const-stability attributes wholly separate from normal stability

// attributes. `#[unstable]` should be irrelevant.

Which doesn't really give an answer here.

I suppose it's useful to remind us we're also stabilizing the constness of the function at the point when we stabilize these new functions. So it wouldn't hurt to add it to the first one too, just in case.

That's a fair point. I'll make this change then.

m-ou-se · 2021-01-06T16:55:56Z

One more thought: It might be good to explicitly mention that None becomes a null pointer. As it is now, that might not be obvious without checking the implementation or the documentation of pointer::as_ref.

JohnCSimon · 2021-02-08T14:31:53Z

ping from triage:
@nvzqz can you post your update on this PR? Thanks

Dylan-DPC-zz · 2021-02-11T21:17:53Z

@nvzqz
thanks for taking the time to contribute. I have to close this due to inactivity. If you wish and you have the time you can open a new PR with these changes and we'll take it from there. Thanks

rust-highfive assigned sfackler Dec 22, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 22, 2020

rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 22, 2020

Add missing "mut" to docs

1e0627e

rustbot added the A-result-option Area: Result and Option combinators label Jan 1, 2021

m-ou-se assigned m-ou-se and unassigned sfackler Jan 6, 2021

m-ou-se reviewed Jan 6, 2021

View reviewed changes

m-ou-se added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2021

Dylan-DPC-zz closed this Feb 11, 2021

Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 11, 2021

	// Functions with `#[unstable]` are const-unstable.
	//
	// FIXME(ecstaticmorse): We should keep const-stability attributes wholly separate from normal stability
	// attributes. `#[unstable]` should be irrelevant.

Uh oh!

Conversation

nvzqz commented Dec 22, 2020

Uh oh!

rust-highfive commented Dec 22, 2020

Uh oh!

nvzqz commented Dec 22, 2020

Uh oh!

nvzqz commented Jan 1, 2021

Uh oh!

m-ou-se left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nvzqz Jan 6, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m-ou-se Jan 6, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m-ou-se commented Jan 6, 2021

Uh oh!

JohnCSimon commented Feb 8, 2021

Uh oh!

Dylan-DPC-zz commented Feb 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nvzqz Jan 6, 2021 •

edited

Loading

m-ou-se Jan 6, 2021 •

edited

Loading