diff --git a/sdk/lib/_internal/vm_shared/lib/compact_hash.dart b/sdk/lib/_internal/vm_shared/lib/compact_hash.dart index 4d30021b0d4..705915a9c47 100644 --- a/sdk/lib/_internal/vm_shared/lib/compact_hash.dart +++ b/sdk/lib/_internal/vm_shared/lib/compact_hash.dart @@ -629,8 +629,8 @@ mixin _LinkedHashMapMixin on _HashBase, _EqualsAndHashCode { } } - Iterable get keys => _CompactIterable(this, _data, _usedData, -2, 2); - Iterable get values => _CompactIterable(this, _data, _usedData, -1, 2); + Iterable get keys => _CompactIterable(this, -2, 2); + Iterable get values => _CompactIterable(this, -1, 2); } base class _CompactLinkedIdentityHashMap extends _HashFieldBase @@ -675,17 +675,13 @@ base class _CompactLinkedCustomHashMap extends _HashFieldBase // and checks for concurrent modification. class _CompactIterable extends Iterable { final _HashBase _table; - // dart:core#_List (sdk/lib/_internal/vm/lib/array.dart). - final List _data; - final int _len; final int _offset; final int _step; - _CompactIterable( - this._table, this._data, this._len, this._offset, this._step); + _CompactIterable(this._table, this._offset, this._step); - Iterator get iterator => - _CompactIterator(_table, _data, _len, _offset, _step); + Iterator get iterator => _CompactIterator( + _table, _table._data, _table._usedData, _offset, _step); int get length => _table.length; bool get isEmpty => length == 0; @@ -928,6 +924,7 @@ mixin _LinkedHashSetMixin on _HashBase, _EqualsAndHashCode { i = _HashBase._nextProbe(i, sizeMask); pair = _index[i]; } + return false; } diff --git a/tests/corelib/map_test.dart b/tests/corelib/map_test.dart index a5f749f9715..cedf51396cc 100644 --- a/tests/corelib/map_test.dart +++ b/tests/corelib/map_test.dart @@ -140,6 +140,8 @@ void main() { isValidKey: (a) => a is int)); testFrom(); + + testLazyKeysValueEntries(); } void test(Map map) { @@ -1048,3 +1050,124 @@ void testUnmodifiableMaps() { checkUnmodifiable(UnmodifiableMapView({1: 1})); checkUnmodifiable(const MapView({1: 1})); } + +void testLazyKeysValueEntries() { + // Regression test for https://dartbug.com/48282 + // + // Checks that the keys, values and entries iterables are lazy iterables + // backed by the map. + // + // Creates a fresh map, then fills in entries, and then removes them again. + // At each step, check that the `keys`, `values` and `entries` iterables + // contain the expected elements. (Not checking ordering, since HashMap + // doesn't guarantee one.) + const mapSize = 129; + void testWithKeyType>( + String keyTypeName, K Function(int) toKey) { + void testWithMap(String mapType, Map map) { + // This test does a lot of linear work per element, so too large + // mapSize makes it slow. + // It prints a diagnostic at the end if a test is particularly slow. + var sw = Stopwatch()..start(); + String testName = "$mapType<$keyTypeName, int>"; + var keys = map.keys; + var values = map.values; + var entries = map.entries; + Expect.equals(0, map.length, testName); + Expect.equals(0, keys.length, testName); + Expect.equals(0, values.length, testName); + Expect.equals(0, entries.length, testName); + Expect.isTrue(keys.toList().isEmpty, testName); + Expect.isTrue(values.toList().isEmpty, testName); + Expect.isTrue(entries.toList().isEmpty, testName); + + for (var i = 0; i < mapSize; i++) { + map[toKey(i)] = i; + Expect.equals(i + 1, map.length, testName); + Expect.equals(i + 1, keys.length, testName); + Expect.equals(i + 1, values.length, testName); + Expect.equals(i + 1, entries.length, testName); + Expect.listEquals( + [for (var j = 0; j <= i; j++) toKey(j)]..sort(), + keys.map((x) => x).toList()..sort(), + testName, + ); + Expect.listEquals( + [for (var j = 0; j <= i; j++) j], + values.map((x) => x).toList()..sort(), + testName, + ); + { + // No `operator==` on `MapEntry`. + var currentEntries = entries.map((x) => x).toList() + ..sort((e1, e2) => e1.value.compareTo(e2.value)); + for (var j = 0; j <= i; j++) { + var currentEntry = currentEntries[j]; + Expect.equals(toKey(j), currentEntry.key, testName); + Expect.equals(j, currentEntry.value, testName); + } + } + Expect.equals(map.keys.last, keys.last); + Expect.equals(map.values.last, values.last); + Expect.equals(map.keys.last, entries.last.key); + Expect.equals(map.values.last, entries.last.value); + } + for (var i = 0; i < mapSize; i++) { + Expect.equals(map.keys.first, keys.first); + Expect.equals(map.values.first, values.first); + Expect.equals(map.keys.first, entries.first.key); + Expect.equals(map.values.first, entries.first.value); + var removed = map.remove(toKey(i)); + Expect.equals(i, removed, testName); + Expect.equals(mapSize - 1 - i, map.length, testName); + Expect.equals(mapSize - 1 - i, keys.length, testName); + Expect.equals(mapSize - 1 - i, values.length, testName); + Expect.equals(mapSize - 1 - i, entries.length, testName); + Expect.listEquals( + [for (var j = i + 1; j < mapSize; j++) toKey(j)]..sort(), + keys.map((x) => x).toList()..sort(), + testName, + ); + Expect.listEquals( + [for (var j = i + 1; j < mapSize; j++) j], + values.map((x) => x).toList()..sort(), + testName, + ); + { + // No `operator==` on `MapEntry`. + var currentEntries = entries.map((x) => x).toList() + ..sort((e1, e2) => e1.value.compareTo(e2.value)); + for (var j = i + 1; j < mapSize; j++) { + var currentEntry = currentEntries[j - i - 1]; + Expect.equals(toKey(j), currentEntry.key, testName); + Expect.equals(j, currentEntry.value, testName); + } + } + } + Expect.isTrue(keys.isEmpty, testName); + Expect.isTrue(values.isEmpty, testName); + Expect.isTrue(entries.isEmpty, testName); + var elapsed = sw.elapsedMilliseconds; + if (elapsed > 200) { + print("$testName: $elapsed ms"); + } + } + + testWithMap("SplayTreeMap", SplayTreeMap()); + testWithMap("HashMap", HashMap()); + testWithMap("LinkedHashMap", LinkedHashMap()); + testWithMap("LinkedHashMap-literal", {}); + } + + testWithKeyType("int", (int x) => x); + testWithKeyType("String", (int x) => "$x"); + testWithKeyType("Key", Key.new); +} + +class Key implements Comparable { + final int id; + Key(this.id); + int get hashCode => id.hashCode ^ 1023; + bool operator ==(Object other) => other is Key && id == other.id; + int compareTo(Key other) => id.compareTo(other.id); +}