Skip to content

Implement Index for SliceIndex types for str #55603

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
alercah opened this issue Nov 2, 2018 · 5 comments · Fixed by #57604
Closed

Implement Index for SliceIndex types for str #55603

alercah opened this issue Nov 2, 2018 · 5 comments · Fixed by #57604
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alercah
Copy link
Contributor

alercah commented Nov 2, 2018

While writing a generic implementation today, I was surprised that you can't do s[i] for i: SliceIndex<S>, if s is of slice type &S. Doing s.get(i).unwrap() is an easy enough workaround, but feels really bad. Replacing the various explicit impls with one for SliceIndex would feel much nicer.
@eddyb
Copy link
Member

eddyb commented Nov 4, 2018

cc @rust-lang/libs

@sfackler
Copy link
Member

sfackler commented Nov 4, 2018

The bounds on [T]'s get and Index impls seem the same?

https://doc.rust-lang.org/std/primitive.slice.html#method.get
https://doc.rust-lang.org/std/primitive.slice.html#impl-Index%3CI%3E

@SimonSapin
Copy link
Contributor

Perhaps @alercah meant for str rather than slices? https://doc.rust-lang.org/std/primitive.str.html#impl-Index%3CRange%3Cusize%3E%3E

@alercah
Copy link
Contributor Author

alercah commented Nov 4, 2018

Ah, yes, I did. I mistakenly assumed they are in fact the same. :)

@sfackler
Copy link
Member

sfackler commented Nov 4, 2018

Ah yeah, that seems like an oversight!

@alercah alercah changed the title Implement Index for SliceIndex types Implement Index for SliceIndex types for str Nov 4, 2018
@sfackler sfackler mentioned this issue Nov 5, 2018
Closed
bors pushed a commit that referenced this issue Nov 5, 2018
@sfackler
Use SliceIndex for str's Index/IndexMut impls
55beec2 
This was missed by accident when the initial implementation landed!

Closes #55603
bors added a commit that referenced this issue Nov 5, 2018
@bors
Auto merge of #55683 - sfackler:str-index, r=<try>
32bb15d 
Use SliceIndex for str's Index/IndexMut impls

This was missed by accident when the initial implementation landed!

Closes #55603
@estebank estebank added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 11, 2019
@alercah alercah mentioned this issue Jan 14, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Jan 22, 2019
@Centril
Rollup merge of rust-lang#57604 - alercah:str-index, r=sfackler
ad55b73 
Make `str` indexing generic on `SliceIndex`.

Fixes rust-lang#55603
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make str indexing generic on SliceIndex.
5 participants
@eddyb @SimonSapin @sfackler @estebank @alercah