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

rustc: Remove &str indexing from the language. #15085

Merged
merged 1 commit into from
Jul 2, 2014
Merged

Conversation

brson
Copy link
Contributor

@brson brson commented Jun 21, 2014

Being able to index into the bytes of a string encourages
poor UTF-8 hygiene. To get a view of &[u8] from either
a String or &str slice, use the as_bytes() method.

Closes #12710.

[breaking-change]

If the diffstat is any indication this shouldn't have a huge impact but it will have some. Most changes in the str and path module. A lot of the existing usages were in tests where ascii is expected. There are a number of other legit uses where the characters are known to be ascii.

@emberian
Copy link
Member

It might be valuable to have an as_ascii method that returns &[Ascii]

@alexcrichton
Copy link
Member

As a language change, this has no RFC corresponding with it, but it's small enough that it may be ok to discuss here instead.

I personally would like to keep indexing. It is a fact that all our strings are utf-8 encoded, and that fact will never change. The type system will very quickly tell you you're getting a byte and not a char.

@aturon
Copy link
Member

aturon commented Jun 23, 2014

@alexcrichton As @brson points out, you can get the utf-8 view from as_bytes, so that's not a very compelling argument by itself.

On the other hand, a bunch of the other methods on &str use byte indices, e.g. slicing, char_at, find. So we're not really hiding the fact the utf-8-ness of strings. It seems like we need a consistent rationale about where and how to permit working with string data as utf-8-encoded.

@brson Do you see these other methods as also encouraging poor hygiene? Or is there something that makes indexing particularly pernicious?

@alexcrichton
Copy link
Member

It seems that we have a bit of a push-back from any byte-based methods on strings, but there are so many that I think it's ergonomically infeasible to remove all of them. It seems a little odd to remove half but leave the other half, leading to my opinion that this is a nice sugar that should stick around.

@brson
Copy link
Contributor Author

brson commented Jun 24, 2014

@aturon I don't have a strong opinion about whether this is the correct thing to do or not. It has been previously-accepted as a milestone issue, so I implemented it.

@brson
Copy link
Contributor Author

brson commented Jul 2, 2014

@alexcrichton
Copy link
Member

r=me with a rebase

Being able to index into the bytes of a string encourages
poor UTF-8 hygiene. To get a view of `&[u8]` from either
a `String` or `&str` slice, use the `as_bytes()` method.

Closes rust-lang#12710.

[breaking-change]
bors added a commit that referenced this pull request Jul 2, 2014
Being able to index into the bytes of a string encourages
poor UTF-8 hygiene. To get a view of `&[u8]` from either
a `String` or `&str` slice, use the `as_bytes()` method.

Closes #12710.

[breaking-change]

If the diffstat is any indication this shouldn't have a huge impact but it will have some. Most changes in the `str` and `path` module. A lot of the existing usages were in tests where ascii is expected. There are a number of other legit uses where the characters are known to be ascii.
@huonw huonw mentioned this pull request Jul 2, 2014
@bors bors closed this Jul 2, 2014
@bors bors merged commit d21336e into rust-lang:master Jul 2, 2014
@SimonSapin
Copy link
Contributor

@cmr, the to_ascii and into_ascii methods already exist, but you have to use the traits from std::ascii.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove string [] indexing
6 participants