From 223033de5e253bc9afe173097a6f7bdf934c2c26 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 1 Nov 2023 23:24:14 +0000 Subject: [PATCH 1/4] Avoid setRange with potentially incompatible types Fixes #317 --- CHANGELOG.md | 2 ++ lib/src/algorithms.dart | 5 +++-- test/algorithms_test.dart | 9 +++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c27f014f..df89964f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ - Adds `shuffled` to `IterableExtension`. - Shuffle `IterableExtension.sample` results. +- Fix `mergeSort` when the runtime iterable generic is a subtype of the static + generic. ## 1.18.0 diff --git a/lib/src/algorithms.dart b/lib/src/algorithms.dart index 5833bbd1..e551dad1 100644 --- a/lib/src/algorithms.dart +++ b/lib/src/algorithms.dart @@ -394,8 +394,9 @@ void _merge( } // First list empties first. Reached by break above. target[targetOffset++] = secondElement; - target.setRange( - targetOffset, targetOffset + (secondEnd - cursor2), secondList, cursor2); + for (var i = targetOffset; i < targetOffset + (secondEnd - cursor2); i++) { + target[i] = secondList[(i - targetOffset) + cursor2]; + } } /// Sort [elements] using a quick-sort algorithm. diff --git a/test/algorithms_test.dart b/test/algorithms_test.dart index c2ffb7f2..009f6a0a 100644 --- a/test/algorithms_test.dart +++ b/test/algorithms_test.dart @@ -366,6 +366,15 @@ void main() { reverse(l, 0, 6); expect(l, equals([2, 1, 4, 3, 6, 5])); }); + + test('mergeSort works when runtime generic is a subtype of the static type', + () { + // Regression test for https://github.com/dart-lang/collection/issues/317 + final length = 32; // _mergeSortLimit + // In order list, first half empties first during merge. + final list = List.generate(length, (i) => i); + expect(() => mergeSort(list), returnsNormally); + }); } class C { From d75e68bf165806356cc8ccec5e92fa73783c26a4 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Wed, 8 Nov 2023 00:18:16 +0000 Subject: [PATCH 2/4] Larger for safety --- test/algorithms_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/algorithms_test.dart b/test/algorithms_test.dart index 009f6a0a..359231b2 100644 --- a/test/algorithms_test.dart +++ b/test/algorithms_test.dart @@ -370,7 +370,7 @@ void main() { test('mergeSort works when runtime generic is a subtype of the static type', () { // Regression test for https://github.com/dart-lang/collection/issues/317 - final length = 32; // _mergeSortLimit + final length = 1000; // Larger than _mergeSortLimit // In order list, first half empties first during merge. final list = List.generate(length, (i) => i); expect(() => mergeSort(list), returnsNormally); From e3149c62cc506f94c9b7575d6955dc4f7bd829d4 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Thu, 9 Nov 2023 00:29:00 +0000 Subject: [PATCH 3/4] Use sublist to get correct type --- lib/src/algorithms.dart | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/src/algorithms.dart b/lib/src/algorithms.dart index e551dad1..f5ea8d3d 100644 --- a/lib/src/algorithms.dart +++ b/lib/src/algorithms.dart @@ -232,7 +232,7 @@ void mergeSort(List elements, var middle = start + firstLength; var secondLength = end - middle; // secondLength is always the same as firstLength, or one greater. - var scratchSpace = List.filled(secondLength, elements[start]); + var scratchSpace = elements.sublist(0, secondLength); _mergeSort(elements, identity, compare, middle, end, scratchSpace, 0); var firstTarget = end - firstLength; _mergeSort( @@ -268,7 +268,7 @@ void mergeSortBy(List elements, K Function(E element) keyOf, var firstLength = middle - start; var secondLength = end - middle; // secondLength is always the same as firstLength, or one greater. - var scratchSpace = List.filled(secondLength, elements[start]); + var scratchSpace = elements.sublist(0, secondLength); _mergeSort(elements, keyOf, compare, middle, end, scratchSpace, 0); var firstTarget = end - firstLength; _mergeSort(elements, keyOf, compare, start, middle, elements, firstTarget); @@ -394,9 +394,8 @@ void _merge( } // First list empties first. Reached by break above. target[targetOffset++] = secondElement; - for (var i = targetOffset; i < targetOffset + (secondEnd - cursor2); i++) { - target[i] = secondList[(i - targetOffset) + cursor2]; - } + target.setRange( + targetOffset, targetOffset + (secondEnd - cursor2), secondList, cursor2); } /// Sort [elements] using a quick-sort algorithm. From ed0f6d06457fc2f660268a87f1b60c62a4700aef Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Mon, 20 Nov 2023 22:59:39 +0000 Subject: [PATCH 4/4] use and unsorted list --- test/algorithms_test.dart | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/algorithms_test.dart b/test/algorithms_test.dart index ab220d8c..4bc1d54b 100644 --- a/test/algorithms_test.dart +++ b/test/algorithms_test.dart @@ -373,8 +373,12 @@ void main() { () { // Regression test for https://github.com/dart-lang/collection/issues/317 final length = 1000; // Larger than _mergeSortLimit - // In order list, first half empties first during merge. - final list = List.generate(length, (i) => i); + // Out of order list, with first half guaranteed to empty first during + // merge. + final list = [ + for (var i = 0; i < length / 2; i++) -i, + for (var i = 0; i < length / 2; i++) i + length, + ]; expect(() => mergeSort(list), returnsNormally); }); }