Update LinkedList.contains to take advantage of internal structure.

Made it explicit in the documentation that elements of the `LinkedList`
are compared using identity, not `operator==`, even if they choose to
override that.
The linked list entries are directly linked to their containing list,
which is what defined their containing list. Merely being equal to an entry
of a list does not make another entry belong to that list.

The `contains` method now simply checks that the `list` property of the
the provided entry is the list itself, which matches the existing behavior
of `remove`.

Fixes #44189.

Bug: https://github.com/dart-lang/sdk/issues/44189
Change-Id: Ia6a7fd461ddf18f99121662f87e83415746e1ca4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/172161
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
This commit is contained in:
Lasse Reichstein Holst Nielsen 2020-12-02 14:08:08 +00:00 committed by commit-bot@chromium.org
parent 321af0844e
commit 8129c40c5e
4 changed files with 140 additions and 57 deletions

View file

@ -25,6 +25,11 @@ opted out of null safety by adding `// @dart=2.9` to the beginning of the file.
constructors, a name which can be associated with the port and displayed in
tooling.
#### `dart:collection`
* `LinkedList` made it explicit that elements are compared by identity,
and updated `contains` to take advantage of this.
### Dart VM
* **Breaking Change** [#42312][]: `Dart_WeakPersistentHandle`s will no longer

View file

@ -6,7 +6,7 @@ part of dart.collection;
/// A specialized double-linked list of elements that extends [LinkedListEntry].
///
/// This is not a generic data structure. It only accepts elements that extend
/// This is not a generic data structure. It only accepts elements that *extend*
/// the [LinkedListEntry] class. See the [Queue] implementations for generic
/// collections that allow constant time adding and removing at the ends.
///
@ -17,6 +17,9 @@ part of dart.collection;
/// Because the elements themselves contain the links of this linked list,
/// each element can be in only one list at a time. To add an element to another
/// list, it must first be removed from its current list (if any).
/// For the same reason, the [remove] and [contains] methods
/// are based on *identity*, even if the [LinkedListEntry] chooses
/// to override [Object.operator==].
///
/// In return, each element knows its own place in the linked list, as well as
/// which list it is in. This allows constant time
@ -61,6 +64,13 @@ class LinkedList<E extends LinkedListEntry<E>> extends Iterable<E> {
return true;
}
/// Whether [entry] is a [LinkedListEntry] belonging to this list.
///
/// The [entry] is considered as belonging to this list if
/// its [LinkedListEntry.list] is this list.
bool contains(Object? entry) =>
entry is LinkedListEntry && identical(this, entry.list);
Iterator<E> get iterator => _LinkedListIterator<E>(this);
int get length => _length;

View file

@ -11,16 +11,19 @@ class MyEntry extends LinkedListEntry<MyEntry> {
MyEntry(int this.value);
String toString() => value.toString();
int get hashCode => value.hashCode;
bool operator ==(Object o) => o is MyEntry && value == o.value;
}
testPreviousNext() {
var list = new LinkedList<MyEntry>();
void testPreviousNext() {
var list = LinkedList<MyEntry>();
Expect.throws(() => list.first);
Expect.throws(() => list.last);
Expect.equals(0, list.length);
for (int i = 0; i < 3; i++) {
list.add(new MyEntry(i));
list.add(MyEntry(i));
}
Expect.equals(3, list.length);
@ -39,11 +42,11 @@ testPreviousNext() {
Expect.isNull(entry.previous);
}
testUnlinked() {
var unlinked = new MyEntry(0);
void testUnlinked() {
var unlinked = MyEntry(0);
Expect.isNull(unlinked.previous);
Expect.isNull(unlinked.next);
var list = new LinkedList<MyEntry>();
var list = LinkedList<MyEntry>();
list.add(unlinked);
Expect.isNull(unlinked.previous);
Expect.isNull(unlinked.next);
@ -51,7 +54,7 @@ testUnlinked() {
Expect.isNull(unlinked.previous);
Expect.isNull(unlinked.next);
list.add(unlinked);
list.add(new MyEntry(1));
list.add(MyEntry(1));
Expect.isNull(unlinked.previous);
Expect.equals(1, unlinked.next!.value);
list.remove(unlinked);
@ -62,11 +65,11 @@ testUnlinked() {
Expect.equals(1, unlinked.previous!.value);
}
testInsert() {
void testInsert() {
// Insert last.
var list = new LinkedList<MyEntry>();
var list = LinkedList<MyEntry>();
for (int i = 0; i < 10; i++) {
list.add(new MyEntry(i));
list.add(MyEntry(i));
}
Expect.equals(10, list.length);
@ -83,7 +86,7 @@ testInsert() {
// Insert first.
for (int i = 0; i < 10; i++) {
list.addFirst(new MyEntry(i));
list.addFirst(MyEntry(i));
}
Expect.equals(10, list.length);
@ -97,9 +100,9 @@ testInsert() {
list.clear();
// Insert after.
list.addFirst(new MyEntry(0));
list.addFirst(MyEntry(0));
for (int i = 1; i < 10; i++) {
list.last.insertAfter(new MyEntry(i));
list.last.insertAfter(MyEntry(i));
}
Expect.equals(10, list.length);
@ -115,9 +118,9 @@ testInsert() {
list.clear();
// Insert before.
list.addFirst(new MyEntry(0));
list.addFirst(MyEntry(0));
for (int i = 1; i < 10; i++) {
list.first.insertBefore(new MyEntry(i));
list.first.insertBefore(MyEntry(i));
}
Expect.equals(10, list.length);
@ -131,10 +134,10 @@ testInsert() {
list.clear();
}
testRemove() {
var list = new LinkedList<MyEntry>();
void testRemove() {
var list = LinkedList<MyEntry>();
for (int i = 0; i < 10; i++) {
list.add(new MyEntry(i));
list.add(MyEntry(i));
}
Expect.equals(10, list.length);
@ -162,21 +165,51 @@ testRemove() {
Expect.equals(0, list.length);
}
testBadAdd() {
var list1 = new LinkedList<MyEntry>();
list1.addFirst(new MyEntry(0));
void testContains() {
var list = LinkedList<MyEntry>();
var entry5 = MyEntry(5);
var list2 = new LinkedList<MyEntry>();
Expect.throws(() => list2.addFirst(list1.first));
// Empty lists contains nothing.
Expect.isFalse(list.contains(null));
Expect.isFalse(list.contains(Object()));
Expect.isFalse(list.contains(entry5));
Expect.throws(() => new MyEntry(0).unlink());
// Works for singleton lists.
list.add(MyEntry(0));
Expect.isTrue(list.contains(list.first));
Expect.isFalse(list.contains(null));
Expect.isFalse(list.contains(Object()));
Expect.isFalse(list.contains(entry5));
// Works for larger lists.
for (int i = 1; i < 10; i++) {
list.add(MyEntry(i));
}
for (var entry in list) {
Expect.isTrue(list.contains(entry));
}
Expect.isFalse(list.contains(Object()));
Expect.isFalse(list.contains(null));
Expect.isFalse(list.contains(entry5));
// Based on identity, not equality.
Expect.equals(entry5, list.elementAt(5));
}
testConcurrentModificationError() {
void testBadAdd() {
var list1 = LinkedList<MyEntry>();
list1.addFirst(MyEntry(0));
var list2 = LinkedList<MyEntry>();
Expect.throws(() => list2.addFirst(list1.first));
Expect.throws(() => MyEntry(0).unlink());
}
void testConcurrentModificationError() {
test(function(LinkedList<MyEntry> ll)) {
var ll = new LinkedList<MyEntry>();
var ll = LinkedList<MyEntry>();
for (int i = 0; i < 10; i++) {
ll.add(new MyEntry(i));
ll.add(MyEntry(i));
}
Expect.throws(() => function(ll), (e) => e is ConcurrentModificationError);
}
@ -269,11 +302,12 @@ testConcurrentModificationError() {
});
}
main() {
void main() {
testPreviousNext();
testUnlinked();
testInsert();
testRemove();
testContains();
testBadAdd();
testConcurrentModificationError();
}

View file

@ -11,16 +11,19 @@ class MyEntry extends LinkedListEntry<MyEntry> {
MyEntry(int this.value);
String toString() => value.toString();
int get hashCode => value.hashCode;
bool operator ==(Object o) => o is MyEntry && value == o.value;
}
testPreviousNext() {
var list = new LinkedList<MyEntry>();
void testPreviousNext() {
var list = LinkedList<MyEntry>();
Expect.throws(() => list.first);
Expect.throws(() => list.last);
Expect.equals(0, list.length);
for (int i = 0; i < 3; i++) {
list.add(new MyEntry(i));
list.add(MyEntry(i));
}
Expect.equals(3, list.length);
@ -39,11 +42,11 @@ testPreviousNext() {
Expect.isNull(entry.previous);
}
testUnlinked() {
var unlinked = new MyEntry(0);
void testUnlinked() {
var unlinked = MyEntry(0);
Expect.isNull(unlinked.previous);
Expect.isNull(unlinked.next);
var list = new LinkedList<MyEntry>();
var list = LinkedList<MyEntry>();
list.add(unlinked);
Expect.isNull(unlinked.previous);
Expect.isNull(unlinked.next);
@ -51,7 +54,7 @@ testUnlinked() {
Expect.isNull(unlinked.previous);
Expect.isNull(unlinked.next);
list.add(unlinked);
list.add(new MyEntry(1));
list.add(MyEntry(1));
Expect.isNull(unlinked.previous);
Expect.equals(1, unlinked.next.value);
list.remove(unlinked);
@ -62,11 +65,11 @@ testUnlinked() {
Expect.equals(1, unlinked.previous.value);
}
testInsert() {
void testInsert() {
// Insert last.
var list = new LinkedList<MyEntry>();
var list = LinkedList<MyEntry>();
for (int i = 0; i < 10; i++) {
list.add(new MyEntry(i));
list.add(MyEntry(i));
}
Expect.equals(10, list.length);
@ -83,7 +86,7 @@ testInsert() {
// Insert first.
for (int i = 0; i < 10; i++) {
list.addFirst(new MyEntry(i));
list.addFirst(MyEntry(i));
}
Expect.equals(10, list.length);
@ -97,9 +100,9 @@ testInsert() {
list.clear();
// Insert after.
list.addFirst(new MyEntry(0));
list.addFirst(MyEntry(0));
for (int i = 1; i < 10; i++) {
list.last.insertAfter(new MyEntry(i));
list.last.insertAfter(MyEntry(i));
}
Expect.equals(10, list.length);
@ -115,9 +118,9 @@ testInsert() {
list.clear();
// Insert before.
list.addFirst(new MyEntry(0));
list.addFirst(MyEntry(0));
for (int i = 1; i < 10; i++) {
list.first.insertBefore(new MyEntry(i));
list.first.insertBefore(MyEntry(i));
}
Expect.equals(10, list.length);
@ -131,10 +134,10 @@ testInsert() {
list.clear();
}
testRemove() {
var list = new LinkedList<MyEntry>();
void testRemove() {
var list = LinkedList<MyEntry>();
for (int i = 0; i < 10; i++) {
list.add(new MyEntry(i));
list.add(MyEntry(i));
}
Expect.equals(10, list.length);
@ -162,21 +165,51 @@ testRemove() {
Expect.equals(0, list.length);
}
testBadAdd() {
var list1 = new LinkedList<MyEntry>();
list1.addFirst(new MyEntry(0));
void testContains() {
var list = LinkedList<MyEntry>();
var entry5 = MyEntry(5);
var list2 = new LinkedList<MyEntry>();
Expect.throws(() => list2.addFirst(list1.first));
// Empty lists contains nothing.
Expect.isFalse(list.contains(null));
Expect.isFalse(list.contains(Object()));
Expect.isFalse(list.contains(entry5));
Expect.throws(() => new MyEntry(0).unlink());
// Works for singleton lists.
list.add(MyEntry(0));
Expect.isTrue(list.contains(list.first));
Expect.isFalse(list.contains(null));
Expect.isFalse(list.contains(Object()));
Expect.isFalse(list.contains(entry5));
// Works for larger lists.
for (int i = 1; i < 10; i++) {
list.add(MyEntry(i));
}
for (var entry in list) {
Expect.isTrue(list.contains(entry));
}
Expect.isFalse(list.contains(Object()));
Expect.isFalse(list.contains(null));
Expect.isFalse(list.contains(entry5));
// Based on identity, not equality.
Expect.isTrue(list.elementAt(5) == entry5);
}
testConcurrentModificationError() {
void testBadAdd() {
var list1 = LinkedList<MyEntry>();
list1.addFirst(MyEntry(0));
var list2 = LinkedList<MyEntry>();
Expect.throws(() => list2.addFirst(list1.first));
Expect.throws(() => MyEntry(0).unlink());
}
void testConcurrentModificationError() {
test(function(LinkedList<MyEntry> ll)) {
var ll = new LinkedList<MyEntry>();
var ll = LinkedList<MyEntry>();
for (int i = 0; i < 10; i++) {
ll.add(new MyEntry(i));
ll.add(MyEntry(i));
}
Expect.throws(() => function(ll), (e) => e is ConcurrentModificationError);
}
@ -269,11 +302,12 @@ testConcurrentModificationError() {
});
}
main() {
void main() {
testPreviousNext();
testUnlinked();
testInsert();
testRemove();
testContains();
testBadAdd();
testConcurrentModificationError();
}