Skip to content

Commit

Permalink
fixes #202, check IndexExpression
Browse files Browse the repository at this point in the history
R=leafp@google.com

Review URL: https://codereview.chromium.org/1160673003
  • Loading branch information
John Messerly committed Jun 3, 2015
1 parent 8865229 commit 8524217
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 12 deletions.
2 changes: 1 addition & 1 deletion pkg/dev_compiler/lib/runtime/dart/_internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,7 @@ var _js_primitives = dart.lazyImport(_js_primitives);
this[_values] = values;
}
get(key) {
return this.containsKey(key) ? this[_values][core.$get](key) : null;
return this.containsKey(key) ? this[_values][core.$get](dart.as(key, core.int)) : null;
}
get length() {
return this[_values][core.$length];
Expand Down
6 changes: 3 additions & 3 deletions pkg/dev_compiler/lib/runtime/dart/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ var math = dart.lazyImport(math);
static from(other) {
let result = HashMap$(K, V).new();
other.forEach(dart.fn((k, v) => {
result.set(k, dart.as(v, V));
result.set(dart.as(k, K), dart.as(v, V));
}));
return result;
}
Expand Down Expand Up @@ -1218,7 +1218,7 @@ var math = dart.lazyImport(math);
static from(other) {
let result = LinkedHashMap$(K, V).new();
other.forEach(dart.fn((k, v) => {
result.set(k, dart.as(v, V));
result.set(dart.as(k, K), dart.as(v, V));
}));
return result;
}
Expand Down Expand Up @@ -3324,7 +3324,7 @@ var math = dart.lazyImport(math);
isValidKey = null;
let result = new (SplayTreeMap$(K, V))();
other.forEach(dart.fn((k, v) => {
result.set(k, dart.as(v, V));
result.set(dart.as(k, K), dart.as(v, V));
}));
return result;
}
Expand Down
29 changes: 26 additions & 3 deletions pkg/dev_compiler/lib/src/checker/checker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -719,9 +719,12 @@ class CodeChecker extends RecursiveAstVisitor {
// Method invocation.
if (element is MethodElement) {
var type = element.type as FunctionType;
assert(type.normalParameterTypes.length == 1);
node.rightOperand =
checkArgument(node.rightOperand, type.normalParameterTypes[0]);
// Analyzer should enforce number of parameter types, but check in
// case we have erroneous input.
if (type.normalParameterTypes.isNotEmpty) {
node.rightOperand =
checkArgument(node.rightOperand, type.normalParameterTypes[0]);
}
} else {
// TODO(vsm): Assert that the analyzer found an error here?
}
Expand All @@ -744,6 +747,26 @@ class CodeChecker extends RecursiveAstVisitor {
node.visitChildren(this);
}

@override
void visitIndexExpression(IndexExpression node) {
if (_rules.isDynamicTarget(node.target)) {
_recordDynamicInvoke(node);
} else {
var element = node.staticElement;
if (element is MethodElement) {
var type = element.type as FunctionType;
// Analyzer should enforce number of parameter types, but check in
// case we have erroneous input.
if (type.normalParameterTypes.isNotEmpty) {
node.index = checkArgument(node.index, type.normalParameterTypes[0]);
}
} else {
// TODO(vsm): Assert that the analyzer found an error here?
}
}
node.visitChildren(this);
}

DartType getType(TypeName name) {
return (name == null) ? _rules.provider.dynamicType : name.type;
}
Expand Down
22 changes: 21 additions & 1 deletion pkg/dev_compiler/test/checker/checker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1923,7 +1923,7 @@ void main() {
}, inferFromOverrides: true);
});

