-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes the strong-mode analysis errors. #290
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,7 @@ abstract class TreeSet<V> extends IterableBase<V> implements Set<V> { | |
* default `(a, b) => a.compareTo(b)`. | ||
*/ | ||
factory TreeSet({Comparator<V> comparator}) { | ||
if (comparator == null) { | ||
comparator = (a, b) => a.compareTo(b); | ||
} | ||
comparator ??= (a, b) => (a as dynamic).compareTo(b); | ||
return new AvlTreeSet(comparator: comparator); | ||
} | ||
|
||
|
@@ -148,7 +146,7 @@ abstract class _TreeNode<V> { | |
if (node.left != null) { | ||
return node.left.maximumNode; | ||
} | ||
while (node.parent != null && node.parent._left == node) { | ||
while (node.parent != null && node.parent.left == node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uh oh! Seems like our tests are not reaching this code. @cbracken |
||
node = node.parent; | ||
} | ||
return node.parent; | ||
|
@@ -438,7 +436,9 @@ class AvlTreeSet<V> extends TreeSet<V> { | |
} | ||
|
||
bool remove(Object item) { | ||
AvlNode<V> x = _getNode(item); | ||
if (item is! V) return false; | ||
|
||
AvlNode<V> x = _getNode(item as V); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will throw if item is not a V, is this the intended behaviour? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@cbracken What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, nvm, we are implementing if (item is! V) {
return false;
}
... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shame that the analyzer doesn't infer that... FYI filed dart-lang/sdk#26195 |
||
if (x != null) { | ||
_removeNode(x); | ||
return true; | ||
|
@@ -627,9 +627,9 @@ class AvlTreeSet<V> extends TreeSet<V> { | |
* See [Set.retainAll] | ||
*/ | ||
void retainAll(Iterable<Object> elements) { | ||
List<V> chosen = []; | ||
List<V> chosen = <V>[]; | ||
for (var target in elements) { | ||
if (contains(target)) { | ||
if (target is V && contains(target)) { | ||
chosen.add(target); | ||
} | ||
} | ||
|
@@ -686,11 +686,11 @@ class AvlTreeSet<V> extends TreeSet<V> { | |
* See [Set.lookup] | ||
*/ | ||
V lookup(Object element) { | ||
if (element == null || _root == null) return null; | ||
if (element is! V || _root == null) return null; | ||
AvlNode<V> x = _root; | ||
int compare = 0; | ||
while (x != null) { | ||
compare = comparator(element, x.object); | ||
compare = comparator(element as V, x.object); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar situation here. It seems everywhere we take if (element is! V) {
// return false, or otherwise ignore the element
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the check to the preconditions |
||
if (compare == 0) { | ||
return x.object; | ||
} else if (compare < 0) { | ||
|
@@ -792,8 +792,8 @@ class AvlTreeSet<V> extends TreeSet<V> { | |
Set<V> intersection(Set<Object> other) { | ||
TreeSet<V> set = new TreeSet(comparator: comparator); | ||
|
||
// Opitimized for sorted sets | ||
if (other is TreeSet) { | ||
// Optimized for sorted sets | ||
if (other is TreeSet<V>) { | ||
var i1 = iterator; | ||
var i2 = other.iterator; | ||
var hasMore1 = i1.moveNext(); | ||
|
@@ -925,7 +925,7 @@ class _AvlTreeIterator<V> implements BidirectionalIterator<V> { | |
final bool reversed; | ||
final AvlTreeSet<V> tree; | ||
final int _modCountGuard; | ||
final Object anchorObject; | ||
final V anchorObject; | ||
final bool inclusive; | ||
|
||
_IteratorMove _moveNext; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vsmenon @leafpetersen Can this be inferred?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think not, if I read the code correctly, because of the Dart rules about type promotion. In regular Dart, items is dynamic, and promotion is not allowed to promote from dynamic. We allow promotion from dynamic in strong mode, but we still don't allow promotion between non-subtype related types, and I believe (if I read this correctly) that items will be inferred to have type T, and T and List are not subtype related, and so no type promotion will be done.