From aae9f46dab610b47a432486d44d1ff1b97488143 Mon Sep 17 00:00:00 2001 From: Tim Sviridov Date: Fri, 11 Dec 2015 00:35:48 +1100 Subject: [PATCH 1/2] Improve BinaryHeap's handling of ranges --- std/container/binaryheap.d | 71 +++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/std/container/binaryheap.d b/std/container/binaryheap.d index 6886ab74b8a..c015226ecdb 100644 --- a/std/container/binaryheap.d +++ b/std/container/binaryheap.d @@ -115,11 +115,26 @@ if (isRandomAccessRange!(Store) || isRandomAccessRange!(typeof(Store.init[]))) { assert(!store.empty, "Cannot pop an empty store."); if (store.length == 1) return; - auto t1 = moveFront(store[]); - auto t2 = moveBack(store[]); + static if(isRandomAccessRange!Store) + { + auto t1 = moveFront(store); + auto t2 = moveBack(store); + } + else + { + auto t1 = moveFront(store[]); + auto t2 = moveBack(store[]); + } store.front = move(t2); store.back = move(t1); - percolate(store[], 0, store.length - 1); + static if(isRandomAccessRange!Store) + { + percolate(store, 0, store.length - 1); + } + else + { + percolate(store[], 0, store.length - 1); + } } public: @@ -147,7 +162,10 @@ the heap work incorrectly. _store = move(s); _length = min(_store.length, initialSize); if (_length < 2) return; - buildHeap(s[]); + static if (isRandomAccessRange!Store) + buildHeap(s); + else + buildHeap(s[]); assertValid(); } @@ -293,13 +311,28 @@ Removes the largest element from the heap. enforce(!empty, "Cannot call removeFront on an empty heap."); if (_length > 1) { - auto t1 = moveFront(_store[]); - auto t2 = moveAt(_store[], _length - 1); + static if(isRandomAccessRange!Store) + { + auto t1 = moveFront(_store); + auto t2 = moveAt(_store, _length - 1); + } + else + { + auto t1 = moveFront(_store[]); + auto t2 = moveAt(_store[], _length - 1); + } _store.front = move(t2); _store[_length - 1] = move(t1); } --_length; - percolate(_store[], 0, _length); + static if(isRandomAccessRange!Store) + { + percolate(_store, 0, _length); + } + else + { + percolate(_store[], 0, _length); + } } /// ditto @@ -325,7 +358,14 @@ Replaces the largest element in the store with $(D value). // must replace the top assert(!empty, "Cannot call replaceFront on an empty heap."); _store.front = value; - percolate(_store[], 0, _length); + static if(isRandomAccessRange!Store) + { + percolate(_store, 0, _length); + } + else + { + percolate(_store[], 0, _length); + } debug(BinaryHeap) assertValid(); } @@ -349,7 +389,10 @@ must be collected. assert(!_store.empty, "Cannot replace front of an empty heap."); if (!comp(value, _store.front)) return false; // value >= largest _store.front = value; - percolate(_store[], 0, _length); + static if (isRandomAccessRange!Store) + percolate(_store, 0, _length); + else + percolate(_store[], 0, _length); debug(BinaryHeap) assertValid(); return true; } @@ -427,3 +470,13 @@ unittest static assert(isInputRange!(typeof(h))); assert(h.equal([16, 14, 10, 9, 8, 7, 4, 3, 2, 1])); } + +unittest +{ + import std.internal.test.dummytest; + foreach(DummyRange; AllDummyRanges) + { + auto a = [4, 1, 3, 2, 16, 9, 10, 14, 8, 7]; + auto h = heapify(a); + } +} From d524a9de6e2756c26b01f6d72147a4f59b344bf1 Mon Sep 17 00:00:00 2001 From: Tim Sviridov Date: Tue, 8 Dec 2015 23:04:42 +1100 Subject: [PATCH 2/2] Update topN(R, R) to match topN(R, i) behaviour The behaviour with regard to the bottoms or right ranges of the topNs differed in that topN(R, i) correctly moved elements around to preserve each element, but that topN(R, R) did not and ended up with duplicate and missing elements. --- std/algorithm/sorting.d | 52 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/std/algorithm/sorting.d b/std/algorithm/sorting.d index 5d64b1d30f8..d316bc08dcf 100644 --- a/std/algorithm/sorting.d +++ b/std/algorithm/sorting.d @@ -2294,9 +2294,13 @@ void topN(alias less = "a < b", static assert(ss == SwapStrategy.unstable, "Stable topN not yet implemented"); auto heap = BinaryHeap!Range1(r1); - for (; !r2.empty; r2.popFront()) + foreach (ref e; r2) { - heap.conditionalInsert(r2.front); + auto f = heap.front; + if (heap.conditionalInsert(e)) + { + e = f; + } } } @@ -2310,6 +2314,50 @@ unittest assert(a == [0, 1, 2, 2, 3]); } +unittest +{ + import std.algorithm.comparison : equal; + import std.internal.test.dummyrange; + import std.meta : AliasSeq; + alias RandomRanges = AliasSeq!( + DummyRange!(ReturnBy.Reference, Length.Yes, RangeType.Random), + DummyRange!(ReturnBy.Value, Length.Yes, RangeType.Random), + ); + + foreach (T1; RandomRanges) + { + foreach (T2; AllDummyRanges) + { + T1 a; + T2 b; + topN(a, b); + sort(a); + assert(equal(a, [ 1, 1, 2, 2, 3, 3, 4, 4, 5, 5 ])); + sort(b); + assert(equal(b, [ 6, 6, 7, 7, 8, 8, 9, 9, 10, 10 ])); + } + } +} + +// bug 15421 +unittest +{ + auto a = [ 9, 8, 0, 3, 5, 25, 43, 4, 2, 0, 7 ]; + auto b = [ 9, 8, 0, 3, 5, 25, 43, 4, 2, 0, 7 ]; + + topN(a, 4); + topN(b[0 .. 4], b[4 .. $]); + + sort(a[0 .. 4]); + sort(a[4 .. $]); + sort(b[0 .. 4]); + sort(b[4 .. $]); + + assert(a[0 .. 4] == b[0 .. 4]); + assert(a[4 .. $] == b[4 .. $]); + assert(a == b); +} + /** Copies the top $(D n) elements of the input range $(D source) into the random-access range $(D target), where $(D n =