[hashmap] Adds `drain`: a way to sneak out the elements while clearing a map. #19946

cgaebel · 2014-12-17T03:33:35Z

It is useful to move all the elements out of a hashmap without deallocating
the underlying buffer. It came up in IRC, and this patch implements it as
drain.

r? @gankro
cc: @frankmcsherry

alexcrichton · 2014-12-17T04:41:06Z

Let's please hold off merging this until rust-lang/rfcs#509 is merged as well (but review is fine!)

pczarn · 2014-12-17T16:28:47Z

src/libstd/collections/hash/table.rs

nit: debug_assert_eq!

reem · 2014-12-17T17:11:31Z

cc @aturon this is what we discussed on IRC yesterday.

cgaebel · 2014-12-18T03:19:20Z

cc @thestinger for Vec changes.

RingBuf/Vec .drain has been added if y'all want to review that too!

Gankra · 2014-12-18T03:21:44Z

src/libcollections/ring_buf.rs

cgaebel · 2014-12-18T03:40:03Z

BinaryHeap is done now, too. Not sure who "owns" that (and ringbuf too, for that matter), but someone should cc them.

cgaebel · 2014-12-18T03:46:46Z

src/libcollections/vec.rs

I don't think this deprecation is in the RFC. It should probably be added.

Unsure if this is strictly desirable. MoveItems in theory lets you move the collection to somewhere else by passing around an iterator.

Yeah that's why I'm thinking it should be added to the RFC so people can debate it.

Gankra · 2014-12-18T03:48:26Z

RingBuf is primarily authored by @csherratt. BinaryHeap hasn't really had any heavy work done on it in a while. Pretty simple collection relatively speaking.

sinistersnare · 2014-12-18T03:58:53Z

src/libcollections/binary_heap.rs

s/BinarHeap/BinaryHeap

Gankra · 2014-12-18T04:02:33Z

I take it back, @apasel422 is in charge of BinaryHeap forever now.

ghost · 2014-12-18T05:42:53Z

src/libcollections/ring_buf.rs

This should be ///

cgaebel · 2014-12-18T22:59:48Z

This has been rebased, and I believe all review comments have been addressed.

r? @gankro

ghost · 2014-12-18T23:59:33Z

The changes to ringbuf LGTM.

Gankra · 2014-12-19T00:43:45Z

src/libcollections/binary_heap.rs

Word on high from lord @aturon is that these should be called Drain (as Iterators should be named after their creation method -- Iter, IterMut, IntoIter.

Gankra · 2014-12-19T00:51:47Z

src/libcollections/ring_buf.rs

Awkward sentence. Either have a period after iterator, or just have "... iterator that clears the..."

Gankra · 2014-12-19T01:54:49Z

src/libstd/collections/hash/map.rs

All the other docs in this file say "Example", but in other files it says "Examples" even when there's only one test.

New convention is examples per the rule of @steveklabnik; anything else is out of date.

Even when the english language disagrees? afaik singular forms of nouns should be used when there's only one.

Gankra · 2014-12-19T02:28:00Z

Ok, done my full pass.

cgaebel · 2014-12-19T02:57:48Z

Moi aussi.

Gankra · 2014-12-19T03:14:04Z

r=me with fixed doc + squash

@frankmcsherry

…aring. It is useful to move all the elements out of some collections without deallocating the underlying buffer. It came up in IRC, and this patch implements it as `drain`. This has been discussed as part of RFC 509. r? @gankro cc: @frankmcsherry

cgaebel · 2014-12-19T03:17:45Z

@frankmcsherry

It is useful to move all the elements out of a hashmap without deallocating the underlying buffer. It came up in IRC, and this patch implements it as `drain`. r? @gankro cc: @frankmcsherry

pczarn reviewed Dec 17, 2014
View reviewed changes

src/libstd/collections/hash/table.rs Outdated

Copy link

Contributor

pczarn Dec 17, 2014

Choose a reason for hiding this comment

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

nit: debug_assert_eq!

Gankra reviewed Dec 18, 2014
View reviewed changes

src/libcollections/ring_buf.rs

Copy link

Contributor

Gankra Dec 18, 2014

Choose a reason for hiding this comment

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

<3

cgaebel reviewed Dec 18, 2014
View reviewed changes

sinistersnare reviewed Dec 18, 2014
View reviewed changes

src/libcollections/binary_heap.rs Outdated

Copy link

Contributor

sinistersnare Dec 18, 2014

Choose a reason for hiding this comment

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

s/BinarHeap/BinaryHeap

ghost reviewed Dec 18, 2014
View reviewed changes

src/libcollections/ring_buf.rs Outdated

Copy link

ghost Dec 18, 2014

Choose a reason for hiding this comment

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

This should be ///

cgaebel force-pushed the hashmap-drain-iter branch from 0c721de to fcfbefb Compare December 18, 2014 22:58

rust-highfive assigned Gankra Dec 18, 2014

cgaebel force-pushed the hashmap-drain-iter branch from fcfbefb to 9ccd998 Compare December 18, 2014 23:01

Gankra reviewed Dec 19, 2014
View reviewed changes

cgaebel force-pushed the hashmap-drain-iter branch from 9ccd998 to 0925212 Compare December 19, 2014 00:50

Gankra reviewed Dec 19, 2014
View reviewed changes

cgaebel force-pushed the hashmap-drain-iter branch from 992eeef to d57f259 Compare December 19, 2014 03:16

bors merged commit d57f259 into rust-lang:master Dec 21, 2014

[hashmap] Adds drain: a way to sneak out the elements while clearing a map. #19946

[hashmap] Adds drain: a way to sneak out the elements while clearing a map. #19946

Uh oh!

Conversation

cgaebel commented Dec 17, 2014

Uh oh!

alexcrichton commented Dec 17, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reem commented Dec 17, 2014

Uh oh!

cgaebel commented Dec 18, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgaebel commented Dec 18, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gankra commented Dec 18, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gankra commented Dec 18, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgaebel commented Dec 18, 2014

Uh oh!

ghost commented Dec 18, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gankra commented Dec 19, 2014

Uh oh!

cgaebel commented Dec 19, 2014

Uh oh!

Gankra commented Dec 19, 2014

Uh oh!

cgaebel commented Dec 19, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

[hashmap] Adds `drain`: a way to sneak out the elements while clearing a map. #19946

[hashmap] Adds `drain`: a way to sneak out the elements while clearing a map. #19946