[vm/aot/tfa] Fix corner case in tracking of field values in TFA
The detailed description of the corner case can be found in comment in
_FieldValue.setValue() method (in analysis.dart).
Unfortunately, this bugfix increases analysis time of Flutter gallery
from 10383ms to 11624ms.
Change-Id: I6161fea0747a27d6461e7ff9a1fcedd6b2f91d1e
Reviewed-on: https://dart-review.googlesource.com/66103
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/pkg/vm/lib/transformations/type_flow/analysis.dart b/pkg/vm/lib/transformations/type_flow/analysis.dart
index 4c6eff1..1f4a615 100644
--- a/pkg/vm/lib/transformations/type_flow/analysis.dart
+++ b/pkg/vm/lib/transformations/type_flow/analysis.dart
@@ -731,9 +731,38 @@
}
void setValue(Type newValue, TypeFlowAnalysis typeFlowAnalysis) {
- final Type newType = value.union(
- newValue.intersection(staticType, typeFlowAnalysis.hierarchyCache),
- typeFlowAnalysis.hierarchyCache);
+ // Make sure type cones are specialized before putting them into field
+ // value, in order to ensure that dependency is established between
+ // cone's base type and corresponding field setter.
+ //
+ // This ensures correct invalidation in the following scenario:
+ //
+ // 1) setValue(Cone(X)).
+ // It sets field value to Cone(X).
+ //
+ // 2) setValue(Y).
+ // It calculates Cone(X) U Y, specializing Cone(X).
+ // This establishes class X --> setter(Y) dependency.
+ // If X does not have allocated subclasses, then Cone(X) is specialized
+ // to Empty and the new field value is Y.
+ //
+ // 3) A new allocated subtype is added to X.
+ // This invalidates setter(Y). However, recalculation of setter(Y)
+ // does not yield correct field value, as value calculated on step 1 is
+ // already lost, and repeating setValue(Y) will not change field value.
+ //
+ // The eager specialization of field value ensures that specialization
+ // will happen on step 1 and dependency class X --> setter(Cone(X))
+ // is established.
+ //
+ final hierarchy = typeFlowAnalysis.hierarchyCache;
+ Type newType = value
+ .union(
+ newValue.specialize(hierarchy).intersection(staticType, hierarchy),
+ hierarchy)
+ .specialize(hierarchy);
+ assertx(newType.isSpecialized);
+
if (newType != value) {
if (kPrintTrace) {
tracePrint("Set field $field value $newType");
@@ -887,8 +916,8 @@
@override
bool isSubtype(DartType subType, DartType superType) {
if (kPrintTrace) {
- tracePrint("isSubtype for sub = $subType (${subType
- .runtimeType}), sup = $superType (${superType.runtimeType})");
+ tracePrint("isSubtype for sub = $subType (${subType.runtimeType}),"
+ " sup = $superType (${superType.runtimeType})");
}
if (subType == superType) {
return true;
diff --git a/pkg/vm/lib/transformations/type_flow/summary.dart b/pkg/vm/lib/transformations/type_flow/summary.dart
index e918a6d..f4a99d2 100644
--- a/pkg/vm/lib/transformations/type_flow/summary.dart
+++ b/pkg/vm/lib/transformations/type_flow/summary.dart
@@ -83,7 +83,9 @@
Type get argumentType => _argumentType;
void _observeArgumentType(Type argType, TypeHierarchy typeHierarchy) {
+ assertx(argType.isSpecialized);
_argumentType = _argumentType.union(argType, typeHierarchy);
+ assertx(_argumentType.isSpecialized);
}
}
@@ -183,10 +185,11 @@
if (selector is! DirectSelector) {
_observeReceiverType(argTypes[0]);
}
- final Type result = callHandler.applyCall(
+ Type result = callHandler.applyCall(
this, selector, new Args<Type>(argTypes, names: args.names),
isResultUsed: isResultUsed);
if (isResultUsed) {
+ result = result.specialize(typeHierarchy);
_observeResultType(result, typeHierarchy);
}
return result;
@@ -252,7 +255,9 @@
}
void _observeResultType(Type result, TypeHierarchy typeHierarchy) {
+ assertx(result.isSpecialized);
_resultType = _resultType.union(result, typeHierarchy);
+ assertx(_resultType.isSpecialized);
}
}
@@ -311,36 +316,37 @@
List<Type> types = new List<Type>(_statements.length);
- types.setRange(0, positionalArgCount, args);
-
for (int i = 0; i < positionalArgCount; i++) {
- Parameter param = _statements[i] as Parameter;
- param._observeArgumentType(args[i], typeHierarchy);
- types[i] = args[i].intersection(param.staticType, typeHierarchy);
+ final Parameter param = _statements[i] as Parameter;
+ final argType = args[i].specialize(typeHierarchy);
+ param._observeArgumentType(argType, typeHierarchy);
+ types[i] = argType.intersection(param.staticType, typeHierarchy);
}
for (int i = positionalArgCount; i < positionalParameterCount; i++) {
- Parameter param = _statements[i] as Parameter;
- assertx(param.defaultValue != null);
- param._observeArgumentType(param.defaultValue, typeHierarchy);
- types[i] = param.defaultValue;
+ final Parameter param = _statements[i] as Parameter;
+ final argType = param.defaultValue.specialize(typeHierarchy);
+ param._observeArgumentType(argType, typeHierarchy);
+ types[i] = argType;
}
final argNames = arguments.names;
int argIndex = 0;
for (int i = positionalParameterCount; i < parameterCount; i++) {
- Parameter param = _statements[i] as Parameter;
+ final Parameter param = _statements[i] as Parameter;
assertx(param.defaultValue != null);
if ((argIndex < namedArgCount) && (argNames[argIndex] == param.name)) {
- Type argType = args[positionalArgCount + argIndex];
+ final argType =
+ args[positionalArgCount + argIndex].specialize(typeHierarchy);
argIndex++;
param._observeArgumentType(argType, typeHierarchy);
types[i] = argType.intersection(param.staticType, typeHierarchy);
} else {
assertx((argIndex == namedArgCount) ||
(param.name.compareTo(argNames[argIndex]) < 0));
- param._observeArgumentType(param.defaultValue, typeHierarchy);
- types[i] = param.defaultValue;
+ final argType = param.defaultValue.specialize(typeHierarchy);
+ param._observeArgumentType(argType, typeHierarchy);
+ types[i] = argType;
}
}
assertx(argIndex == namedArgCount);
diff --git a/pkg/vm/lib/transformations/type_flow/types.dart b/pkg/vm/lib/transformations/type_flow/types.dart
index a964682..2100c02 100644
--- a/pkg/vm/lib/transformations/type_flow/types.dart
+++ b/pkg/vm/lib/transformations/type_flow/types.dart
@@ -101,6 +101,12 @@
/// Order of precedence for evaluation of union/intersection.
int get order;
+ /// Returns true iff this type is fully specialized.
+ bool get isSpecialized => true;
+
+ /// Returns specialization of this type using the given [TypeHierarchy].
+ Type specialize(TypeHierarchy typeHierarchy) => this;
+
/// Calculate union of this and [other] types.
Type union(Type other, TypeHierarchy typeHierarchy);
@@ -165,6 +171,14 @@
int get order => TypeOrder.Nullable.index;
@override
+ bool get isSpecialized => baseType.isSpecialized;
+
+ @override
+ Type specialize(TypeHierarchy typeHierarchy) => baseType.isSpecialized
+ ? this
+ : new NullableType(baseType.specialize(typeHierarchy));
+
+ @override
Type union(Type other, TypeHierarchy typeHierarchy) {
if (other.order < this.order) {
return other.union(this, typeHierarchy);
@@ -396,6 +410,13 @@
int get order => TypeOrder.Cone.index;
@override
+ bool get isSpecialized => false;
+
+ @override
+ Type specialize(TypeHierarchy typeHierarchy) =>
+ typeHierarchy.specializeTypeCone(dartType);
+
+ @override
Type union(Type other, TypeHierarchy typeHierarchy) {
if (other.order < this.order) {
return other.union(this, typeHierarchy);
diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/invalidation_set_field2.dart b/pkg/vm/testcases/transformations/type_flow/transformer/invalidation_set_field2.dart
new file mode 100644
index 0000000..12cc624
--- /dev/null
+++ b/pkg/vm/testcases/transformations/type_flow/transformer/invalidation_set_field2.dart
@@ -0,0 +1,36 @@
+// Copyright (c) 2018, 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 T1 {
+ T3 go() => new T3();
+}
+
+class T2 {}
+
+class T3 {
+ run() {
+ print('hi');
+ }
+}
+
+class Q<T> {
+ final T result;
+ Q(this.result);
+}
+
+foo1(List<T1> list) {
+ list.map((T1 t1) => new Q<T1>(t1)).first.result.go().run();
+}
+
+Q foo2NewValue() => new Q<T2>(new T2());
+
+foo3NewT1() {
+ new T1();
+}
+
+main(List<String> args) {
+ foo1([]);
+ foo2NewValue();
+ foo3NewT1();
+}
diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/invalidation_set_field2.dart.expect b/pkg/vm/testcases/transformations/type_flow/transformer/invalidation_set_field2.dart.expect
new file mode 100644
index 0000000..ac18742
--- /dev/null
+++ b/pkg/vm/testcases/transformations/type_flow/transformer/invalidation_set_field2.dart.expect
@@ -0,0 +1,43 @@
+library #lib;
+import self as self;
+import "dart:core" as core;
+
+class T1 extends core::Object {
+ synthetic constructor •() → void
+ : super core::Object::•()
+ ;
+ method go() → self::T3
+ return new self::T3::•();
+}
+class T2 extends core::Object {
+ synthetic constructor •() → void
+ : super core::Object::•()
+ ;
+}
+class T3 extends core::Object {
+ synthetic constructor •() → void
+ : super core::Object::•()
+ ;
+ method run() → dynamic {
+ core::print("hi");
+ }
+}
+class Q<T extends core::Object = dynamic> extends core::Object {
+ final field self::Q::T result;
+ constructor •(self::Q::T result) → void
+ : self::Q::result = result, super core::Object::•()
+ ;
+}
+static method foo1(core::List<self::T1> list) → dynamic {
+ [@vm.direct-call.metadata=#lib::T3::run] [@vm.direct-call.metadata=#lib::T1::go??] [@vm.inferred-type.metadata=#lib::T3] [@vm.direct-call.metadata=#lib::Q::result??] [@vm.inferred-type.metadata=!] list.{core::Iterable::map}<self::Q<self::T1>>((self::T1 t1) → self::Q<self::T1> => new self::Q::•<self::T1>(t1)).{core::Iterable::first}.{self::Q::result}.{self::T1::go}().{self::T3::run}();
+}
+static method foo2NewValue() → self::Q<dynamic>
+ return new self::Q::•<self::T2>(new self::T2::•());
+static method foo3NewT1() → dynamic {
+ new self::T1::•();
+}
+static method main(core::List<core::String> args) → dynamic {
+ self::foo1(<self::T1>[]);
+ self::foo2NewValue();
+ self::foo3NewT1();
+}
