-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Signed integer indexing and slicing #2249
Comments
Also should clarify this applies to indexing as well as slicing, i.e. |
Sounds like this could be really neat when dealing with things like "index to the player on the left" where you subtract when going to the left or the dining philosophers problem-like cases. Please write that RFC =) |
@Centril honestly the main reason why I asked is I'm legitimately confused why this hasn't been brought up yet. I feel like there's a reason why this wasn't done that I wasn't able to find, so, I figured I'd ask what people think before writing something up. |
One thing that's bothersome about Python's solution is that def last_n_items(xs, n):
return xs[-n:]
last_n_items(range(10), 3) # [7, 8, 9]
last_n_items(range(10), 2) # [8, 9]
last_n_items(range(10), 1) # [9]
last_n_items(range(10), 0) # [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] For this reason I could never see this as being incorporated into rust as indexing by actual negative integers. Maybe indexing by some new enum type with a |
Negative indices in C mean something completely different and are super scary (but are defined behaviour!!). Imitating the python behaviour will be surprising to anyone with a C/C++ background and would be a bad idea IMO. Alternative suggestions I would prefer really much:
|
I think we should here first and foremost focus on semantics and not whether or not we should have a wrapper type around That said, |
A question that every proposal to change the language has to answer: why should it be in the standard library instead of being inside a third party crate? I believe it deserves to be in the standard library, especially given that python has support for this too. I am only against the actual |
@est31 In an effort to cheat Wadler's law I will agree with you on the concrete syntax To be clear, what I was proposing was (playground): // Inside core::ops:
pub trait Index<Idx>
where
Idx: ?Sized,
{
type Output: ?Sized;
fn index(&self, index: Idx) -> &Self::Output;
}
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] // <- discuss
pub struct FromEnd<T> {
pub index: T,
}
pub fn end<T>(index: T) -> FromEnd<T> { // <- bikeshed name
FromEnd { index }
}
impl<T> From<T> for FromEnd<T> {
fn from(index: T) -> Self { end(index) }
}
impl<T> Index<FromEnd<usize>> for Vec<T> {
type Output = T;
fn index(&self, index: FromEnd<usize>) -> &Self::Output {
// Panicing is taken care of for us... so no buffer underflow.
&self[self.len() - 1 - index.index]
}
}
fn main() {
let vec = (0..10).collect::<Vec<_>>();
println!("{:?}", vec.index(end(0)));
// ^- obviously vec[end(0)] but this isn't inside libcore.
} |
Hmm.. on second thought, I think |
To add to your argument for the proposition, if you replace the trait definition above with:
you run into coherence troubles:
Therefore it has to be in libcore, or no dice. |
I think that having a I may go ahead with the |
@clarcharr If you want to co-author I'm interested =) You can find me at #rust-lang @ irc.mozilla.org for quicker discussions. |
@Centril I will get in touch! :) I need to rethink this for a bit though. Right now, thinking again, it still makes sense to support signed indices for the sake of ranges. For example, I also question the usefulness of negative indices from a C background. Doing such an index is inherently unsafe, and Rust allowing it leads to suspicion. I feel like that problem could easily be avoided by having a clippy lint that points out negative indexing, potentially enabled alongside a bunch of other "catch-ups for C Devs" lints. |
@petrochenkov and @koutheir, mind adding any extra thoughts considering how you both downvoted this? |
@Centril good point!
I have personally could have used
As I've said in my comment, negative indexing is defined behaviour in C. I've linked to a discussion on stack overflow where the C standard was quoted. char *c = malloc(40);
c += 20;
c[-1] = 10;
printf("%d\n", c[-1]); All non-ub "safe" C here, except that you should maybe verify that malloc doesnt return NULL. |
Right now, pub struct Range<Idx> {
pub start: Idx,
pub end: Idx,
} It could be extended to be a struct like pub struct Range<IdxStart, IdxEnd=IdxStart> {
pub start: IdxStart,
pub end: IdxEnd,
} This would be 100% backwards compatible, wouldnt it? |
Also, if you went with the signed indices proposal, you'd have to change how the compiler is doing inference when it sees an indexing operation in order to make stuff work. Right now, impl'ing index for isize will totally break inference: Although inference breakages are not guaranteed by the language, I think in this case the breakage would be too much to not be fixed by a compiler change :). I'm not even 100% sure whether inference happens before or after operators are desugared, if it happens after, the required changes would be even more wide-ranging. |
My thoughts on
Edit: Actually, I can also think of an alternative model, but I'm not sure if I like it:
Edit 2: Argh! I just noticed that neither of these models let you do something like |
I guess there are really multiple distinct things that python's negative indices are useful for, none of which it is really ideal for, and which may each be better served by distinct apis in rust. My current thoughts are at least two features:
impl<T> [T] {
pub fn rsplit_at(&self, k: usize) -> (&[T], &[T]) {
self.split_at(self.len() - k)
}
} (note: A disadvantage of |
WRT original proposal: IMO this would reduce safety by making accidental index underflows harder to detect. Typing |
I'm glad to see this leading away from indexing by mixed ranges. The thing I'm saddest about there are situations like "is A bunch of the scenarios I've seen in the thread remind me of iterator adapters, just directly on slices. For example, |
The very nice solution of D language:
That is equivalent to Python:
In D you can even overload the $ (defining opDollar) if you want to define multi-dimensional tensor-like data structures (you overload each $ for each dimension of the tensor): |
@scottmcm that's actually a nice idea! All of these adapters should be constant-time for slice iterators, and perhaps there may be a way to add convenience functions if they're desired. |
Random person from the internet here Basic
Ranges
Reverse ranges
Negative zero for inclusive reverse ranges
I understand that some of these may seem weird but I just wanted to show what could be the equivalent using iterators. |
I personally really like the idea from D of having a value like For example, instead of |
C# 8 implemented it with |
I made a crate to try out the "slice adapter" idea above (#2249 (comment)) let r = [1, 2, 4, 9, 16, 25].rev();
assert_eq!(r[0], 25);
assert_eq!(r[1..3].rev(), &[9, 16]);
assert_eq!(r.split_first().unwrap().0, &25);
let mut it = r.iter().cloned().skip(2);
assert_eq!(it.next(), Some(9));
assert_eq!(it.next(), Some(4));
assert_eq!(it.next(), Some(2)); |
Um, I'm from Python where we have iterators (about any container) and sequences (indexable, lengthed containers). I see iterators have a .count(), method, which is a bit confusing, but I'm too lazy to RTFM. So would someone kind link the documentation or explain what is what. Otherwise, I like the solution used in D. It also exists in GNU Octave, where they reused the keyword |
In general, in Rust you can’t efficiently know the length of an iterator. You just have consume all the elements and count them. This is what the count() method does. |
Right, it's very different from python's |
The C# example -- which works by having a newtype that means "from the end" reminded me that we have (I can't decide if this is a good idea or an abuse of the type, though.) |
To me using something like the D syntax (with # instead of $) seems way better than |
did this ever go anywhere? |
You can already implement this yourself by defining a type |
How about using curly braces like Or normal braces I really do use negative indices a lot in Python and always hate it when I have to use
Let's assume you have a list of files in the format:
Then you can quickly extract the number via This could even be applied to single elements like Edit: Wait a minute, I just realized that using |
@danielfaust This would break literally every bracket matching and cold folding algorithm out there, and looks just as confusing to humans. |
Even if this doesn't support everything we want, can we get something out? The discussion seems generally happy with |
I think that one thing that's ever so very slightly confusing about |
I would suggest against a Rust-only keyword for a quite standard operation like As a daily Python programmer, experienced in other languages and a beginner Rust learning going through Rustlings, I was surprised that negative indices are not a thing. While If I have programmed for the past days in a different language and would try to do a negative offset in a slice in Rust, I would start with
Also,
If I ask ChatGPT if
Personally, even if While it is just the opinion of a single person, I hope it helps the discussion. |
I haven't found an RFC or Rust issue for this, so, I figured I'd mention it here. The only ones I've seen suggested panicking on negative values, which would not be desirable.
Is there a reason why there aren't impls for signed indexing? For example, being able to make slices like
s[..-2]
might be useful. This would still technically be opt-in, as indexing onusize
would not have the extra branch, but indexing onisize
would.And just to clarify, negative indexes like
-1
and-2
get added to the length of the slice to become real indices, likelen - 1
andlen - 2
. If the value is less than the absolute value of the length, then it would be marked as OOB like any other index.This seems pretty ergonomic as languages like Python and Bash have been using negative indices for a long time. Technically, this would be a bigger project than simply adding
Index
and other impls, as the const evaluator would also need to be updated.If there is desire for this, I'd be happy to write an RFC for it.
The text was updated successfully, but these errors were encountered: