[dart2js] Fix linearization issues.
- For parameters of closurized members make sure to mark the virtual parameter as closurized where appropriate. Closurized parameters are treated as dynamic and this needs to propagate to all the virtual target's overrides. Previously only the concrete target was getting marked as closurized.
- For mixins defining an abstract member, foo, implementations of foo (either directly on the mixin target or superclasses of it) should propagate their types to the abstract foo as they are effectively overriding it. Calls to foo within the body of the mixin can only target the abstract foo with a virtual call. So that virtual target needs to reflect the types of all its overrides.
Tests have been added that capture both of these cases. The values here reflect the non-linearized algorithm but prior to these fixes the linearized algorithms showed a diff for both tests. After these changes there is no diff.
This fixes all failures referenced in b/277876666.
Change-Id: Icaca99cb6902c54f481eebdbb65cbd7cf81aa2c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294960
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
diff --git a/pkg/compiler/lib/src/inferrer_experimental/engine.dart b/pkg/compiler/lib/src/inferrer_experimental/engine.dart
index e9e3a2f..ce828eb 100644
--- a/pkg/compiler/lib/src/inferrer_experimental/engine.dart
+++ b/pkg/compiler/lib/src/inferrer_experimental/engine.dart
@@ -839,7 +839,7 @@
? types.getInferredTypeOfVirtualMember(callee)
: types.getInferredTypeOfMember(callee);
_markForClosurization(memberInfo, callSiteType,
- remove: remove, addToQueue: addToQueue);
+ remove: remove, addToQueue: addToQueue, isVirtualCall: virtualCall);
return true;
} else {
final method = callee as FunctionEntity;
@@ -1012,7 +1012,9 @@
void _markForClosurization(
MemberTypeInformation memberInfo, TypeInformation callSiteType,
- {required bool remove, required bool addToQueue}) {
+ {required bool remove,
+ required bool addToQueue,
+ required bool isVirtualCall}) {
final member = memberInfo.member;
if (remove) {
memberInfo.closurizedCount--;
@@ -1027,8 +1029,9 @@
}
types.strategy.forEachParameter(member as FunctionEntity,
(Local parameter) {
- ParameterTypeInformation info =
- types.getInferredTypeOfParameter(parameter);
+ ParameterTypeInformation info = isVirtualCall
+ ? types.getInferredTypeOfVirtualParameter(parameter)
+ : types.getInferredTypeOfParameter(parameter);
info.tagAsTearOffClosureParameter(this);
if (addToQueue) _workQueue.add(info);
});
@@ -1054,7 +1057,9 @@
if (needsClosurization) {
_markForClosurization(info, callSiteType,
- remove: false, addToQueue: false);
+ remove: false,
+ addToQueue: false,
+ isVirtualCall: target.isVirtual);
}
}
return true;
diff --git a/pkg/compiler/lib/src/universe/member_hierarchy.dart b/pkg/compiler/lib/src/universe/member_hierarchy.dart
index a29833b..d36780e 100644
--- a/pkg/compiler/lib/src/universe/member_hierarchy.dart
+++ b/pkg/compiler/lib/src/universe/member_hierarchy.dart
@@ -357,17 +357,29 @@
}
void _handleMember(MemberEntity member, ClassEntity cls, Selector selector,
- void Function(MemberEntity parent, MemberEntity override) join) {
+ void Function(MemberEntity parent, MemberEntity override) join,
+ {required bool isMixinUse}) {
final elementEnv = closedWorld.elementEnvironment;
final name = selector.memberName;
bool foundSuperclass = false;
- void addParent(MemberEntity child, ClassEntity childCls,
- MemberEntity parent, ClassEntity parentCls) {
+ void addParent(MemberEntity child, MemberEntity parent) {
if (child == parent) return;
- if ((_overrides[parent] ??= Setlet()).add(child)) {
+ if (!isMixinUse && (_overrides[parent] ??= Setlet()).add(child)) {
join(parent, child);
}
+
+ // For mixins defining an abstract member, foo, implementations of foo
+ // (either directly on the mixin target or superclasses of it) should
+ // propagate their types to the abstract foo as they are effectively
+ // overriding it. Calls to foo within the body of the mixin can only
+ // target the abstract foo with a virtual call so that virtual target
+ // needs to reflect the types of all its overrides.
+ if (isMixinUse &&
+ child.isAbstract &&
+ (_overrides[child] ??= Setlet()).add(parent)) {
+ join(child, parent);
+ }
}
// Check each superclass in ascending order until we find an ancestor.
@@ -375,7 +387,7 @@
while (current != null) {
final match = elementEnv.lookupLocalClassMember(current, name);
if (match != null && !MemberHierarchyBuilder._skipMemberInternal(match)) {
- addParent(member, cls, match, current);
+ addParent(member, match);
foundSuperclass = true;
break;
}
@@ -384,8 +396,7 @@
closedWorld.classHierarchy.getClassSet(cls).forEachSubtype((subtype) {
final override = elementEnv.lookupClassMember(subtype, name);
- if (override == null) return IterationStep.CONTINUE;
- addParent(override, subtype, member, cls);
+ if (override != null) addParent(override, member);
return IterationStep.CONTINUE;
}, ClassHierarchyNode.INSTANTIATED, strict: true);
@@ -402,9 +413,11 @@
// Process each selector matching member separately.
for (final selector in _selectorsForMember(member)) {
for (final mixinUse in mixinUses) {
- _handleMember(member, mixinUse, selector, join);
+ // For mixin uses we treat the mixin's members as if they are part of
+ // the mixin target itself.
+ _handleMember(member, mixinUse, selector, join, isMixinUse: true);
}
- _handleMember(member, cls, selector, join);
+ _handleMember(member, cls, selector, join, isMixinUse: false);
}
}
diff --git a/pkg/compiler/test/inference/data/abstract_mixin_override.dart b/pkg/compiler/test/inference/data/abstract_mixin_override.dart
new file mode 100644
index 0000000..684bf91
--- /dev/null
+++ b/pkg/compiler/test/inference/data/abstract_mixin_override.dart
@@ -0,0 +1,63 @@
+// Copyright (c) 2023, 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.
+
+// @dart = 2.7
+
+/*member: A1.:[exact=A1]*/
+class A1 {
+ /*member: A1.foo:[null]*/
+ void foo(/*[exact=JSUInt31]*/ x) => print(x);
+}
+
+/*member: B1.:[exact=B1]*/
+class B1 extends A1 with M1 {}
+
+/*member: C1.:[exact=C1]*/
+class C1 with M1 {
+ /*member: C1.foo:[null]*/
+ void foo(/*[exact=JSUInt31]*/ x) => print(x);
+}
+
+mixin M1 {
+ void foo(x);
+ /*member: M1.bar:[null]*/
+ void bar(/*[exact=JSUInt31]*/ y) {
+ /*invoke: [subtype=M1]*/ foo(y);
+ }
+}
+
+/*member: A2.:[exact=A2]*/
+class A2 {
+ /*member: A2.foo:[null]*/
+ void foo(/*Value([exact=JSString], value: "")*/ x) => print(x);
+}
+
+/*member: B2.:[exact=B2]*/
+class B2 extends A2 with M2 {}
+
+/*member: C2.:[exact=C2]*/
+class C2 with M2 {
+ /*member: C2.foo:[null]*/
+ void foo(/*Value([exact=JSString], value: "")*/ x) => print(x);
+}
+
+mixin M2 {
+ /*member: M2.foo:[exact=JSUInt31]*/
+ void foo(/*Value([exact=JSString], value: "")*/ x) => 5;
+ /*member: M2.bar:[null]*/
+ void bar(/*Value([exact=JSString], value: "")*/ y) {
+ /*invoke: [subtype=M2]*/ foo(y);
+ }
+}
+
+/*member: getB1:Union([exact=B1], [exact=C1])*/
+getB1(bool /*Value([exact=JSBool], value: false)*/ x) => x ? B1() : C1();
+/*member: getB2:Union([exact=B2], [exact=C2])*/
+getB2(bool /*Value([exact=JSBool], value: false)*/ x) => x ? B2() : C2();
+
+/*member: main:[null]*/
+main() {
+ getB1(false). /*invoke: Union([exact=B1], [exact=C1])*/ bar(3);
+ getB2(false). /*invoke: Union([exact=B2], [exact=C2])*/ bar("");
+}
diff --git a/pkg/compiler/test/inference/data/closurized_tear_off.dart b/pkg/compiler/test/inference/data/closurized_tear_off.dart
new file mode 100644
index 0000000..6880186
--- /dev/null
+++ b/pkg/compiler/test/inference/data/closurized_tear_off.dart
@@ -0,0 +1,34 @@
+// Copyright (c) 2023, 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.
+
+// @dart = 2.7
+
+/*member: A.:[exact=A]*/
+class A {
+ /*member: A.foo:[exact=JSUInt31]*/
+ foo(/*[null|subclass=Object]*/ a) => 3;
+ /*member: A.bar:[exact=MappedListIterable]*/
+ bar(/*Container([exact=JSExtendableArray], element: [subclass=JSPositiveInt], length: 10)*/ x) =>
+ x. /*invoke: Container([exact=JSExtendableArray], element: [subclass=JSPositiveInt], length: 10)*/ map(
+ /*[subclass=A]*/ foo);
+}
+
+/*member: B.:[exact=B]*/
+class B extends A {
+ /*member: B.foo:[exact=JSUInt31]*/
+ foo(/*[null|subclass=Object]*/ b) {
+ b.abs();
+ return 4;
+ }
+}
+
+/*member: getA:[subclass=A]*/
+getA(bool /*Value([exact=JSBool], value: false)*/ x) => x ? A() : B();
+
+/*member: main:[null]*/
+main() {
+ getA(false). /*invoke: [subclass=A]*/ bar(List
+ . /*update: Container([exact=JSExtendableArray], element: [subclass=JSPositiveInt], length: 10)*/ generate(
+ 10, (i) => i));
+}
