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

Remove SliceConcatExt, add them as methods on [T]. #1141

Open
bluss opened this issue May 31, 2015 · 7 comments
Open

Remove SliceConcatExt, add them as methods on [T]. #1141

bluss opened this issue May 31, 2015 · 7 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@bluss
Copy link
Member

bluss commented May 31, 2015

Remove SliceConcatExt, add them as methods on [T].

We don't need extension traits for this anymore.

@gkoz
Copy link

gkoz commented May 31, 2015

Related PR: Rename connect to join.

BTW I'd very much like Iterator to have those methods instead. I know that would be less efficient than the current implementation. Maybe impl Iterator for slice::Iter could still salvage the more efficient implementation.

@bluss
Copy link
Member Author

bluss commented Jun 12, 2015

Moving a method from an unstable trait to an inherent impl is a minor breaking change I guess.

This quirky stuff compiles because concat is on a prelude trait:

<_>::concat(&["1", "2"][..])

@frewsxcv
Copy link
Member

Here's how Servo joins strings. It's nice that it works directly with iterators to avoid unnecessary allocations

@gkoz
Copy link

gkoz commented Sep 30, 2015

@frewsxcv
Is push_str in that implementation not performing any reallocations? Ones that SliceConcatExt is avoiding?

@bluss
Copy link
Member Author

bluss commented Sep 30, 2015

Right, the slice version avoids reallocation while building the string by iterating through to sum up the exact capacity needed up front.

@frewsxcv
Copy link
Member

I meant that if I have some other data structure like a HashSet, I'd have to iterate through all the string and collect them (causing an allocation to occur I think?) before calling join on it

@bluss
Copy link
Member Author

bluss commented Sep 30, 2015

Yes. Notice that servo could use SliceConcatExt's join when it has a slice for a much more efficient join operation. It's only unfortunate that we can't offer both the efficient slice version and the less efficient iterator version in a single method call. Specialization could help.

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Aug 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

4 participants