test('binary operators', () {
test('binary and index operators', () {
testChecker({
'/main.dart': '''
class A {
Expand All @@ -1938,6 +1938,7 @@ void main() {
A operator &(B b) {}
A operator ^(B b) {}
A operator |(B b) {}
A operator[](B b) {}
}
class B {
Expand Down Expand Up @@ -1979,6 +1980,11 @@ void main() {
p = (/*info:DynamicCast*/c) && /*info:DynamicCast*/c;
p = (/*severe:StaticTypeError*/y) && p;
p = c == y;
a = a[b];
a = a[/*info:DynamicCast*/c];
c = (/*info:DynamicInvoke*/c[b]);
a[/*severe:StaticTypeError*/y];
}
'''
});
Expand All @@ -1999,12 +2005,18 @@ void main() {
A operator &(B b) {}
A operator ^(B b) {}
A operator |(B b) {}
D operator [](B index) {}
void operator []=(B index, D value) {}
}
class B {
A operator -(B b) {}
}
class D {
D operator +(D d) {}
}
foo() => new A();
test() {
Expand Down Expand Up @@ -2049,6 +2061,14 @@ void main() {
a ^= b;
a |= b;
(/*info:DynamicInvoke*/c += b);
var d = new D();
a[b] += d;
a[/*info:DynamicCast*/c] += d;
a[/*severe:StaticTypeError*/z] += d;
a[b] += /*info:DynamicCast*/c;
a[b] += /*severe:StaticTypeError*/z;
(/*info:DynamicInvoke*/(/*info:DynamicInvoke*/c[b]) += d);
}
'''
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ RangeError.checkValidIndex(index, this);
}
class ListMapView<E> implements Map<int, E> {List<E> _values;
ListMapView(this._values);
E operator [](Object key) => containsKey(key) ? _values[key] : null;
E operator [](Object key) => containsKey(key) ? _values[DEVC$RT.cast(key, Object, int, "ImplicitCast", """line 251, column 59 of dart:_internal/list.dart: """, key is int, true)] : null;
int get length => _values.length;
Iterable<E> get values => new SubListIterable<E>(_values, 0, null);
Iterable<int> get keys => new _ListIndicesIterable(_values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ part of dart.collection;
factory HashMap.from(Map other) {
HashMap<K, V> result = new HashMap<K, V>();
other.forEach((k, v) {
result[k] = DEVC$RT.cast(v, dynamic, V, "CompositeCast", """line 87, column 40 of dart:collection/hash_map.dart: """, v is V, false);
result[DEVC$RT.cast(k, dynamic, K, "CompositeCast", """line 87, column 35 of dart:collection/hash_map.dart: """, k is K, false)] = DEVC$RT.cast(v, dynamic, V, "CompositeCast", """line 87, column 40 of dart:collection/hash_map.dart: """, v is V, false);
}
);
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ part of dart.collection;
factory LinkedHashMap.from(Map other) {
LinkedHashMap<K, V> result = new LinkedHashMap<K, V>();
other.forEach((k, v) {
result[k] = DEVC$RT.cast(v, dynamic, V, "CompositeCast", """line 74, column 40 of dart:collection/linked_hash_map.dart: """, v is V, false);
result[DEVC$RT.cast(k, dynamic, K, "CompositeCast", """line 74, column 35 of dart:collection/linked_hash_map.dart: """, k is K, false)] = DEVC$RT.cast(v, dynamic, V, "CompositeCast", """line 74, column 40 of dart:collection/linked_hash_map.dart: """, v is V, false);
}
);
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ _root = null;
factory SplayTreeMap.from(Map other, [int compare(K key1, K key2), bool isValidKey(Object potentialKey)]) {
SplayTreeMap<K, V> result = new SplayTreeMap<K, V>();
other.forEach((k, v) {
result[k] = DEVC$RT.cast(v, dynamic, V, "CompositeCast", """line 275, column 40 of dart:collection/splay_tree.dart: """, v is V, false);
result[DEVC$RT.cast(k, dynamic, K, "CompositeCast", """line 275, column 35 of dart:collection/splay_tree.dart: """, k is K, false)] = DEVC$RT.cast(v, dynamic, V, "CompositeCast", """line 275, column 40 of dart:collection/splay_tree.dart: """, v is V, false);
}
);
return result;
Expand Down
12 changes: 12 additions & 0 deletions pkg/dev_compiler/tool/sdk_expected_errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,9 @@ warning: line 1033, column 35 of dart:_internal/iterable.dart: [DownCastComposit
warning: line 1115, column 31 of dart:_internal/iterable.dart: [DownCastComposite] l (List<dynamic>) will need runtime check to cast to type List<T>
return new ListMapView<T>(l);
^
warning: line 251, column 59 of dart:_internal/list.dart: [DownCastImplicit] key (Object) will need runtime check to cast to type int
E operator[] (Object key) => containsKey(key) ? _values[key] : null;
^^^
warning: line 179, column 12 of dart:collection: [DownCastComposite] JS('var', '#.splice(#, 2)[1]', bucket, index) (dynamic) will need runtime check to cast to type V
return JS('var', '#.splice(#, 2)[1]', bucket, index);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -820,6 +823,9 @@ warning: line 1678, column 18 of dart:collection: [DownCastComposite] _cell._ele
warning: line 112, column 40 of dart:collection/hash_map.dart: [DownCastComposite] v (dynamic) will need runtime check to cast to type V
other.forEach((k, v) { result[k] = v; });
^
warning: line 112, column 35 of dart:collection/hash_map.dart: [DownCastComposite] k (dynamic) will need runtime check to cast to type K
other.forEach((k, v) { result[k] = v; });
^
warning: line 128, column 17 of dart:collection/hash_set.dart: [DownCastComposite] elements (Iterable<dynamic>) will need runtime check to cast to type Iterable<E>
for (E e in elements) result.add(e);
^^^^^^^^
Expand All @@ -829,6 +835,9 @@ warning: line 33, column 16 of dart:collection/iterator.dart: [DownCastComposite
warning: line 99, column 40 of dart:collection/linked_hash_map.dart: [DownCastComposite] v (dynamic) will need runtime check to cast to type V
other.forEach((k, v) { result[k] = v; });
^
warning: line 99, column 35 of dart:collection/linked_hash_map.dart: [DownCastComposite] k (dynamic) will need runtime check to cast to type K
other.forEach((k, v) { result[k] = v; });
^
warning: line 142, column 12 of dart:collection/linked_hash_map.dart: [DownCastComposite] fillLiteralMap(keyValuePairs, new _LinkedHashMap<K, V>()) (dynamic) will need runtime check to cast to type LinkedHashMap<K, V>
return fillLiteralMap(keyValuePairs, new _LinkedHashMap<K, V>());
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -925,6 +934,9 @@ severe: line 266, column 65 of dart:collection/splay_tree.dart: [InvalidRuntimeC
warning: line 275, column 40 of dart:collection/splay_tree.dart: [DownCastComposite] v (dynamic) will need runtime check to cast to type V
other.forEach((k, v) { result[k] = v; });
^
warning: line 275, column 35 of dart:collection/splay_tree.dart: [DownCastComposite] k (dynamic) will need runtime check to cast to type K
other.forEach((k, v) { result[k] = v; });
^
warning: line 328, column 25 of dart:collection/splay_tree.dart: [DownCastComposite] key (Object) will need runtime check to cast to type K
int comp = _splay(key);
^^^
Expand Down

0 comments on commit 8524217

Please sign in to comment.