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

Implement ordered/sorted iterators on BinaryHeap as per #59278 #65091

Merged
merged 4 commits into from
Oct 31, 2019

Conversation

sekineh
Copy link
Contributor

@sekineh sekineh commented Oct 4, 2019

I've implemented the ordered version of iterator on BinaryHeap as per #59278.

Added methods:

  • .into_iter_sorted()
    • like .into_iter(); but returns elements in heap order
  • .drain_sorted()
    • like .drain(); but returns elements in heap order
    • It's a bit lazy; elements are removed on drop. (Edit: it’s similar to vec::Drain)

For DrainSorted struct, I implemented Drop trait following @scottmcm 's suggestion

TODO DONE

  • I think I need to add more tests other than doctest.

Notes:

  • we renamed _ordered to _sorted, because the latter is more common in rust libs. (as suggested by @KodrAus )

@rust-highfive
Copy link
Collaborator

r? @KodrAus

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 4, 2019
impl<T: Ord> ExactSizeIterator for IntoIterOrdered<T> {}

#[unstable(feature = "binary_heap_into_iter_ordered", issue = "59278")]
impl<T: Ord> FusedIterator for IntoIterOrdered<T> {}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can add TrustedLen for both.

Copy link
Contributor Author

@sekineh sekineh Oct 7, 2019

Choose a reason for hiding this comment

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

I'll add TrustedLen for the two new iterators.

I'm wondering if other existing iterators should implement TrustedLen for consistency.

  • binary_heap::Iter
  • binary_heap::IntoIter
    • Underlying vec::IntoIter implements TrustedLen
    • Clearly binary_heap::IntoIter should implement TrustedLen, too.
  • binary_heap::Drain
    • Underlying vec::Drain does not implement TrustedLen.
    • It is not clear whether it should implement TrustedLen.
  • vec::Drain
    • It is not clear whether it should implement TrustedLen.

Please advise.

Copy link
Contributor

Choose a reason for hiding this comment

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

They should implement TrustedLen if and only if it would be sound to do so. However, I think it's best to make a follow-up issue and treat with them there.

Copy link
Contributor Author

@sekineh sekineh Oct 7, 2019

Choose a reason for hiding this comment

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

I see, I go with the conservative way. (Implement it for the two new ordered iterators only)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TrustedLen was added.


#[unstable(feature = "binary_heap_drain_ordered", issue = "59278")]
impl<'a, T> Drop for DrainOrdered<'a, T> {
/// Removes heap elements in arbitrary order for efficiency.
Copy link
Member

Choose a reason for hiding this comment

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

I think it should clear them in order, it seems surprising otherwise. The common theme of the .drain() iterators are that they have sideeffectful drops, so it wouldn't be out of place? It can be guarded by a needs-drop check?

Copy link
Member

@scottmcm scottmcm Oct 4, 2019

Choose a reason for hiding this comment

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

That has complexity side-effects I'd consider surprising, though -- I'd hope that .drain().take(k) is O(k lg n), but if they have to drop in order, it'd be O(n lg n).

The BinaryHeap itself drops in arbitrary order, so I don't think it's crazy for drain to also do so. If someone really wants to drop in sorted order, then can always .for_each(drop).

EDIT: Or worse, if the element is !needs_drop, the drop being O(n ln n) instead of O(1) seems particularly unfortunate.

Copy link
Member

Choose a reason for hiding this comment

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

In one way it seems to me that .drain_ordered() already is an intention of taking all the elements and dropping them in order. But I think your expectation there is fair.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like expensive operation on implicit call (such as drop()) is not a good thing in the Rust language.
Therefore, I kept the behavior requiring user to explicitly consume elements to perform expensive O(n ln n) operation.

Copy link
Contributor Author

@sekineh sekineh Oct 25, 2019

Choose a reason for hiding this comment

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

Now .drain_sorted() is changed to always remove elements in heap order as @bluss has suggested.

I suppose this simplifies usage a lot. The previous behavior was hard to explain in the example. "hard to explain" feels like a possible sign of "bad API" and I changed it to the more straightforward implementation.

@sekineh sekineh changed the title [WIP] Implement ordered iterators on BinaryHeap as per #59278 Implement ordered iterators on BinaryHeap as per #59278 Oct 7, 2019
@sekineh
Copy link
Contributor Author

sekineh commented Oct 7, 2019

Thanks for comments! I removed [WIP] status from the PR title. Waiting for CI now.

Copy link
Contributor

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @sekineh!

I've just left a few comments.

/// assert_eq!(heap.into_iter_ordered().take(2).collect::<Vec<_>>(), vec![5, 4]);
/// ```
#[unstable(feature = "binary_heap_into_iter_ordered", issue = "59278")]
pub fn into_iter_ordered(self) -> IntoIterOrdered<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the naming convention we have elsewhere would call this into_iter_sorted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, that sorted is more common in std.
done in 4da6540

/// ```
#[inline]
#[unstable(feature = "binary_heap_drain_ordered", issue = "59278")]
pub fn drain_ordered(&mut self) -> DrainOrdered<'_, T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

drain_sorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 4da6540

/// Note:
/// * Unlike other `.drain()` methods, this method removes elements *lazily*.
/// In order to remove elements in heap order, you need to retrieve elements explicitly.
/// * The remained elements are removed on drop (out of scope), in arbitrary order for
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could just say here The remaining elements are removed on drop in arbitrary order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done in 38c77ae

@sekineh sekineh changed the title Implement ordered iterators on BinaryHeap as per #59278 Implement ordered/sorted iterators on BinaryHeap as per #59278 Oct 10, 2019
/// The retrieved elements will be removed from the original heap.
///
/// Note:
/// * Unlike other `.drain()` methods, this method removes elements *lazily*.
Copy link
Contributor

Choose a reason for hiding this comment

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

Having drain_sorted behave differently than other drain methods with respect to optimistically clearing capacity seems unfortunate. Combined with the inconsistent order of any remaining elements in drop it's looking like a method that isn't going to behave the way you expect unless you hold it a specific way.

I feel like we'll need a few examples that are at least close to some real-world usage to get a sense of whether these trade-offs really are reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In 30e8f65, I made .drain_sorted() to always remove elements in heap order.
This simplified doc example a lot.

@bors
Copy link
Contributor

bors commented Oct 17, 2019

☔ The latest upstream changes (presumably #65251) made this pull request unmergeable. Please resolve the merge conflicts.

@wirelessringo
Copy link

Ping from triage. @sekineh any updates on this? Thanks.

@rustbot modify labels to +S-waiting-on-author, -S-waiting-on-review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 18, 2019
@sekineh
Copy link
Contributor Author

sekineh commented Oct 19, 2019

I’ll rebase and work it in the next week.

@sekineh
Copy link
Contributor Author

sekineh commented Oct 25, 2019

(I used git in some wrong way when I tried to do git rebase. trying to fix now)
-> Edit: fix is done.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-10-25T06:01:47.6456442Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-25T06:01:47.6595833Z ##[command]git config gc.auto 0
2019-10-25T06:01:47.6670698Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-25T06:01:47.6722671Z ##[command]git config --get-all http.proxy
2019-10-25T06:01:47.6852356Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65091/merge:refs/remotes/pull/65091/merge
---
2019-10-25T06:57:42.5544196Z .................................................................................................... 1600/9244
2019-10-25T06:57:47.8605470Z .................................................................................................... 1700/9244
2019-10-25T06:57:59.7583567Z ......................................................i...............i............................. 1800/9244
2019-10-25T06:58:07.1507969Z .................................................................................................... 1900/9244
2019-10-25T06:58:20.7039358Z ............................................iiiii................................................... 2000/9244
2019-10-25T06:58:30.6445347Z .................................................................................................... 2200/9244
2019-10-25T06:58:33.0354658Z .................................................................................................... 2300/9244
2019-10-25T06:58:36.8012707Z .................................................................................................... 2400/9244
2019-10-25T06:58:58.2602896Z .................................................................................................... 2500/9244
2019-10-25T06:58:58.2602896Z .................................................................................................... 2500/9244
2019-10-25T06:59:00.7105713Z .................................................................................................... 2600/9244
2019-10-25T06:59:08.4592169Z .................................................................................................... 2700/9244
2019-10-25T06:59:17.5387792Z ...............i.................................................................................... 2800/9244
2019-10-25T06:59:25.3328049Z .................................................................................................... 2900/9244
2019-10-25T06:59:29.5636004Z ...............i.................................................................................... 3000/9244
2019-10-25T06:59:38.3476385Z .................................................................................................... 3100/9244
2019-10-25T06:59:42.9517275Z ................................................................................................ii.. 3200/9244
2019-10-25T06:59:59.8087993Z .................................................................................................... 3400/9244
2019-10-25T07:00:07.8397444Z ..........................................................................................i......... 3500/9244
2019-10-25T07:00:14.8731890Z .....................................i.............................................................. 3600/9244
2019-10-25T07:00:20.9575668Z .................................................................................................... 3700/9244
---
2019-10-25T07:01:41.7035060Z ................................................i...............i................................... 4800/9244
2019-10-25T07:01:50.5470084Z .................................................................................................... 4900/9244
2019-10-25T07:01:58.7593970Z .................................................................................................... 5000/9244
2019-10-25T07:02:05.4140494Z .................................................................................................... 5100/9244
2019-10-25T07:02:15.3737064Z .................................................ii.ii.............................................. 5200/9244
2019-10-25T07:02:25.3350810Z .................................................................................................... 5400/9244
2019-10-25T07:02:34.4272321Z .................................................................................................... 5500/9244
2019-10-25T07:02:42.2207998Z ................i................................................................................... 5600/9244
2019-10-25T07:02:47.9709842Z .................................................................................................... 5700/9244
2019-10-25T07:02:47.9709842Z .................................................................................................... 5700/9244
2019-10-25T07:03:00.2702261Z .................................................................................................... 5800/9244
2019-10-25T07:03:12.0114168Z .............ii...i..ii...........i................................................................. 5900/9244
2019-10-25T07:03:33.3674778Z .................................................................................................... 6100/9244
2019-10-25T07:03:40.5823995Z .................................................................................................... 6200/9244
2019-10-25T07:03:40.5823995Z .................................................................................................... 6200/9244
2019-10-25T07:03:53.5278261Z ...................................i..ii............................................................ 6300/9244
2019-10-25T07:04:14.1421312Z .................................................................................................... 6500/9244
2019-10-25T07:04:16.3745763Z .i.................................................................................................. 6600/9244
2019-10-25T07:04:18.6724995Z ............................................................................i....................... 6700/9244
2019-10-25T07:04:21.4266474Z .................................................................................................... 6800/9244
---
2019-10-25T07:08:44.0368246Z  finished in 5.353
2019-10-25T07:08:44.0556581Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T07:08:44.2350803Z 
2019-10-25T07:08:44.2351531Z running 153 tests
2019-10-25T07:08:47.3269778Z i....iii......iii..iiii...i.............................i..i..................i....i...........ii.i. 100/153
2019-10-25T07:08:49.2191341Z .i.iiii..............i.........iii.i.........ii......
2019-10-25T07:08:49.2191915Z 
2019-10-25T07:08:49.2198280Z  finished in 5.164
2019-10-25T07:08:49.2387701Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T07:08:49.3953020Z 
---
2019-10-25T07:08:51.3839895Z  finished in 2.145
2019-10-25T07:08:51.4027612Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T07:08:51.5666151Z 
2019-10-25T07:08:51.5668660Z running 9 tests
2019-10-25T07:08:51.5669392Z iiiiiiiii
2019-10-25T07:08:51.5669733Z 
2019-10-25T07:08:51.5669771Z  finished in 0.163
2019-10-25T07:08:51.5859180Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T07:08:51.7751020Z 
---
2019-10-25T07:09:09.5042216Z  finished in 17.918
2019-10-25T07:09:09.8427507Z Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T07:09:10.0634016Z 
2019-10-25T07:09:10.0677483Z running 123 tests
2019-10-25T07:09:33.2261119Z .iiiii...i.....i..i...i..i.i.i..i.ii..i.i.....i..i....ii..........iiii..........i...ii...i.......ii. 100/123
2019-10-25T07:09:37.6761395Z i.i.i......iii.i.....ii
2019-10-25T07:09:37.6761807Z 
2019-10-25T07:09:37.6765489Z  finished in 27.833
2019-10-25T07:09:37.6771575Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T07:09:37.6771845Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2019-10-25T07:18:27.6733353Z    Compiling alloc v0.0.0 (/checkout/src/liballoc)
2019-10-25T07:18:28.5636519Z error: unused imports: `AssertUnwindSafe`, `self`
2019-10-25T07:18:28.5671063Z  --> src/liballoc/../liballoc/tests/binary_heap.rs:3:18
2019-10-25T07:18:28.5718141Z   |
2019-10-25T07:18:28.5719147Z 3 | use std::panic::{self, AssertUnwindSafe};
2019-10-25T07:18:28.5719505Z   |                  ^^^^  ^^^^^^^^^^^^^^^^
2019-10-25T07:18:28.5720306Z   = note: `-D unused-imports` implied by `-D warnings`
2019-10-25T07:18:28.5720362Z 
2019-10-25T07:18:28.5720362Z 
2019-10-25T07:18:28.5720673Z error: unused imports: `AtomicUsize`, `Ordering`
2019-10-25T07:18:28.5721389Z   |
2019-10-25T07:18:28.5721783Z 4 | use std::sync::atomic::{AtomicUsize, Ordering};
2019-10-25T07:18:28.5722058Z   |                         ^^^^^^^^^^^  ^^^^^^^^
2019-10-25T07:18:28.5722104Z 
2019-10-25T07:18:28.5722104Z 
2019-10-25T07:18:28.5722308Z error: unused import: `thread_rng`
2019-10-25T07:18:28.5722721Z   |
2019-10-25T07:18:28.5722721Z   |
2019-10-25T07:18:28.5722953Z 7 | use rand::{thread_rng, seq::SliceRandom};
2019-10-25T07:18:28.5723262Z 
2019-10-25T07:18:28.5723262Z 
2019-10-25T07:18:32.7724230Z error: unused import: `seq::SliceRandom`
2019-10-25T07:18:32.7724845Z   |
2019-10-25T07:18:32.7724845Z   |
2019-10-25T07:18:32.7725096Z 7 | use rand::{thread_rng, seq::SliceRandom};
2019-10-25T07:18:32.7725784Z 
2019-10-25T07:18:32.7739875Z error: aborting due to 4 previous errors
2019-10-25T07:18:32.7739936Z 
2019-10-25T07:18:32.8295828Z error: could not compile `alloc`.
---
2019-10-25T07:18:37.3383873Z   local time: Fri Oct 25 07:18:37 UTC 2019
2019-10-25T07:18:37.9432106Z   network time: Fri, 25 Oct 2019 07:18:37 GMT
2019-10-25T07:18:37.9433358Z == end clock drift check ==
2019-10-25T07:18:39.1238309Z 
2019-10-25T07:18:39.1359552Z ##[error]Bash exited with code '1'.
2019-10-25T07:18:39.1394286Z ##[section]Starting: Checkout
2019-10-25T07:18:39.1395742Z ==============================================================================
2019-10-25T07:18:39.1395804Z Task         : Get sources
2019-10-25T07:18:39.1395855Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

* `.drain_sorted()` doc change suggested by @KodrAus
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-10-25T08:10:16.1731451Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-25T08:10:16.9643138Z ##[command]git config gc.auto 0
2019-10-25T08:10:16.9650162Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-25T08:10:16.9651711Z ##[command]git config --get-all http.proxy
2019-10-25T08:10:16.9655063Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65091/merge:refs/remotes/pull/65091/merge
---
2019-10-25T09:09:45.8871760Z .................................................................................................... 1600/9249
2019-10-25T09:09:51.0402240Z .................................................................................................... 1700/9249
2019-10-25T09:10:03.0844713Z ........................................................i...............i........................... 1800/9249
2019-10-25T09:10:10.7645635Z .................................................................................................... 1900/9249
2019-10-25T09:10:25.0160627Z ..............................................iiiii................................................. 2000/9249
2019-10-25T09:10:35.8204967Z .................................................................................................... 2200/9249
2019-10-25T09:10:38.3833645Z .................................................................................................... 2300/9249
2019-10-25T09:10:42.3141451Z .................................................................................................... 2400/9249
2019-10-25T09:11:05.0851973Z .................................................................................................... 2500/9249
---
2019-10-25T09:13:55.1331001Z ..................................................i...............i................................. 4800/9249
2019-10-25T09:14:03.6339141Z .................................................................................................... 4900/9249
2019-10-25T09:14:12.2413828Z .................................................................................................... 5000/9249
2019-10-25T09:14:18.3565486Z .................................................................................................... 5100/9249
2019-10-25T09:14:28.5838722Z ...................................................ii.ii............................................ 5200/9249
2019-10-25T09:14:38.6523731Z .................................................................................................... 5400/9249
2019-10-25T09:14:47.6560966Z .................................................................................................... 5500/9249
2019-10-25T09:14:55.2389443Z ..................i................................................................................. 5600/9249
2019-10-25T09:15:00.6902789Z .................................................................................................... 5700/9249
2019-10-25T09:15:00.6902789Z .................................................................................................... 5700/9249
2019-10-25T09:15:12.6515700Z .................................................................................................... 5800/9249
2019-10-25T09:15:23.9352187Z ...............ii...i..ii...........i............................................................... 5900/9249
2019-10-25T09:15:45.4953002Z .................................................................................................... 6100/9249
2019-10-25T09:15:53.9015795Z .................................................................................................... 6200/9249
2019-10-25T09:15:53.9015795Z .................................................................................................... 6200/9249
2019-10-25T09:16:06.3491447Z ......................................i..ii......................................................... 6300/9249
2019-10-25T09:16:28.7581298Z .................................................................................................... 6500/9249
2019-10-25T09:16:31.0243458Z ....i............................................................................................... 6600/9249
2019-10-25T09:16:33.3488150Z ...............................................................................i.................... 6700/9249
2019-10-25T09:16:35.9903008Z .................................................................................................... 6800/9249
---
2019-10-25T09:21:03.8264001Z  finished in 5.797
2019-10-25T09:21:03.8463095Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T09:21:04.0147986Z 
2019-10-25T09:21:04.0148964Z running 153 tests
2019-10-25T09:21:07.1949917Z i....iii......iii..iiii...i.............................i..i..................i....i...........ii.i. 100/153
2019-10-25T09:21:09.1651045Z i..iiii..............i.........iii.i.........ii......
2019-10-25T09:21:09.1652884Z 
2019-10-25T09:21:09.1657157Z  finished in 5.319
2019-10-25T09:21:09.1842865Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T09:21:09.3355735Z 
---
2019-10-25T09:21:12.1982723Z  finished in 2.118
2019-10-25T09:21:12.1983078Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T09:21:12.1983294Z 
2019-10-25T09:21:12.1983362Z running 9 tests
2019-10-25T09:21:12.1983664Z iiiiiiiii
2019-10-25T09:21:12.1983976Z 
2019-10-25T09:21:12.1988255Z  finished in 0.168
2019-10-25T09:21:12.1988736Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T09:21:12.1988777Z 
---
2019-10-25T09:21:30.1245915Z  finished in 18.610
2019-10-25T09:21:30.1449189Z Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T09:21:31.1903919Z 
2019-10-25T09:21:31.1904913Z running 123 tests
2019-10-25T09:21:53.2703163Z .iiiii...i.....i..i...i..i.i.i..i.ii..i.i.....i..i....ii..........iiii..........i...ii...i.......ii. 100/123
2019-10-25T09:21:57.6819566Z i.i.i......iii.i.....ii
2019-10-25T09:21:57.6821886Z 
2019-10-25T09:21:57.6822187Z  finished in 27.537
2019-10-25T09:21:57.6830809Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-10-25T09:21:57.6831476Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2019-10-25T09:31:07.4466178Z    Compiling alloc v0.0.0 (/checkout/src/liballoc)
2019-10-25T09:31:08.3752168Z error: unused imports: `AssertUnwindSafe`, `self`
2019-10-25T09:31:08.3752754Z  --> src/liballoc/../liballoc/tests/binary_heap.rs:3:18
2019-10-25T09:31:08.3753044Z   |
2019-10-25T09:31:08.3756758Z 3 | use std::panic::{self, AssertUnwindSafe};
2019-10-25T09:31:08.3757050Z   |                  ^^^^  ^^^^^^^^^^^^^^^^
2019-10-25T09:31:08.3757500Z   = note: `-D unused-imports` implied by `-D warnings`
2019-10-25T09:31:08.3757532Z 
2019-10-25T09:31:08.3757532Z 
2019-10-25T09:31:08.3772014Z error: unused imports: `AtomicUsize`, `Ordering`
2019-10-25T09:31:08.3776232Z   |
2019-10-25T09:31:08.3776517Z 4 | use std::sync::atomic::{AtomicUsize, Ordering};
2019-10-25T09:31:08.3776987Z   |                         ^^^^^^^^^^^  ^^^^^^^^
2019-10-25T09:31:08.3777047Z 
2019-10-25T09:31:08.3777047Z 
2019-10-25T09:31:08.3777597Z error: unused import: `thread_rng`
2019-10-25T09:31:08.3805243Z   |
2019-10-25T09:31:08.3805243Z   |
2019-10-25T09:31:08.3805489Z 7 | use rand::{thread_rng, seq::SliceRandom};
2019-10-25T09:31:08.3805791Z 
2019-10-25T09:31:08.3805791Z 
2019-10-25T09:31:13.2523770Z error: unused import: `seq::SliceRandom`
2019-10-25T09:31:13.2525188Z   |
2019-10-25T09:31:13.2525188Z   |
2019-10-25T09:31:13.2525493Z 7 | use rand::{thread_rng, seq::SliceRandom};
2019-10-25T09:31:13.2525861Z 
2019-10-25T09:31:13.2542745Z error: aborting due to 4 previous errors
2019-10-25T09:31:13.2542827Z 
2019-10-25T09:31:13.3300534Z error: could not compile `alloc`.
---
2019-10-25T09:31:17.9554682Z   local time: Fri Oct 25 09:31:17 UTC 2019
2019-10-25T09:31:18.1052667Z   network time: Fri, 25 Oct 2019 09:31:18 GMT
2019-10-25T09:31:18.1057804Z == end clock drift check ==
2019-10-25T09:31:19.7431138Z 
2019-10-25T09:31:19.7536703Z ##[error]Bash exited with code '1'.
2019-10-25T09:31:19.7575782Z ##[section]Starting: Checkout
2019-10-25T09:31:19.7577646Z ==============================================================================
2019-10-25T09:31:19.7577703Z Task         : Get sources
2019-10-25T09:31:19.7577751Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@sekineh
Copy link
Contributor Author

sekineh commented Oct 26, 2019

Now I think it’s ready to be reviewed again.

@KodrAus
Copy link
Contributor

KodrAus commented Oct 31, 2019

Alrighty, this is looking good to me now! Thanks for working on this @sekineh!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 31, 2019

📌 Commit 95442ae has been approved by KodrAus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 31, 2019
@bors
Copy link
Contributor

bors commented Oct 31, 2019

⌛ Testing commit 95442ae with merge aa4e57c...

bors added a commit that referenced this pull request Oct 31, 2019
Implement ordered/sorted iterators on BinaryHeap as per #59278

I've implemented the ordered version of iterator on BinaryHeap as per #59278.

# Added methods:

* `.into_iter_sorted()`
  * like `.into_iter()`; but returns elements in heap order
* `.drain_sorted()`
  * like `.drain()`; but returns elements in heap order
  * It's a bit _lazy_; elements are removed on drop. (Edit: it’s similar to vec::Drain)

For `DrainSorted` struct, I implemented `Drop` trait following @scottmcm 's [suggestion](#59278 (comment))

# ~TODO~ DONE
* ~I think I need to add more tests other than doctest.~

# **Notes:**
* we renamed `_ordered` to `_sorted`, because the latter is more common in rust libs. (as suggested by @KodrAus )
@bors
Copy link
Contributor

bors commented Oct 31, 2019

☀️ Test successful - checks-azure
Approved by: KodrAus
Pushing aa4e57c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 31, 2019
@bors bors merged commit 95442ae into rust-lang:master Oct 31, 2019
@sekineh sekineh deleted the into-iter-sorted branch November 8, 2019 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants