Fixes nits in string guide #17453
Fixes nits in string guide #17453
Conversation
| } | ||
| ``` | ||
|
|
||
| This is prefereable because it can save you allocating copies of what's inside. |
There was a problem hiding this comment.
s/prefereable/preferable/
There was a problem hiding this comment.
@EduardoBautista fixed, thanks
@huonw i'm not sure what you mean, can you give me a better diff :)
There was a problem hiding this comment.
"this is preferable because it can save you allocating extra storage to be able to call the function." (You're not necessarily allocating copies of the contents, e.g. &[&str] just needs to allocate a Vec<&str> with pointers into the old one).
|
I threw up http://steveklabnik.github.io/rust-issue-17340/ , but it seems it doesn't actually link to the css, @huonw . Any idea whats up? |
|
You need to compile like the makefiles do, they insert some headers via rustdoc flags (the easiest way is probably to just compile normally and upload that, but you can also look in |
|
(Oh, looks like it's |
|
Done! |
| ```{rust} | ||
| fn sum_of_string_length<T: Str>(xs: &[T]) -> uint { | ||
| xs.iter().fold(0, |acc, x| acc + x.as_slice().len()) | ||
| } |
There was a problem hiding this comment.
This is actually a pattern that we have been discouraging in the past due to the Str trait not quite being what you'd expect, as well as not being quite designed for this purpose. The case of a &[String] vs &[&str] is a little different, but in general if you're using a string and you don't want ownership you should always use &str.
Right now the ergonomics aren't quite where they could be, but there are a few RFCs open about the auto-borrowing issue which should help here.
cc @aturon, do you think something like this should be in the guide for now?
There was a problem hiding this comment.
Autoborrowing will never solve &[String] vs. &[&str] since they have different representations.
(Also, this sort of generic code was actually the reason that I added the Str trait.)
There was a problem hiding this comment.
I completely agree with @alexcrichton: idiomatic Rust should take &str values, not bound by Str. The ergonomics are going to be improved one way or another before 1.0. The guide should recommend &str.
There was a problem hiding this comment.
@huonw is true that &[String] vs &[&str] will not be solved with autoborrowing, but it's still true that this is not what Str is designed for right now.
There was a problem hiding this comment.
@aturon note the context here: if you have a &[String], the only way to call a &[&str] function is to allocate a Vec<&str> and place a pile of slices into it.
There was a problem hiding this comment.
That said, most instances of this are probably able to take an Iterator<&str>... which that also has flexibility problems, e.g. a Iterator<String> cannot become an Iterator<&str> without collecting to a Vec. On the other hand, collecting to a Vec is always required to call a &[...] function with an Iterator so taking an Iterator<&str> is still a strict improvement.
There was a problem hiding this comment.
@huonw, ah I see; I had missed that.
Even so, bounding by Str seems a bit unfortunate somehow -- I think because it seems a bit ad hoc/overly specific.
I actually wonder if Borrow is the more general concept lurking here? I.e., maybe you want to take &[T] where T: Borrow<str>? Then the same solution would work for vectors (and eventually other sliceable types).
There was a problem hiding this comment.
@alexcrichton &str doesn't need an as_slice method, except for use in generic code. :)
There was a problem hiding this comment.
@aturon yeah, maybe Borrow/Deref are what we want.
(However, I do think that this is rare enough that we can avoid mentioning it in the guides until we've got our story straight.)
There was a problem hiding this comment.
@huonw re: Borrow/Deref distinction, see rust-lang/rfcs#235 (comment)
|
What's the status on on this? |
|
I posted the in-progress version, waiting on @huonw 's feedback. |
|
I'm fine with it as it is, although I'm under the impression that @aturon and @alexcrichton would prefer that the generic |
|
Is that the conclusion here, @aturon and @alexcrichton ? I can do that if that's the path we want to go down. |
|
Yeah let's remove the generic |
|
Cool, i'll do that as soon as this |
|
Removed, rebased, and squashed. |
/cc @huonw