Insert implicit casts when setting interface targets
This preserves the invariant that method invocations and property sets
with an interface target handle static types at the call site.
This is a prerequisite to improving dart2js with implicit checks.
With this we can mark dynamic invocations a statically safe and
discover when instance methods don't need to check parameter types.
Change-Id: Ife7a3f947952b3c1f73316b922b2aa107343068e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95400
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
diff --git a/pkg/compiler/lib/src/io/kernel_source_information.dart b/pkg/compiler/lib/src/io/kernel_source_information.dart
index 3e63370..af69c56 100644
--- a/pkg/compiler/lib/src/io/kernel_source_information.dart
+++ b/pkg/compiler/lib/src/io/kernel_source_information.dart
@@ -109,6 +109,8 @@
location = node.location;
offset = node.fileOffset;
}
+ assert(
+ location != null, "No location found for $node (${node.runtimeType})");
return new KernelSourceLocation(location, offset, name);
}
diff --git a/pkg/compiler/lib/src/ir/static_type.dart b/pkg/compiler/lib/src/ir/static_type.dart
index b8d423c..33ef11e 100644
--- a/pkg/compiler/lib/src/ir/static_type.dart
+++ b/pkg/compiler/lib/src/ir/static_type.dart
@@ -249,11 +249,35 @@
ir.DartType visitPropertySet(ir.PropertySet node) {
ir.DartType receiverType = visitNode(node.receiver);
ir.DartType valueType = super.visitPropertySet(node);
- if (node.interfaceTarget == null && receiverType is ir.InterfaceType) {
- node.interfaceTarget = hierarchy
+ ir.Member interfaceTarget = node.interfaceTarget;
+ if (interfaceTarget == null && receiverType is ir.InterfaceType) {
+ interfaceTarget = hierarchy
.getInterfaceMember(receiverType.classNode, node.name, setter: true);
+ if (interfaceTarget != null) {
+ ir.Class superclass = interfaceTarget.enclosingClass;
+ ir.Substitution receiverSubstitution =
+ ir.Substitution.fromInterfaceType(
+ getTypeAsInstanceOf(receiverType, superclass));
+ ir.DartType setterType =
+ receiverSubstitution.substituteType(interfaceTarget.setterType);
+ if (!typeEnvironment.isSubtypeOf(valueType, setterType)) {
+ // We need to insert an implicit cast to preserve the invariant that
+ // a property set with a known interface target is also statically
+ // checked.
+ ir.AsExpression implicitCast =
+ new ir.AsExpression(node.value, setterType)
+ ..isTypeError = true
+ ..parent = node;
+ node.value = implicitCast;
+ // Visit the newly created as expression; the original value has
+ // already been visited.
+ handleAsExpression(implicitCast, valueType);
+ valueType = setterType;
+ }
+ node.interfaceTarget = interfaceTarget;
+ }
}
- receiverType = _narrowInstanceReceiver(node.interfaceTarget, receiverType);
+ receiverType = _narrowInstanceReceiver(interfaceTarget, receiverType);
handlePropertySet(node, receiverType, valueType);
return valueType;
}
@@ -415,6 +439,106 @@
return false;
}
+ /// Update the interface target on [node].
+ ///
+ /// This inserts any implicit cast of the arguments necessary to uphold the
+ /// invariant that a method invocation with an interface target handles
+ /// the static types at the call site.
+ void _updateMethodInvocationTarget(
+ ir.MethodInvocation node,
+ ArgumentTypes argumentTypes,
+ ir.DartType functionType,
+ ir.Member interfaceTarget) {
+ // TODO(johnniwinther): Handle incremental target improvement.
+ if (node.interfaceTarget != null) return;
+ node.interfaceTarget = interfaceTarget;
+ if (functionType is! ir.FunctionType) return;
+ ir.FunctionType parameterTypes = functionType;
+ Map<int, ir.DartType> neededPositionalChecks = {};
+ for (int i = 0; i < node.arguments.positional.length; i++) {
+ ir.DartType argumentType = argumentTypes.positional[i];
+ ir.DartType parameterType = parameterTypes.positionalParameters[i];
+ if (!typeEnvironment.isSubtypeOf(argumentType, parameterType)) {
+ neededPositionalChecks[i] = parameterType;
+ }
+ }
+ Map<int, ir.DartType> neededNamedChecks = {};
+ for (int argumentIndex = 0;
+ argumentIndex < node.arguments.named.length;
+ argumentIndex++) {
+ ir.NamedExpression namedArgument = node.arguments.named[argumentIndex];
+ ir.DartType argumentType = argumentTypes.named[argumentIndex];
+ ir.DartType parameterType = parameterTypes.namedParameters
+ .singleWhere((namedType) => namedType.name == namedArgument.name)
+ .type;
+ if (!typeEnvironment.isSubtypeOf(argumentType, parameterType)) {
+ neededNamedChecks[argumentIndex] = parameterType;
+ }
+ }
+ if (neededPositionalChecks.isEmpty && neededNamedChecks.isEmpty) {
+ // No implicit casts needed
+ return;
+ }
+
+ List<ir.VariableDeclaration> letVariables = [];
+
+ // Arguments need to be hoisted to an enclosing let expression in order
+ // to ensure that the arguments are evaluated before any implicit cast.
+
+ ir.Expression updateArgument(ir.Expression expression, ir.TreeNode parent,
+ ir.DartType argumentType, ir.DartType checkedParameterType) {
+ ir.VariableDeclaration variable =
+ new ir.VariableDeclaration.forValue(expression, type: argumentType);
+ // Visit the newly created variable declaration.
+ handleVariableDeclaration(variable);
+ letVariables.add(variable);
+ ir.VariableGet get = new ir.VariableGet(variable)..parent = parent;
+ // Visit the newly created variable get.
+ handleVariableGet(get, argumentType);
+ cachedStaticTypes[get] = argumentType;
+
+ if (checkedParameterType == null) {
+ return get;
+ }
+ // We need to insert an implicit cast to preserve the invariant that
+ // a method invocation with a known interface target is also
+ // statically checked.
+ ir.AsExpression implicitCast =
+ new ir.AsExpression(get, checkedParameterType)
+ ..isTypeError = true
+ ..parent = parent;
+ // Visit the newly created as expression; the original value has
+ // already been visited.
+ handleAsExpression(implicitCast, argumentType);
+ return implicitCast;
+ }
+
+ for (int index = 0; index < node.arguments.positional.length; index++) {
+ ir.DartType argumentType = argumentTypes.positional[index];
+ node.arguments.positional[index] = updateArgument(
+ node.arguments.positional[index],
+ node.arguments,
+ argumentType,
+ neededPositionalChecks[index]);
+ }
+ for (int argumentIndex = 0;
+ argumentIndex < node.arguments.named.length;
+ argumentIndex++) {
+ ir.NamedExpression namedArgument = node.arguments.named[argumentIndex];
+ ir.DartType argumentType = argumentTypes.named[argumentIndex];
+ namedArgument.value = updateArgument(namedArgument.value, namedArgument,
+ argumentType, neededNamedChecks[argumentIndex]);
+ }
+
+ ir.Expression dummy = new ir.NullLiteral();
+ node.replaceWith(dummy);
+ ir.Expression body = node;
+ for (ir.VariableDeclaration variable in letVariables.reversed) {
+ body = new ir.Let(variable, body);
+ }
+ dummy.replaceWith(body);
+ }
+
/// Computes the result type of the method invocation [node] on a receiver of
/// type [receiverType].
///
@@ -436,20 +560,17 @@
ir.Member member =
hierarchy.getInterfaceMember(receiverType.classNode, node.name);
if (_isApplicable(node.arguments, member)) {
- interfaceTarget = node.interfaceTarget = member;
+ interfaceTarget = member;
}
}
if (interfaceTarget != null) {
- if (isSpecialCasedBinaryOperator(interfaceTarget)) {
- ir.DartType argumentType = argumentTypes.positional[0];
- return typeEnvironment.getTypeOfOverloadedArithmetic(
- receiverType, argumentType);
- }
ir.Class superclass = interfaceTarget.enclosingClass;
- receiverType = getTypeAsInstanceOf(receiverType, superclass);
- ir.DartType getterType = ir.Substitution.fromInterfaceType(receiverType)
- .substituteType(interfaceTarget.getterType);
+ ir.Substitution receiverSubstitution = ir.Substitution.fromInterfaceType(
+ getTypeAsInstanceOf(receiverType, superclass));
+ ir.DartType getterType =
+ receiverSubstitution.substituteType(interfaceTarget.getterType);
if (getterType is ir.FunctionType) {
+ ir.FunctionType functionType = getterType;
List<ir.DartType> typeArguments = node.arguments.types;
if (interfaceTarget is ir.Procedure &&
interfaceTarget.function.typeParameters.isNotEmpty &&
@@ -457,15 +578,22 @@
// If this was a dynamic call the invocation does not have the
// inferred default type arguments so we need to create them here
// to perform a valid substitution.
- ir.Substitution substitution =
- ir.Substitution.fromInterfaceType(receiverType);
typeArguments = interfaceTarget.function.typeParameters
- .map((t) => substitution.substituteType(t.defaultType))
+ .map((t) => receiverSubstitution.substituteType(t.defaultType))
.toList();
}
- return ir.Substitution.fromPairs(
- getterType.typeParameters, typeArguments)
- .substituteType(getterType.returnType);
+ getterType = ir.Substitution.fromPairs(
+ functionType.typeParameters, typeArguments)
+ .substituteType(functionType.withoutTypeParameters);
+ }
+ _updateMethodInvocationTarget(
+ node, argumentTypes, getterType, interfaceTarget);
+ if (isSpecialCasedBinaryOperator(interfaceTarget)) {
+ ir.DartType argumentType = argumentTypes.positional[0];
+ return typeEnvironment.getTypeOfOverloadedArithmetic(
+ receiverType, argumentType);
+ } else if (getterType is ir.FunctionType) {
+ return getterType.returnType;
} else {
return const ir.DynamicType();
}
diff --git a/tests/compiler/dart2js/impact/data/injected_cast.dart b/tests/compiler/dart2js/impact/data/injected_cast.dart
new file mode 100644
index 0000000..526414c
--- /dev/null
+++ b/tests/compiler/dart2js/impact/data/injected_cast.dart
@@ -0,0 +1,220 @@
+// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
+// 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.
+
+class A {}
+
+class B {}
+
+class C {}
+
+class D {}
+
+class E {}
+
+/*element: Class1.:static=[Object.(0)]*/
+class Class1 {
+ /*element: Class1.field1:type=[inst:JSBool,inst:JSNull,param:A]*/
+ A field1;
+}
+
+/*element: method1:
+ dynamic=[Class1.field1=],
+ type=[
+ impl:A,
+ inst:JSBool,
+ is:Class1]
+*/
+method1(dynamic o, dynamic value) {
+ if (o is! Class1) return;
+ o.field1 = value;
+}
+
+/*element: Class2.:static=[Object.(0)]*/
+class Class2<T> {
+ /*element: Class2.field2:
+ static=*,
+ type=[inst:*,param:Class2.T]
+ */
+ T field2;
+}
+
+/*element: method2:
+ dynamic=[Class2.field2=],
+ static=*,
+ type=[
+ impl:A,
+ inst:*,
+ is:Class2<A>]
+*/
+method2(dynamic o, dynamic value) {
+ if (o is! Class2<A>) return;
+ o.field2 = value;
+}
+
+/*element: Class3.:static=[Object.(0)]*/
+class Class3 {
+ /*element: Class3.method3:type=[inst:JSBool,inst:JSNull,param:A,param:B,param:C]*/
+ method3(A a, [B b, C c]) {}
+}
+
+/*element: method3:
+ dynamic=[Class3.method3(3)],
+ type=[
+ impl:A,
+ impl:C,
+ inst:JSBool,
+ is:Class3,
+ param:B]
+*/
+method3(dynamic o, dynamic a, B b, dynamic c) {
+ if (o is! Class3) return;
+ o.method3(a, b, c);
+}
+
+/*element: Class4.:static=[Object.(0)]*/
+class Class4 {
+ /*element: Class4.method4:
+ type=[inst:JSBool,inst:JSNull,param:A,param:B,param:C]
+ */
+ method4(A a, {B b, C c}) {}
+}
+
+/*element: method4:
+ dynamic=[Class4.method4(1,b,c)],
+ type=[
+ impl:A,
+ impl:C,
+ inst:JSBool,
+ is:Class4,
+ param:B]
+*/
+method4(dynamic o, dynamic a, B b, dynamic c) {
+ if (o is! Class4) return;
+ o.method4(a, c: c, b: b);
+}
+
+/*element: Class5.:static=[Object.(0)]*/
+class Class5<T1, T2> {
+ /*element: Class5.method5:
+ static=*,
+ type=[
+ inst:*,
+ param:C,
+ param:Class5.T1,
+ param:Class5.T2,
+ param:Object,
+ param:method5.S1,
+ param:method5.S2]
+ */
+ method5<S1, S2>(T1 a, [T2 b, C c, S1 d, S2 e]) {}
+}
+
+/*element: method5:
+ dynamic=[Class5.method5<D,E>(5)],
+ static=*,
+ type=[
+ impl:A,
+ impl:D,
+ inst:*,
+ is:Class5<A,B>,
+ param:B,
+ param:C,
+ param:E]
+*/
+method5(dynamic o, dynamic a, B b, C c, dynamic d, E e) {
+ if (o is! Class5<A, B>) return;
+ o.method5<D, E>(a, b, c, d, e);
+}
+
+/*element: Class6.:static=[Object.(0)]*/
+class Class6<T1, T2> {
+ /*element: Class6.method6:
+ static=*,
+ type=[
+ inst:*,
+ param:C,
+ param:Class6.T1,
+ param:Class6.T2,
+ param:Object,
+ param:method6.S1,
+ param:method6.S2]
+ */
+ method6<S1, S2>(T1 a, {T2 b, C c, S1 d, S2 e}) {}
+}
+
+/*element: method6:
+ dynamic=[Class6.method6<D,E>(1,b,c,d,e)],
+ static=*,
+ type=[
+ impl:A,
+ impl:D,
+ inst:*,
+ is:Class6<A,B>,
+ param:B,
+ param:C,
+ param:E]
+*/
+method6(dynamic o, dynamic a, B b, C c, dynamic d, E e) {
+ if (o is! Class6<A, B>) return;
+ o.method6<D, E>(a, d: d, b: b, e: e, c: c);
+}
+
+/*element: Class7.:static=[Object.(0)]*/
+class Class7 {
+ /*element: Class7.f:type=[inst:JSNull]*/
+ A Function(A) get f => null;
+}
+
+/*element: method7:
+ dynamic=[Class7.f(1),call(1)],
+ type=[impl:A,inst:JSBool,is:Class7]
+*/
+method7(dynamic o, dynamic a) {
+ if (o is! Class7) return;
+ o.f(a);
+}
+
+/*element: F.:static=[Object.(0)]*/
+class F<T> {
+ /*element: F.method:static=*,type=[inst:*,param:List<F.T>]*/
+ T method(List<T> list) => null;
+
+ /*element: F.field:static=*,type=[inst:*,param:F.T]*/
+ T field;
+}
+
+/*element: G.:static=[F.(0)]*/
+class G extends F<int> {}
+
+/*element: method8:
+ dynamic=[G.method(1)],
+ static=*,
+ type=[impl:List<int>,inst:*,is:G,param:Iterable<int>]
+*/
+method8(dynamic g, Iterable<int> iterable) {
+ if (g is! G) return null;
+ return g.method(iterable);
+}
+
+/*element: method9:
+ dynamic=[G.field=],
+ type=[impl:int,inst:JSBool,inst:JSNull,is:G,param:num]
+*/
+method9(dynamic g, num value) {
+ if (g is! G) return null;
+ return g.field = value;
+}
+
+/*element: main:**/
+main() {
+ method1(new Class1(), null);
+ method2(new Class2<A>(), null);
+ method3(new Class3(), null, null, null);
+ method4(new Class4(), null, null, null);
+ method5(new Class5<A, B>(), null, null, null, null, null);
+ method6(new Class6<A, B>(), null, null, null, null, null);
+ method7(new Class7(), null);
+ method8(new G(), null);
+ method9(new G(), null);
+}
diff --git a/tests/compiler/dart2js/inference/data/map_tracer_keys.dart b/tests/compiler/dart2js/inference/data/map_tracer_keys.dart
index af96537..576a02a 100644
--- a/tests/compiler/dart2js/inference/data/map_tracer_keys.dart
+++ b/tests/compiler/dart2js/inference/data/map_tracer_keys.dart
@@ -58,16 +58,16 @@
test2() {
dynamic theMap = {'a': 2.2, 'b': 3.3, 'c': 4.4};
theMap
- /*update: Map([subclass=JsLinkedHashMap], key: Union([exact=JSExtendableArray], [exact=JSString]), value: [null|exact=JSDouble])*/
+ /*update: Map([subclass=JsLinkedHashMap], key: [exact=JSString], value: [null|exact=JSDouble])*/
[aList2] = 5.5;
/*iterator: [exact=LinkedHashMapKeyIterable]*/
/*current: [exact=LinkedHashMapKeyIterator]*/
/*moveNext: [exact=LinkedHashMapKeyIterator]*/
for (var key in theMap.
- /*Map([subclass=JsLinkedHashMap], key: Union([exact=JSExtendableArray], [exact=JSString]), value: [null|exact=JSDouble])*/
+ /*Map([subclass=JsLinkedHashMap], key: [exact=JSString], value: [null|exact=JSDouble])*/
keys) {
aDouble2 = theMap
- /*Map([subclass=JsLinkedHashMap], key: Union([exact=JSExtendableArray], [exact=JSString]), value: [null|exact=JSDouble])*/
+ /*Map([subclass=JsLinkedHashMap], key: [exact=JSString], value: [null|exact=JSDouble])*/
[key];
}
// We have to reference it somewhere, so that it always gets resolved.
diff --git a/tests/compiler/dart2js/optimization/data/index.dart b/tests/compiler/dart2js/optimization/data/index.dart
index 12586ad..b6dd480 100644
--- a/tests/compiler/dart2js/optimization/data/index.dart
+++ b/tests/compiler/dart2js/optimization/data/index.dart
@@ -40,6 +40,7 @@
/*strong.element: mutableDynamicListDynamicIndex:Specializer=[!Index]*/
/*omit.element: mutableDynamicListDynamicIndex:Specializer=[Index]*/
@pragma('dart2js:noInline')
+@pragma('dart2js:disableFinal')
mutableDynamicListDynamicIndex(dynamic index) {
dynamic list = [0];
return list[index];
diff --git a/tests/compiler/dart2js/rti/emission/generic_methods_dynamic_02.dart b/tests/compiler/dart2js/rti/emission/generic_methods_dynamic_02.dart
index e091714..947dcca 100644
--- a/tests/compiler/dart2js/rti/emission/generic_methods_dynamic_02.dart
+++ b/tests/compiler/dart2js/rti/emission/generic_methods_dynamic_02.dart
@@ -20,6 +20,7 @@
List<T> bar<T>(Iterable<T> t) => <T>[t.first];
}
+@pragma('dart2js:disableFinal')
main() {
B b = new B();
C c = new C();
diff --git a/tests/compiler/dart2js_extra/injected_cast_test.dart b/tests/compiler/dart2js_extra/injected_cast_test.dart
new file mode 100644
index 0000000..3ab671c
--- /dev/null
+++ b/tests/compiler/dart2js_extra/injected_cast_test.dart
@@ -0,0 +1,46 @@
+// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
+// 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.
+
+import 'package:expect/expect.dart';
+
+var field;
+
+class Class {
+ @pragma('dart2js:noInline')
+ method(int i, int j, int k) {}
+}
+
+@pragma('dart2js:noInline')
+test1(dynamic c, num n) {
+ if (c is Class) {
+ c.method(field = 41, n, field = 42);
+ }
+}
+
+@pragma('dart2js:noInline')
+test2(dynamic c, num n) {
+ if (c is! Class) return;
+ c.method(field = 86, n, field = 87);
+}
+
+main() {
+ try {
+ test1(new Class(), 0.5);
+ field = 123;
+ field = 123;
+ } catch (e) {}
+ // CFE inserts the implicit cast directly on the argument expression, making
+ // it fail before later arguments are evaluated.
+ Expect.equals(41, field);
+
+ try {
+ test2(new Class(), 0.5);
+ field = 321;
+ field = 321;
+ } catch (e) {}
+ // dart2js inserts implicit casts, but does so after evaluating all arguments
+ // to ensure semantics match what it would be like to check the types when
+ // entering the callee.
+ Expect.equals(87, field);
+}
