From 08e73523f6223915ca828cd804ef86bd9be4c29d Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 5 Apr 2023 15:57:33 +0000 Subject: [PATCH] - migrate sound_splay_tree.dart to null safety - exclude from analysis all dart 2 benchmarks This reduces the analysis errors when opening up the SDK significantly. Change-Id: I9e1c4f7e4b790e4962ea2112a293bf0ef5402b10 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292440 Reviewed-by: Stephen Adams Reviewed-by: Kallen Tu Commit-Queue: Jake Macdonald --- .../dart/analysis_options.yaml | 4 + .../dart/sound_splay_tree.dart | 336 +++++++++--------- benchmarks/analysis_options.yaml | 4 +- 3 files changed, 183 insertions(+), 161 deletions(-) create mode 100644 benchmarks/SoundSplayTreeSieve/dart/analysis_options.yaml diff --git a/benchmarks/SoundSplayTreeSieve/dart/analysis_options.yaml b/benchmarks/SoundSplayTreeSieve/dart/analysis_options.yaml new file mode 100644 index 00000000000..89ac3fca893 --- /dev/null +++ b/benchmarks/SoundSplayTreeSieve/dart/analysis_options.yaml @@ -0,0 +1,4 @@ +include: ../../analysis_options.yaml +analyzer: + enable-experiment: + - variance diff --git a/benchmarks/SoundSplayTreeSieve/dart/sound_splay_tree.dart b/benchmarks/SoundSplayTreeSieve/dart/sound_splay_tree.dart index d2126f86d4d..cd9131718c1 100644 --- a/benchmarks/SoundSplayTreeSieve/dart/sound_splay_tree.dart +++ b/benchmarks/SoundSplayTreeSieve/dart/sound_splay_tree.dart @@ -11,20 +11,49 @@ typedef _Predicate = bool Function(T value); /// and right children in the tree. class _SoundSplayTreeNode { final K key; - _SoundSplayTreeNode left; - _SoundSplayTreeNode right; + _SoundSplayTreeNode? left; + _SoundSplayTreeNode? right; _SoundSplayTreeNode(this.key); } +class _DummySoundSplayTreeNode implements _SoundSplayTreeNode { + @override + _SoundSplayTreeNode? left; + + @override + _SoundSplayTreeNode? right; + + @override + // TODO: implement key + K get key => throw UnimplementedError(); + +} + /// A node in a splay tree based map. /// /// A [_SoundSplayTreeNode] that also contains a value class _SoundSplayTreeMapNode extends _SoundSplayTreeNode { - V value; + V? value; _SoundSplayTreeMapNode(K key, this.value) : super(key); } +class _DummySoundSplayTreeMapNode implements _SoundSplayTreeMapNode { + @override + _SoundSplayTreeNode? left; + + @override + _SoundSplayTreeNode? right; + + @override + V? value; + + @override + // TODO: implement key + K get key => throw UnimplementedError(); + +} + /// A splay tree is a self-balancing binary search tree. /// /// It has the additional property that recently accessed elements @@ -32,15 +61,15 @@ class _SoundSplayTreeMapNode extends _SoundSplayTreeNode { /// It performs basic operations such as insertion, look-up and /// removal, in O(log(n)) amortized time. /// TODO(kallentu): Add a variance modifier to the Node type parameter. -abstract class _SoundSplayTree> { +abstract class _SoundSplayTree { // The root node of the splay tree. It will contain either the last // element inserted or the last element looked up. - Node get _root; - set _root(Node newValue); + _SoundSplayTreeNode? get _root; + set _root(_SoundSplayTreeNode? newValue); // The dummy node used when performing a splay on the tree. Reusing it // avoids allocating a node each time a splay is performed. - Node get _dummy; + _SoundSplayTreeNode get _dummy; // Number of elements in the splay tree. int _count = 0; @@ -80,18 +109,18 @@ abstract class _SoundSplayTree> { // the L tree of the algorithm. The left child of the dummy node // will hold the R tree of the algorithm. Using a dummy node, left // and right will always be nodes and we avoid special cases. - Node left = _dummy; - Node right = _dummy; - Node current = _root; + _SoundSplayTreeNode left = _dummy; + _SoundSplayTreeNode right = _dummy; + _SoundSplayTreeNode current = _root!; int comp; while (true) { comp = _compare(current.key, key); if (comp > 0) { if (current.left == null) break; - comp = _compare(current.left.key, key); + comp = _compare(current.left!.key, key); if (comp > 0) { // Rotate right. - _SoundSplayTreeNode tmp = current.left; + final _SoundSplayTreeNode tmp = current.left!; current.left = tmp.right; tmp.right = current; current = tmp; @@ -100,13 +129,13 @@ abstract class _SoundSplayTree> { // Link right. right.left = current; right = current; - current = current.left; + current = current.left!; } else if (comp < 0) { if (current.right == null) break; - comp = _compare(current.right.key, key); + comp = _compare(current.right!.key, key); if (comp < 0) { // Rotate left. - Node tmp = current.right; + final _SoundSplayTreeNode tmp = current.right!; current.right = tmp.left; tmp.left = current; current = tmp; @@ -115,7 +144,7 @@ abstract class _SoundSplayTree> { // Link left. left.right = current; left = current; - current = current.right; + current = current.right!; } else { break; } @@ -137,10 +166,10 @@ abstract class _SoundSplayTree> { // anchored at [node]. // and that node is returned. It should replace the reference to [node] // in any parent tree or root pointer. - Node _splayMin(Node node) { - Node current = node; + _SoundSplayTreeNode _splayMin(_SoundSplayTreeNode node) { + _SoundSplayTreeNode current = node; while (current.left != null) { - Node left = current.left; + final _SoundSplayTreeNode left = current.left!; current.left = left.right; left.right = current; current = left; @@ -153,10 +182,10 @@ abstract class _SoundSplayTree> { // After this, the largest element in the tree is the root of the subtree, // and that node is returned. It should replace the reference to [node] // in any parent tree or root pointer. - Node _splayMax(Node node) { - Node current = node; + _SoundSplayTreeNode _splayMax(_SoundSplayTreeNode node) { + _SoundSplayTreeNode current = node; while (current.right != null) { - Node right = current.right; + final _SoundSplayTreeNode right = current.right!; current.right = right.left; right.left = current; current = right; @@ -164,60 +193,61 @@ abstract class _SoundSplayTree> { return current; } - Node _remove(K key) { + _SoundSplayTreeNode? _remove(K key) { if (_root == null) return null; - int comp = _splay(key); + final int comp = _splay(key); if (comp != 0) return null; - Node result = _root; + final _SoundSplayTreeNode result = _root!; _count--; // assert(_count >= 0); - if (_root.left == null) { - _root = _root.right; + if (_root!.left == null) { + _root = _root!.right; } else { - Node right = _root.right; + final _SoundSplayTreeNode? right = _root!.right; // Splay to make sure that the new root has an empty right child. - _root = _splayMax(_root.left); + _root = _splayMax(_root!.left!); // Insert the original right child as the right child of the new // root. - _root.right = right; + _root!.right = right; } _modificationCount++; return result; } - /// Adds a new root node with the given [key] or [value]. + /// Adds a new root node with the given key or value. /// /// The [comp] value is the result of comparing the existing root's key /// with key. - void _addNewRoot(Node node, int comp) { + void _addNewRoot(_SoundSplayTreeNode node, int comp) { _count++; _modificationCount++; if (_root == null) { _root = node; return; } + final root = _root!; // assert(_count >= 0); if (comp < 0) { - node.left = _root; - node.right = _root.right; - _root.right = null; + node.left = root; + node.right = root.right; + root.right = null; } else { - node.right = _root; - node.left = _root.left; - _root.left = null; + node.right = root; + node.left = root.left; + root.left = null; } _root = node; } - Node get _first { + _SoundSplayTreeNode? get _first { if (_root == null) return null; - _root = _splayMin(_root); + _root = _splayMin(_root!); return _root; } - Node get _last { + _SoundSplayTreeNode? get _last { if (_root == null) return null; - _root = _splayMax(_root); + _root = _splayMax(_root!); return _root; } @@ -228,16 +258,12 @@ abstract class _SoundSplayTree> { } } -class _TypeTest { - bool test(v) => v is T; -} - int _dynamicCompare(dynamic a, dynamic b) => Comparable.compare(a, b); Comparator _defaultCompare() { // If K <: Comparable, then we can just use Comparable.compare // with no casts. - Object compare = Comparable.compare; + final Object compare = Comparable.compare; if (compare is Comparator) { return compare; } @@ -266,15 +292,19 @@ Comparator _defaultCompare() { /// using the `compare` function on an argument value that may not be a [K] /// value. If omitted, the `isValidKey` function defaults to testing if the /// value is a [K]. -class SoundSplayTreeMap extends _SoundSplayTree> +class SoundSplayTreeMap extends _SoundSplayTree with MapMixin { - _SoundSplayTreeMapNode _root; - final _SoundSplayTreeMapNode _dummy = _SoundSplayTreeMapNode(null, null); + @override + covariant _SoundSplayTreeMapNode? _root; + @override + final _SoundSplayTreeMapNode _dummy = _DummySoundSplayTreeMapNode(); + @override Comparator _comparator; + @override _Predicate _validKey; - SoundSplayTreeMap([int compare(K key1, K key2), bool isValidKey(potentialKey)]) + SoundSplayTreeMap([int Function(K key1, K key2)? compare, bool Function(Object? potentialKey)? isValidKey]) : _comparator = compare ?? _defaultCompare(), _validKey = isValidKey ?? ((v) => v is K); @@ -283,8 +313,8 @@ class SoundSplayTreeMap extends _SoundSplayTree result = SoundSplayTreeMap(compare, isValidKey); + [int Function(K key1, K key2)? compare, bool Function(Object? potentialKey)? isValidKey]) { + final SoundSplayTreeMap result = SoundSplayTreeMap(compare, isValidKey); other.forEach((k, v) { result[k] = v; }); @@ -293,7 +323,7 @@ class SoundSplayTreeMap extends _SoundSplayTree other, - [int compare(K key1, K key2), bool isValidKey(potentialKey)]) => + [int Function(K key1, K key2)? compare, bool Function(Object? potentialKey)? isValidKey]) => SoundSplayTreeMap(compare, isValidKey)..addAll(other); /// Creates a [SoundSplayTreeMap] where the keys and values are computed from the @@ -307,22 +337,22 @@ class SoundSplayTreeMap extends _SoundSplayTree map = SoundSplayTreeMap(compare, isValidKey); - fillMapWithMappedIterable(map, iterable, key, value); + static SoundSplayTreeMap fromIterable(Iterable iterable, + {K Function(E element)? key, + V Function(E element)? value, + int Function(K key1, K key2)? compare, + bool Function(Object? potentialKey)? isValidKey}) { + final SoundSplayTreeMap map = SoundSplayTreeMap(compare, isValidKey); + fillMapWithMappedIterable(map, iterable, key, value); return map; } static _id(x) => x; - static void fillMapWithMappedIterable( - Map map, Iterable iterable, key(element), value(element)) { - key ??= _id; - value ??= _id; + static void fillMapWithMappedIterable( + Map map, Iterable iterable, K Function(E element)? key, V Function(E element)? value) { + key ??= _id as K Function(E); + value ??= _id as V Function(E); for (var element in iterable) { map[key(element)] = value(element); @@ -357,7 +387,7 @@ class SoundSplayTreeMap extends _SoundSplayTree keys, Iterable values, - [int compare(K key1, K key2), bool isValidKey(potentialKey)]) { + [int Function(K key1, K key2)? compare, bool Function(Object? potentialKey)? isValidKey]) { SoundSplayTreeMap map = SoundSplayTreeMap(compare, isValidKey); fillMapWithIterables(map, keys, values); return map; @@ -365,22 +395,20 @@ class SoundSplayTreeMap extends _SoundSplayTree _comparator(key1, key2); - SoundSplayTreeMap._internal(); - - V operator [](Object key) { + V? operator [](Object? key) { if (!_validKey(key)) return null; if (_root != null) { - int comp = _splay(key); + int comp = _splay(key as K); if (comp == 0) { - return _root.value; + return _root!.value; } } return null; } - V remove(Object key) { + V? remove(Object? key) { if (!_validKey(key)) return null; - _SoundSplayTreeMapNode mapRoot = _remove(key); + final _SoundSplayTreeMapNode? mapRoot = _remove(key as K) as _SoundSplayTreeMapNode?; if (mapRoot != null) return mapRoot.value; return null; } @@ -391,17 +419,17 @@ class SoundSplayTreeMap extends _SoundSplayTree(key, value), comp); } V putIfAbsent(K key, V ifAbsent()) { if (key == null) throw ArgumentError(key); int comp = _splay(key); if (comp == 0) { - return _root.value; + return _root!.value!; } int modificationCount = _modificationCount; int splayCount = _splayCount; @@ -414,7 +442,7 @@ class SoundSplayTreeMap extends _SoundSplayTree(key, value), comp); return value; } @@ -431,10 +459,10 @@ class SoundSplayTreeMap extends _SoundSplayTree !isEmpty; void forEach(void f(K key, V value)) { - Iterator<_SoundSplayTreeNode> nodes = _SoundSplayTreeNodeIterator(this); + Iterator<_SoundSplayTreeNode?> nodes = _SoundSplayTreeNodeIterator(this); while (nodes.moveNext()) { - _SoundSplayTreeMapNode node = nodes.current; - f(node.key, node.value); + _SoundSplayTreeMapNode node = nodes.current as _SoundSplayTreeMapNode; + f(node.key, node.value!); } } @@ -446,20 +474,20 @@ class SoundSplayTreeMap extends _SoundSplayTree node) { + bool visit(_SoundSplayTreeMapNode? node) { while (node != null) { if (node.value == value) return true; if (initialSplayCount != _splayCount) { throw ConcurrentModificationError(this); } - if (node.right != null && visit(node.right)) return true; - node = node.left; + if (node.right != null && visit(node.right as _SoundSplayTreeMapNode)) return true; + node = node.left as _SoundSplayTreeMapNode?; } return false; } @@ -472,27 +500,27 @@ class SoundSplayTreeMap extends _SoundSplayTree get values => _SoundSplayTreeValueIterable(this); /// Get the first key in the map. Returns [:null:] if the map is empty. - K firstKey() { + K? firstKey() { if (_root == null) return null; - return _first.key; + return _first!.key; } /// Get the last key in the map. Returns [:null:] if the map is empty. - K lastKey() { + K? lastKey() { if (_root == null) return null; - return _last.key; + return _last!.key; } /// Get the last key in the map that is strictly smaller than [key]. Returns /// [:null:] if no key was not found. - K lastKeyBefore(K key) { + K? lastKeyBefore(K key) { if (key == null) throw ArgumentError(key); if (_root == null) return null; int comp = _splay(key); - if (comp < 0) return _root.key; - _SoundSplayTreeNode node = _root.left; + if (comp < 0) return _root!.key; + _SoundSplayTreeNode? node = _root!.left; if (node == null) return null; - while (node.right != null) { + while (node!.right != null) { node = node.right; } return node.key; @@ -500,14 +528,14 @@ class SoundSplayTreeMap extends _SoundSplayTree 0) return _root.key; - _SoundSplayTreeNode node = _root.right; + if (comp > 0) return _root!.key; + _SoundSplayTreeNode? node = _root!.right; if (node == null) return null; - while (node.left != null) { + while (node!.left != null) { node = node.left; } return node.key; @@ -516,7 +544,7 @@ class SoundSplayTreeMap extends _SoundSplayTree implements Iterator { - final _SoundSplayTree> _tree; + final _SoundSplayTree _tree; /// Worklist of nodes to visit. /// @@ -540,38 +568,26 @@ abstract class _SoundSplayTreeIterator implements Iterator /// Count of splay operations on [_tree] when [_workList] was built. /// /// If the splay count on [_tree] increases, [_workList] becomes invalid. - int _splayCount; + int? _splayCount; /// Current node. - _SoundSplayTreeNode _currentNode; + _SoundSplayTreeNode? _currentNode; - _SoundSplayTreeIterator(_SoundSplayTree> tree) + _SoundSplayTreeIterator(_SoundSplayTree tree) : _tree = tree, _modificationCount = tree._modificationCount, _splayCount = tree._splayCount { _findLeftMostDescendant(tree._root); } - _SoundSplayTreeIterator.startAt(_SoundSplayTree> tree, K startKey) - : _tree = tree, - _modificationCount = tree._modificationCount { - if (tree._root == null) return; - int compare = tree._splay(startKey); - _splayCount = tree._splayCount; - if (compare < 0) { - // Don't include the root, start at the next element after the root. - _findLeftMostDescendant(tree._root.right); - } else { - _workList.add(tree._root); - } - } - T get current { - if (_currentNode == null) return null; - return _getValue(_currentNode); + if (_currentNode == null) { + throw StateError('Use moveNext to detect the end of an iterator'); + } + return _getValue(_currentNode!); } - void _findLeftMostDescendant(_SoundSplayTreeNode node) { + void _findLeftMostDescendant(_SoundSplayTreeNode? node) { while (node != null) { _workList.add(node); node = node.left; @@ -584,14 +600,14 @@ abstract class _SoundSplayTreeIterator implements Iterator /// If the key-set changes, iteration is aborted before getting /// here, so we know that the keys are the same as before, it's /// only the tree that has been reordered. - void _rebuildWorkList(_SoundSplayTreeNode currentNode) { + void _rebuildWorkList(_SoundSplayTreeNode? currentNode) { assert(_workList.isNotEmpty); _workList.clear(); if (currentNode == null) { _findLeftMostDescendant(_tree._root); } else { _tree._splay(currentNode.key); - _findLeftMostDescendant(_tree._root.right); + _findLeftMostDescendant(_tree._root!.right); assert(_workList.isNotEmpty); } } @@ -613,7 +629,7 @@ abstract class _SoundSplayTreeIterator implements Iterator _rebuildWorkList(_currentNode); } _currentNode = _workList.removeLast(); - _findLeftMostDescendant(_currentNode.right); + _findLeftMostDescendant(_currentNode!.right); return true; } @@ -621,7 +637,7 @@ abstract class _SoundSplayTreeIterator implements Iterator } class _SoundSplayTreeKeyIterable extends EfficientLengthIterable { - _SoundSplayTree> _tree; + _SoundSplayTree _tree; _SoundSplayTreeKeyIterable(this._tree); int get length => _tree._count; bool get isEmpty => _tree._count == 0; @@ -644,24 +660,24 @@ class _SoundSplayTreeValueIterable extends EfficientLengthIter } class _SoundSplayTreeKeyIterator extends _SoundSplayTreeIterator { - _SoundSplayTreeKeyIterator(_SoundSplayTree> map) : super(map); + _SoundSplayTreeKeyIterator(_SoundSplayTree map) : super(map); + @override K _getValue(_SoundSplayTreeNode node) => node.key; } class _SoundSplayTreeValueIterator extends _SoundSplayTreeIterator { _SoundSplayTreeValueIterator(SoundSplayTreeMap map) : super(map); V _getValue(_SoundSplayTreeNode node) { - _SoundSplayTreeMapNode mapNode = node; - return mapNode.value; + _SoundSplayTreeMapNode mapNode = node as _SoundSplayTreeMapNode; + return mapNode.value!; } } class _SoundSplayTreeNodeIterator extends _SoundSplayTreeIterator> { - _SoundSplayTreeNodeIterator(_SoundSplayTree> tree) : super(tree); - _SoundSplayTreeNodeIterator.startAt( - _SoundSplayTree> tree, K startKey) - : super.startAt(tree, startKey); + _SoundSplayTreeNodeIterator(_SoundSplayTree tree) : super(tree); + + @override _SoundSplayTreeNode _getValue(_SoundSplayTreeNode node) => node; } @@ -679,10 +695,10 @@ class _SoundSplayTreeNodeIterator /// [Comparable], and are compared using their [Comparable.compareTo] method. /// Non-comparable objects (including `null`) will not work as an element /// in that case. -class SoundSplayTreeSet extends _SoundSplayTree> +class SoundSplayTreeSet extends _SoundSplayTree with IterableMixin, SetMixin { - _SoundSplayTreeNode _root; - final _SoundSplayTreeNode _dummy = _SoundSplayTreeNode(null); + _SoundSplayTreeNode? _root; + final _SoundSplayTreeNode _dummy = _DummySoundSplayTreeNode(); Comparator _comparator; _Predicate _validKey; @@ -709,7 +725,7 @@ class SoundSplayTreeSet extends _SoundSplayTree(), _validKey = isValidKey ?? ((v) => v is E); @@ -727,7 +743,7 @@ class SoundSplayTreeSet extends _SoundSplayTree.from(superSet.whereType()); /// ``` factory SoundSplayTreeSet.from(Iterable elements, - [int compare(E key1, E key2), bool isValidKey(potentialKey)]) { + [int Function(E key1, E key2)? compare, bool Function(Object? potentialKey)? isValidKey]) { SoundSplayTreeSet result = SoundSplayTreeSet(compare, isValidKey); for (final element in elements) { E e = element; @@ -742,7 +758,7 @@ class SoundSplayTreeSet extends _SoundSplayTree elements, - [int compare(E key1, E key2), bool isValidKey(potentialKey)]) => + [int Function(E key1, E key2)? compare, bool Function(Object? potentialKey)? isValidKey]) => SoundSplayTreeSet(compare, isValidKey)..addAll(elements); Set _newSet() => @@ -761,64 +777,64 @@ class SoundSplayTreeSet extends _SoundSplayTree 1) throw IterableElementError.tooMany(); - return _root.key; + return _root!.key; } // From Set. - bool contains(Object element) { - return _validKey(element) && _splay(element) == 0; + bool contains(Object? element) { + return _validKey(element) && _splay(element as E) == 0; } bool add(E element) { int compare = _splay(element); if (compare == 0) return false; - _addNewRoot(_SoundSplayTreeNode(element), compare); + _addNewRoot(_SoundSplayTreeNode(element), compare); return true; } - bool remove(Object object) { + bool remove(Object? object) { if (!_validKey(object)) return false; - return _remove(object) != null; + return _remove(object as E) != null; } void addAll(Iterable elements) { for (E element in elements) { int compare = _splay(element); if (compare != 0) { - _addNewRoot(_SoundSplayTreeNode(element), compare); + _addNewRoot(_SoundSplayTreeNode(element), compare); } } } - void removeAll(Iterable elements) { - for (Object element in elements) { - if (_validKey(element)) _remove(element); + void removeAll(Iterable elements) { + for (Object? element in elements) { + if (_validKey(element)) _remove(element as E); } } - void retainAll(Iterable elements) { + void retainAll(Iterable elements) { // Build a set with the same sense of equality as this set. SoundSplayTreeSet retainSet = SoundSplayTreeSet(_comparator, _validKey); int modificationCount = _modificationCount; - for (Object object in elements) { + for (Object? object in elements) { if (modificationCount != _modificationCount) { // The iterator should not have side effects. throw ConcurrentModificationError(this); } // Equivalent to this.contains(object). - if (_validKey(object) && _splay(object) == 0) { - retainSet.add(_root.key); + if (_validKey(object) && _splay(object as E) == 0) { + retainSet.add(_root!.key); } } // Take over the elements from the retained set, if it differs. @@ -829,14 +845,14 @@ class SoundSplayTreeSet extends _SoundSplayTree intersection(Set other) { + Set intersection(Set other) { Set result = SoundSplayTreeSet(_comparator, _validKey); for (E element in this) { if (other.contains(element)) result.add(element); @@ -844,7 +860,7 @@ class SoundSplayTreeSet extends _SoundSplayTree difference(Set other) { + Set difference(Set other) { Set result = SoundSplayTreeSet(_comparator, _validKey); for (E element in this) { if (!other.contains(element)) result.add(element); @@ -865,11 +881,11 @@ class SoundSplayTreeSet extends _SoundSplayTree _copyNode(_SoundSplayTreeNode node) { + _SoundSplayTreeNode? _copyNode(_SoundSplayTreeNode? node) { if (node == null) return null; - return _SoundSplayTreeNode(node.key) + return (_SoundSplayTreeNode(node.key) ..left = _copyNode(node.left) - ..right = _copyNode(node.right); + ..right = _copyNode(node.right)); } void clear() { diff --git a/benchmarks/analysis_options.yaml b/benchmarks/analysis_options.yaml index 5a82c007a60..1dd81617ef9 100644 --- a/benchmarks/analysis_options.yaml +++ b/benchmarks/analysis_options.yaml @@ -2,7 +2,9 @@ # for details. All rights reserved. Use of this source code is governed by a # BSD-style license that can be found in the LICENSE file. -#analyzer: +analyzer: + exclude: + - '*/dart2/' # These don't analyze cleanly on newer sdks # strong-mode: # implicit-casts: false linter: