Skip to content

std: Add Vec::from_iter comment - #22394

Merged
huonw merged 1 commit into
rust-lang:masterfrom
alexcrichton:vec-from-iter-comment
Feb 18, 2015
Merged

std: Add Vec::from_iter comment#22394
huonw merged 1 commit into
rust-lang:masterfrom
alexcrichton:vec-from-iter-comment

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

Requested by Niko in #22200 (and is good to have anyway)

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @gankro

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

@brson

Copy link
Copy Markdown
Contributor

@bors: r+ 42053 rollup y'all

Comment threadsrc/libcollections/vec.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I'm not super keen on this fuse approach. In general it adds a check to every next call, to protect against a case that is basically "unspecified behaviour" anyway (the iterator being shorter than its stated size_hint).

I'd prefer adding a check after the loop:

// the iterator ran out of elements before filling the whole vector, so we're done.if vector.len() < vector.capacity(){return vector }

since that is guaranteed to be only a single check and outside the inner loop, even if the optimiser can't remove it.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I have updated and added an appropriate comment as well.

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 17, 2015
… r=brson
Requested by Niko in rust-lang#22200 (and is good to have anyway)
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 17, 2015
… r=brson
Requested by Niko in rust-lang#22200 (and is good to have anyway)
@alexcrichton
alexcrichtonforce-pushed the vec-from-iter-comment branch 2 times, most recently from f8251e7 to bebfa78CompareFebruary 17, 2015 17:06
@alexcrichton

Copy link
Copy Markdown
MemberAuthor

It appears this landed in a rollup but then wasn't auto-closed.

re-r? @huonw

@rust-highfiverust-highfive assigned huonw and unassigned GankraFeb 17, 2015
Comment threadsrc/libcollections/vec.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/ear/era/

@huonw

Copy link
Copy Markdown
Contributor

r=me with the speeling fix.

@alexcrichton

Copy link
Copy Markdown
MemberAuthor

@bors: r+ 95a28c9

Requested by Niko in rust-lang#22200 (and is good to have anyway)
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 18, 2015
@huonw
huonw merged commit 95a28c9 into rust-lang:masterFeb 18, 2015
@alexcrichton
alexcrichton deleted the vec-from-iter-comment branch March 27, 2015 20:43
Sign up for freeto 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.

5 participants

@alexcrichton@rust-highfive@brson@huonw@Gankra