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

Rename to as str #17171

Closed
wants to merge 3 commits into from
Closed

Rename to as str #17171

wants to merge 3 commits into from

Conversation

richo
Copy link
Contributor

@richo richo commented Sep 11, 2014

Closes #14433

Testing is difficult (I'm on a laptop without power so builds are concerning) but stage0 builds fine with this patch.

Waiting on a stage1 now. Obviously this breaks the vast majority of code that uses String

@richo
Copy link
Contributor Author

richo commented Sep 11, 2014

I just realised that I've definitely missed a bunch of windows and linux specific code. I can stand up a test environment for linux, but will probably need something with a windows environment to help me, it will probably really suck to shotgun patch this with bors.

@huonw
Copy link
Member

huonw commented Sep 11, 2014

cc @aturon, @alexcrichton, @brson

@alexcrichton
Copy link
Member

We were talking about this with reference to the stabilization of String, and the conclusion that we reached is that while we think that as_str is probably the right name for this method, we don't necessarily want to change it right now.

Once rust-lang/rfcs#198 is implemented, then we'll likely just use foo[] to get a slice, rather than calling as_str explicitly. This means that if we make this migration now all code will have to migrate from as_slice to as_str, and then migrate from as_str to []. In the interest of less churn, we may want to hold off on a change such as this.

@richo
Copy link
Contributor Author

richo commented Sep 11, 2014

Works for me.

Edit, thinking about it, having just been through ~1000 instances of peeking at a String's underlying slice in rust/the stdlib it seems like a great deal of it is either not indexing it, or indexing it elsewhere (eg, it either passes the slice into something else, or returns the slice). That said, I'm not against shelving this and dealing with it down the road.

@pcwalton
Copy link
Contributor

@alexcrichton Wait, did that just implicitly make slice syntax a 1.0 blocker? We should discuss that and make a decision explicitly...

@alexcrichton
Copy link
Member

Slice notation has since landed, but with rust-lang/rfcs#235 landing soon it should hopefully reduce the need for lots of .as_slice() (or .as_str()) in the first place. In the meantime I'm going to close this (it's been inactive for awhile), but I think we should start making more progress once the RFC has landed and we have Deref for String/Vec implemented to start lessening the need for .as_slice() and help with the merge conflicts.

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.

core::str::Str.as_slice() should be renamed as_str()
4 participants