-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 get
and get_unchecked
to *const [T]
and *mut [T]
#60639
Comments
My specific case would have been for |
no, I'm just lazy and didn't want to type everything twice. There should be an equivalent of all the above for |
get
and get_unchecked
to *const [T]
get
and get_unchecked
to *const [T]
and *mut [T]
Pardon me if It's stupid but why not just let the user go from fn main() {
let x_ptr = &[1, 2, 4] as *const [_];
unsafe {
let x = &*x_ptr;
println!("{:?}", x.get(1).map(|x| x as *const _));
println!("{:?}", x.get_unchecked(1) as *const _);
}
} Is that really to long to write ? Also, want to ask, when do you need a |
The original context was this code: use std::collections::BTreeSet;
fn uniq_refs<'i, 'd: 'i, T>(
data: &'d mut [T],
indices: &'i BTreeSet<usize>,
) -> impl Iterator<Item = &'d mut T> + 'i {
let in_bounds_indices = indices.range(0..data.len());
in_bounds_indices.map(move |&i| {
// I copied this from a Stack Overflow answer
// without reading the text that explains why this is safe
unsafe {
let r = data.get_unchecked_mut(i);
let p: *mut T = r;
&mut *p
}
})
} Miri detects that this usage triggers undefined behavior: use std::iter::FromIterator;
fn main() {
let mut scores = vec![1, 2, 3];
let indices = BTreeSet::from_iter(vec![0, 2]);
let selected_scores: Vec<_> = uniq_refs(&mut scores, &indices).collect();
for score in selected_scores {
*score += 1;
}
} I wanted to make use of fn uniq_refs<'i, 'd: 'i, T>(
data: &'d mut [T],
indices: &'i BTreeSet<usize>,
) -> impl Iterator<Item = &'d mut T> + 'i {
let start = data.as_mut_ptr();
let in_bounds_indices = indices.range(0..data.len());
in_bounds_indices.map(move |&i| unsafe { &mut *start.add(i) })
} With the proposed functions, that code could look like fn uniq_refs<'i, 'd: 'i, T>(
data: &'d mut [T],
indices: &'i BTreeSet<usize>,
) -> impl Iterator<Item = &'d mut T> + 'i {
let start = data as *mut [T];
let in_bounds_indices = indices.range(0..data.len());
in_bounds_indices.map(move |&i| unsafe { &mut *data.get_unchecked(i) })
} |
Yeah, this seems reasonable. However, I think it should be seen in the context of our general need for a better story for raw pointers. |
impl<T> *const [T] {
fn get(self, idx: usize) -> Option<*const T>;
} Wouldn't that need to be |
Since it would be bounds-checked, you can't overflow, so |
Ah no, you're right. One can create a wrong slice that would overflow for some indices... |
It could use |
just to note that in C, it's UB to create a out of bound pointer of more than len, for exemple: char *ptr = malloc(42); // assert no null
char *ub = ptr + 43;
char *also_ub = ptr - 1; Look like Rust allow this, but use raw pointer is often use with C binding, I just through this could be useful to know. |
But then people would have a performance incentive to not use the clearer and more convenient method. And what for? If one uses this method they'll almost always go on to access memory through the resulting pointer, and then they'd almost certainly have UB anyway if the index is too large or goes otherwise out of bounds. While wrapping_offset does allow "allocation crossing" with a suitably crafted offset, this is extremely rare and so I don't see why we should prioritize it in a convenience method. |
If they want the performance, wouldn't they use Put differently, what is even the point of having |
This is a good question. I feel like the answer might well be "there is no point to having I can see some potential value in how an unsafe
I believe this distinction between trusting the slice and trusting the indices also answers this question:
They might need the bounds-check anyway (because the indices are untrusted) and then they might as well leave that to the standard library rather than open-coding it. |
For that |
@gnzlbg why that? A |
I was missing this, makes sense.
So does this mean that if the pointer is NULL, the len must be zero, or else the pointer is invalid ? |
It's a raw pointer, I don't think we are making any such requirements. Both pointer and integer can be just about anything that a |
So, in terms of implementing this as an unstable feature... the main problem is that we want these to be inherent methods on |
Yea, we need to add the same kind of magic that the integer types have in Lines 2380 to 2381 in 1572c43
Though in this case it may be more difficult due to the involved generics, so you should look at the implementation of the magic for slices for inspiration: Lines 56 to 58 in 1572c43
|
#71082 does all the hard parts here -- now adding more raw slice methods (like indexing) is a libcore-only patch, rustc does not need any further changes. |
#73986 should resolve this. |
add (unchecked) indexing methods to raw (and NonNull) slices This complements the existing (unstable) `len` method. Unfortunately, for non-null slices, we cannot call this method `as_ptr` as that overlaps with the existing method of the same name. If this looks reasonable to accept, I propose to reuse the rust-lang#71146 tracking issue and rename the feature get to `slice_ptr_methods` or so. Cc @SimonSapin Fixes rust-lang#60639
add (unchecked) indexing methods to raw (and NonNull) slices This complements the existing (unstable) `len` method. Unfortunately, for non-null slices, we cannot call this method `as_ptr` as that overlaps with the existing method of the same name. If this looks reasonable to accept, I propose to reuse the rust-lang#71146 tracking issue and rename the feature get to `slice_ptr_methods` or so. Cc @SimonSapin Fixes rust-lang#60639
add (unchecked) indexing methods to raw (and NonNull) slices This complements the existing (unstable) `len` method. Unfortunately, for non-null slices, we cannot call this method `as_ptr` as that overlaps with the existing method of the same name. If this looks reasonable to accept, I propose to reuse the rust-lang#71146 tracking issue and rename the feature get to `slice_ptr_methods` or so. Cc @SimonSapin Fixes rust-lang#60639
add (unchecked) indexing methods to raw (and NonNull) slices This complements the existing (unstable) `len` method. Unfortunately, for non-null slices, we cannot call this method `as_ptr` as that overlaps with the existing method of the same name. If this looks reasonable to accept, I propose to reuse the rust-lang#71146 tracking issue and rename the feature get to `slice_ptr_methods` or so. Cc @SimonSapin Fixes rust-lang#60639
add (unchecked) indexing methods to raw (and NonNull) slices This complements the existing (unstable) `len` method. Unfortunately, for non-null slices, we cannot call this method `as_ptr` as that overlaps with the existing method of the same name. If this looks reasonable to accept, I propose to reuse the rust-lang#71146 tracking issue and rename the feature get to `slice_ptr_methods` or so. Cc @SimonSapin Fixes rust-lang#60639
add (unchecked) indexing methods to raw (and NonNull) slices This complements the existing (unstable) `len` method. Unfortunately, for non-null slices, we cannot call this method `as_ptr` as that overlaps with the existing method of the same name. If this looks reasonable to accept, I propose to reuse the rust-lang#71146 tracking issue and rename the feature get to `slice_ptr_methods` or so. Cc @SimonSapin Fixes rust-lang#60639
The checked version, impl<T> *const [T] {
fn get(self, idx: usize) -> Option<*const T>;
} wasn't implemented. Why? |
As far as I am concerned, mostly due to lack of imagination -- I figured if you'd want checks you'd use references, not raw pointers. The return type |
If one has a
*const [T]
and wants a pointer to a specific element of that slice, one needs to go through a reference and index, or cast the pointer to*const T
and offset that by the number of elements (which doesn't do a length check, so you need that, too). I propose that we add two new methods to*const [T]
and*mut [T]
:get_unchecked
is unsafe, because if a large index is used, one may overflow the pointer, which is UBTo make the implementation of
get
simpler, I propose to additionally add alen
method:cc @RalfJung @shepmaster
The text was updated successfully, but these errors were encountered: