Skip to content
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

Add [T]::as_ptr_range() #2791

Open
wants to merge 1 commit into
base: master
from

Conversation

@m-ou-se
Copy link

m-ou-se commented Oct 25, 2019

Rendered

Adds .as_ptr_range() and .as_mut_ptr_range() to slices, which give both the start and the (one-past-the-)end pointer of a slice:

let r = slice.as_ptr_range();
some_ffi_function(r.start, r.end);
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Oct 25, 2019

@rust-lang/libs Does this need an RFC? Unsure on the process here.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Oct 25, 2019

Unless there’s a lot of design to discuss we’re generally OK with accepting a PR adding “small” APIs to the standard library, without an RFC. Having two people (submitter + reviewer) think something is a good idea is deemed good enough for including unstable APIs, on the basis that there will be wider review and consensus-gathering before FCP for stabilization.

@rust-lang/libs We should probably tweak https://github.com/rust-lang/rfcs/blob/master/libs_changes.md#is-an-rfc-required to reflect current practice regarding new APIs.

@Centril

This comment has been minimized.

Copy link
Member

Centril commented Oct 25, 2019

Seems well-written and well-motivated regardless. So when the text is written why not merge it as a bit of advertisement? ;)

@m-ou-se

This comment has been minimized.

Copy link
Author

m-ou-se commented Oct 25, 2019

Unless there’s a lot of design to discuss we’re generally OK with accepting a PR adding “small” APIs to the standard library, without an RFC. Having two people (submitter + reviewer) think something is a good idea is deemed good enough for including unstable APIs, on the basis that there will be wider review and consensus-gathering before FCP for stabilization.

Good to know! I'll send a PR to the Rust repository then.

Before I make the same mistake again: Would the things I describe in future possibilities warrant an RFC, or could that also be a Rust PR directly? (Still a small change, but there's more to discuss about signatures and names, I'd guess.)

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Oct 25, 2019

I should have clarified: my previous comment is about the process in general. I don’t mean that this RFC should be closed, now that it’s written.

@m-ou-se It’s not really a mistake. There are two options, if unsure do what’s easiest to you.

One of the reasons the RFC process exists is that people sometimes poured a lot of effort into implementing something that turned later out not to be accepted, or not in that form. This is not a concern when an implementation PR is easy to make.

m-ou-se added a commit to fusion-engineering-forks/rust that referenced this pull request Oct 25, 2019
m-ou-se added a commit to fusion-engineering-forks/rust that referenced this pull request Oct 25, 2019
@m-ou-se

This comment has been minimized.

Copy link
Author

m-ou-se commented Oct 25, 2019

Ah okay :) Thanks for the explanation!

Well here's an implementation: rust-lang/rust#65806

Centril added a commit to Centril/rust that referenced this pull request Oct 25, 2019
…range, r=Centril

Add [T]::as_ptr_range() and [T]::as_mut_ptr_range().

Implementation of rust-lang/rfcs#2791
Centril added a commit to Centril/rust that referenced this pull request Oct 25, 2019
…range, r=Centril

Add [T]::as_ptr_range() and [T]::as_mut_ptr_range().

Implementation of rust-lang/rfcs#2791
Centril added a commit to Centril/rust that referenced this pull request Oct 25, 2019
…range, r=Centril

Add [T]::as_ptr_range() and [T]::as_mut_ptr_range().

Implementation of rust-lang/rfcs#2791
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 25, 2019
…range, r=Centril

Add [T]::as_ptr_range() and [T]::as_mut_ptr_range().

Implementation of rust-lang/rfcs#2791
The slice is half-open, which means that the end pointer points *one past* the
last element of the slice. This way, an empty slice is represented by two equal
pointers, and the difference between the two pointers represents the size of
the size.

This comment has been minimized.

Copy link
@jplatte

jplatte Oct 31, 2019

Suggested change
the size.
the slice.
It also allows for things like

```rust
slice.as_ptr_range().contains(some_element)

This comment has been minimized.

Copy link
@thomcc

thomcc Nov 2, 2019

Is this well defined? In many other low level languages (okay, in C), it's only well defined if it returns true. I had heard this was the case for rust as well, but could have been misinformed.

This comment has been minimized.

Copy link
@KrishnaSannasi

KrishnaSannasi Nov 2, 2019

We could use the wrapping_* pointer operations to make this sound

This comment has been minimized.

Copy link
@eddyb

eddyb Nov 2, 2019

Member

contains only uses comparisons though, which are safe, aren't they?

This comment has been minimized.

Copy link
@golddranks

golddranks Nov 3, 2019

Is this well defined? In many other low level languages (okay, in C), it's only well defined if it returns true.

In C, pointer comparisons are defined only in cases where the pointers being compared originate from the same object. (Here, I think that object is commonly understood to mean "a stack or heap allocated value of some type") As Rust defines std::ptr::eq and Ord comparisons to be safe APIs (see here: https://doc.rust-lang.org/std/primitive.pointer.html), that means that they are guaranteed to be well defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.