Skip to content

Commit

Permalink
Infer parameter types on overrides
Browse files Browse the repository at this point in the history
Fixes #105

This overlaps with brian's recent CL moving override inference logic.  If this looks good, we'll need to move it over as well.

R=brianwilkerson@google.com, leafp@google.com

Review URL: https://codereview.chromium.org/1311863005 .
  • Loading branch information
vsmenon committed Aug 25, 2015
1 parent c38d2e8 commit 50eb282
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 29 deletions.
12 changes: 6 additions & 6 deletions pkg/dev_compiler/lib/runtime/dart/js.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,15 @@ dart_library.library('dart/js', null, /* Imports */[
}
}
get(index) {
if (dart.is(index, core.num) && dart.equals(index, dart.dsend(index, 'toInt'))) {
this[_checkIndex](dart.as(index, core.int));
if (dart.is(index, core.num) && index == index[dartx.toInt]()) {
this[_checkIndex](index);
}
return dart.as(super.get(index), E);
}
set(index, value) {
dart.as(value, E);
if (dart.is(index, core.num) && dart.equals(index, dart.dsend(index, 'toInt'))) {
this[_checkIndex](dart.as(index, core.int));
if (dart.is(index, core.num) && index == index[dartx.toInt]()) {
this[_checkIndex](index);
}
super.set(index, value);
}
Expand Down Expand Up @@ -282,8 +282,8 @@ dart_library.library('dart/js', null, /* Imports */[
methods: () => ({
[_checkIndex]: [dart.dynamic, [core.int]],
[_checkInsertIndex]: [dart.dynamic, [core.int]],
get: [E, [dart.dynamic]],
set: [dart.void, [dart.dynamic, E]],
get: [E, [core.int]],
set: [dart.void, [core.int, E]],
add: [dart.void, [E]],
addAll: [dart.void, [core.Iterable$(E)]],
insert: [dart.void, [core.int, E]],
Expand Down
40 changes: 30 additions & 10 deletions pkg/dev_compiler/lib/src/checker/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ class LibraryResolverWithInference extends LibraryResolver {
.forEach((f) => _inferFieldTypeFromOverride(f, pending));
if (pending.isNotEmpty) _inferVariableFromInitializer(pending);

// Infer return-types from overrides
// Infer return-types and param-types from overrides
cls.members
.where((m) => m is MethodDeclaration && !m.isStatic)
.forEach(_inferMethodReturnTypeFromOverride);
.forEach(_inferMethodTypesFromOverride);
} else {
_inferVariableFromInitializer(cls.members
.where(_isInstanceField)
Expand Down Expand Up @@ -231,18 +231,38 @@ class LibraryResolverWithInference extends LibraryResolver {
}
}

void _inferMethodReturnTypeFromOverride(MethodDeclaration method) {
void _inferMethodTypesFromOverride(MethodDeclaration method) {
var methodElement = method.element;
if ((methodElement is MethodElement ||
methodElement is PropertyAccessorElement) &&
methodElement.returnType.isDynamic &&
method.returnType == null) {
var enclosingElement = methodElement.enclosingElement as ClassElement;
var type = searchTypeFor(enclosingElement.type, methodElement);
if (type != null && !type.returnType.isDynamic) {
if (methodElement is! MethodElement &&
methodElement is! PropertyAccessorElement) return;

var enclosingElement = methodElement.enclosingElement as ClassElement;
FunctionType type = null;

// Infer the return type if omitted
if (methodElement.returnType.isDynamic && method.returnType == null) {
type = searchTypeFor(enclosingElement.type, methodElement);
if (type == null) return;
if (!type.returnType.isDynamic) {
methodElement.returnType = type.returnType;
}
}

// Infer parameter types if omitted
if (method.parameters == null) return;
var parameters = method.parameters.parameters;
var length = parameters.length;
for (int i = 0; i < length; ++i) {
var parameter = parameters[i];
if (parameter is DefaultFormalParameter) parameter = parameter.parameter;
if (parameter is SimpleFormalParameter && parameter.type == null) {
type = type ?? searchTypeFor(enclosingElement.type, methodElement);
if (type == null) return;
if (type.parameters.length > i && !type.parameters[i].type.isDynamic) {
parameter.element.type = type.parameters[i].type;
}
}
}
}

void _inferVariableFromInitializer(Iterable<VariableDeclaration> variables) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/dev_compiler/test/checker/checker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,7 @@ void main() {
class Child extends Base {
void set f1(A value) {}
/*severe:InvalidMethodOverride*/void set f2(C value) {}
/*severe:InvalidMethodOverride*/void set f3(value) {}
void set f3(value) {}
/*severe:InvalidMethodOverride*/void set f4(dynamic value) {}
set f5(B value) {}
}
Expand Down Expand Up @@ -1518,7 +1518,7 @@ void main() {
void set f1(A value) {}
/*severe:InvalidMethodOverride*/void set f2(C value) {}
/*severe:InvalidMethodOverride*/void set f3(value) {}
void set f3(value) {}
/*severe:InvalidMethodOverride*/void set f4(dynamic value) {}
set f5(B value) {}
}
Expand Down Expand Up @@ -1547,7 +1547,7 @@ void main() {
/*severe:InvalidMethodOverride*/C m2(C value) {}
/*severe:InvalidMethodOverride*/A m3(C value) {}
C m4(A value) {}
/*severe:InvalidMethodOverride*/m5(value) {}
m5(value) {}
/*severe:InvalidMethodOverride*/dynamic m6(dynamic value) {}
}
'''
Expand Down Expand Up @@ -2025,14 +2025,14 @@ void main() {
implements I1 {}
class T2 extends Base implements I1 {
/*severe:InvalidMethodOverride,severe:InvalidMethodOverride*/m(a) {}
/*severe:InvalidMethodOverride*/m(a) {}
}
class T3 extends Object with /*severe:InvalidMethodOverride*/Base
implements I1 {}
class T4 extends Object with Base implements I1 {
/*severe:InvalidMethodOverride,severe:InvalidMethodOverride*/m(a) {}
/*severe:InvalidMethodOverride*/m(a) {}
}
'''
});
Expand Down
16 changes: 8 additions & 8 deletions pkg/dev_compiler/tool/sdk_expected_errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,23 @@ severe: [AnalyzerMessage] Classes cannot implement 'num' (dart:_interceptors/js_
severe: [AnalyzerMessage] The return type 'JSNumber' is not a 'double', as defined by the method 'toDouble' (dart:_interceptors/js_number.dart, line 110, col 17)
severe: [StaticTypeError] Type check failed: this (JSNumber) is not of type double (dart:_interceptors/js_number.dart, line 110, col 17)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.sign (() → num) is not a subtype of int.sign (() → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.>> ((num) → num) is not a subtype of int.>> ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.& ((num) → num) is not a subtype of int.& ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.abs (() → num) is not a subtype of int.abs (() → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.unary- (() → num) is not a subtype of int.unary- (() → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.<< ((num) → num) is not a subtype of int.<< ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.& ((num) → num) is not a subtype of int.& ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.>> ((num) → num) is not a subtype of int.>> ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.| ((num) → num) is not a subtype of int.| ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.^ ((num) → num) is not a subtype of int.^ ((int) → int). (dart:_interceptors/js_number.dart, line 349, col 13)
severe: [AnalyzerMessage] Classes cannot implement 'int' (dart:_interceptors/js_number.dart, line 349, col 41)
severe: [AnalyzerMessage] Classes cannot implement 'double' (dart:_interceptors/js_number.dart, line 349, col 46)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.sign (() → num) is not a subtype of double.sign (() → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.% ((num) → num) is not a subtype of double.% ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.* ((num) → num) is not a subtype of double.* ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.- ((num) → num) is not a subtype of double.- ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.+ ((num) → num) is not a subtype of double.+ ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.unary- (() → num) is not a subtype of double.unary- (() → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.abs (() → num) is not a subtype of double.abs (() → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.sign (() → num) is not a subtype of double.sign (() → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.remainder ((num) → num) is not a subtype of double.remainder ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.abs (() → num) is not a subtype of double.abs (() → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.+ ((num) → num) is not a subtype of double.+ ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.- ((num) → num) is not a subtype of double.- ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.* ((num) → num) is not a subtype of double.* ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [InvalidMethodOverride] Base class introduces an invalid override. The type of JSNumber.% ((num) → num) is not a subtype of double.% ((num) → double). (dart:_interceptors/js_number.dart, line 420, col 16)
severe: [AnalyzerMessage] Classes cannot implement 'double' (dart:_interceptors/js_number.dart, line 420, col 44)
severe: [AnalyzerMessage] Classes cannot implement 'String' (dart:_interceptors/js_string.dart, line 14, col 47)
severe: [StaticTypeError] Type check failed: s += s (String) is not of type JSString (dart:_interceptors/js_string.dart, line 346, col 7)
Expand Down

0 comments on commit 50eb282

Please sign in to comment.