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

impl Clone for .split_whitespace() #41659

Merged
merged 2 commits into from May 10, 2017

Conversation

Projects
None yet
10 participants
@bluss
Contributor

bluss commented Apr 30, 2017

Use custom closure structs for the predicates so that the iterator's
clone can simply be derived. This should also reduce virtual call
overhead by not using function pointers.

Fixes #41655

std_unicode: impl Clone for .split_whitespace()
Use custom closure structs for the predicates so that the iterator's
clone can simply be derived. This should also reduce virtual call
overhead by not using function pointers.
@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Apr 30, 2017

Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

Collaborator

rust-highfive commented Apr 30, 2017

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Apr 30, 2017

Contributor

It's not pretty but I think it's OK to use this long kind of implementation due to its positive features.

Contributor

bluss commented Apr 30, 2017

It's not pretty but I think it's OK to use this long kind of implementation due to its positive features.

@arielb1

This comment has been minimized.

Show comment
Hide comment
@arielb1

arielb1 May 2, 2017

Contributor

r? @aturon

Contributor

arielb1 commented May 2, 2017

r? @aturon

@arielb1 arielb1 assigned aturon and unassigned nikomatsakis May 2, 2017

@arielb1 arielb1 requested a review from aturon May 2, 2017

@arielb1

This comment has been minimized.

Show comment
Hide comment
@arielb1

arielb1 May 2, 2017

Contributor

Thanks for the PR @bluss. @aturon (or someone else) should be reviewing your PR shortly.

Contributor

arielb1 commented May 2, 2017

Thanks for the PR @bluss. @aturon (or someone else) should be reviewing your PR shortly.

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis May 2, 2017

Contributor

cc @rust-lang/libs (code seems fine to me; and I can't see any reason that this iterator shouldn't be iterable...)

Contributor

nikomatsakis commented May 2, 2017

cc @rust-lang/libs (code seems fine to me; and I can't see any reason that this iterator shouldn't be iterable...)

@alexcrichton alexcrichton added the T-libs label May 3, 2017

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 3, 2017

Member

@rfcbot fcp merge

Member

alexcrichton commented May 3, 2017

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot May 3, 2017

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot commented May 3, 2017

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon May 5, 2017

Member

LGTM. r=me once libs has signed off (new stable addition to std).

Member

aturon commented May 5, 2017

LGTM. r=me once libs has signed off (new stable addition to std).

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon May 9, 2017

Member

@bors: r+

Member

aturon commented May 9, 2017

@bors: r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 9, 2017

Contributor

📌 Commit 41aeb9d has been approved by aturon

Contributor

bors commented May 9, 2017

📌 Commit 41aeb9d has been approved by aturon

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot May 9, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot commented May 9, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

frewsxcv added a commit to frewsxcv/rust that referenced this pull request May 10, 2017

Rollup merge of #41659 - bluss:clone-split-whitespace, r=aturon
impl Clone for .split_whitespace()

Use custom closure structs for the predicates so that the iterator's
clone can simply be derived. This should also reduce virtual call
overhead by not using function pointers.

Fixes #41655

bors added a commit that referenced this pull request May 10, 2017

Auto merge of #41869 - frewsxcv:rollup, r=frewsxcv
Rollup of 11 pull requests

- Successful merges: #41531, #41536, #41659, #41684, #41764, #41815, #41843, #41861, #41862, #41863, #41864
- Failed merges:
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 10, 2017

Contributor

⌛️ Testing commit 41aeb9d with merge 58b33ad...

Contributor

bors commented May 10, 2017

⌛️ Testing commit 41aeb9d with merge 58b33ad...

bors added a commit that referenced this pull request May 10, 2017

Auto merge of #41659 - bluss:clone-split-whitespace, r=aturon
impl Clone for .split_whitespace()

Use custom closure structs for the predicates so that the iterator's
clone can simply be derived. This should also reduce virtual call
overhead by not using function pointers.

Fixes #41655
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 10, 2017

Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: aturon
Pushing 58b33ad to master...

Contributor

bors commented May 10, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: aturon
Pushing 58b33ad to master...

@bors bors merged commit 41aeb9d into rust-lang:master May 10, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@bluss bluss deleted the bluss:clone-split-whitespace branch May 10, 2017

@bluss bluss added the relnotes label May 10, 2017

gerd2002 pushed a commit to MegatonHammer/rust that referenced this pull request Apr 2, 2018

Auto merge of #41659 - bluss:clone-split-whitespace, r=aturon
impl Clone for .split_whitespace()

Use custom closure structs for the predicates so that the iterator's
clone can simply be derived. This should also reduce virtual call
overhead by not using function pointers.

Fixes #41655
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment