Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit bb56d45

Browse files
johnniwinthercommit-bot@chromium.org
authored andcommitted
Fix receiver type for js-interop access
This includes the unknown potential targets of access to js-interop members. Since we don't know actual classes implementing the js-interop classes we just assume it could be any of them. Change-Id: I4d91ab673fa8221eb701b34e9c32fd16e5a1c381 Reviewed-on: https://dart-review.googlesource.com/74980 Reviewed-by: Stephen Adams <[email protected]> Commit-Queue: Johnni Winther <[email protected]>
1 parent 8dea9aa commit bb56d45

File tree

5 files changed

+68
-12
lines changed

5 files changed

+68
-12
lines changed

pkg/compiler/lib/src/ssa/kernel_impact.dart

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import '../constants/values.dart';
1414
import '../elements/entities.dart';
1515
import '../elements/types.dart';
1616
import '../ir/util.dart';
17+
import '../js_backend/native_data.dart';
1718
import '../kernel/element_map.dart';
1819
import '../kernel/runtime_type_analysis.dart';
1920
import '../options.dart';
@@ -58,6 +59,8 @@ class KernelImpactBuilder extends ir.Visitor {
5859

5960
CommonElements get commonElements => elementMap.commonElements;
6061

62+
NativeBasicData get _nativeBasicData => elementMap.nativeBasicData;
63+
6164
/// Add a checked-mode type use of [type] if it is not `dynamic`.
6265
DartType checkType(ir.DartType irType, TypeUseKind kind) {
6366
DartType type = elementMap.getDartType(irType);
@@ -117,7 +120,7 @@ class KernelImpactBuilder extends ir.Visitor {
117120
if (field.isInstanceMember &&
118121
elementMap.isNativeClass(field.enclosingClass)) {
119122
MemberEntity member = elementMap.getMember(field);
120-
bool isJsInterop = elementMap.nativeBasicData.isJsInteropMember(member);
123+
bool isJsInterop = _nativeBasicData.isJsInteropMember(member);
121124
impactBuilder.registerNativeData(elementMap
122125
.getNativeBehaviorForFieldLoad(field, isJsInterop: isJsInterop));
123126
impactBuilder
@@ -132,7 +135,7 @@ class KernelImpactBuilder extends ir.Visitor {
132135
visitNode(constructor.function.body);
133136
MemberEntity member = elementMap.getMember(constructor);
134137
if (constructor.isExternal && !commonElements.isForeignHelper(member)) {
135-
bool isJsInterop = elementMap.nativeBasicData.isJsInteropMember(member);
138+
bool isJsInterop = _nativeBasicData.isJsInteropMember(member);
136139
impactBuilder.registerNativeData(elementMap
137140
.getNativeBehaviorForMethod(constructor, isJsInterop: isJsInterop));
138141
}
@@ -187,7 +190,7 @@ class KernelImpactBuilder extends ir.Visitor {
187190
handleAsyncMarker(procedure.function);
188191
MemberEntity member = elementMap.getMember(procedure);
189192
if (procedure.isExternal && !commonElements.isForeignHelper(member)) {
190-
bool isJsInterop = elementMap.nativeBasicData.isJsInteropMember(member);
193+
bool isJsInterop = _nativeBasicData.isJsInteropMember(member);
191194
impactBuilder.registerNativeData(elementMap
192195
.getNativeBehaviorForMethod(procedure, isJsInterop: isJsInterop));
193196
}
@@ -493,7 +496,8 @@ class KernelImpactBuilder extends ir.Visitor {
493496
// TODO(johnniwinther): Restrict the dynamic use to only match the known
494497
// target.
495498
// TODO(johnniwinther): Restrict this to subclasses?
496-
Object constraint = new StrongModeConstraint(member.enclosingClass);
499+
Object constraint = new StrongModeConstraint(
500+
commonElements, _nativeBasicData, member.enclosingClass);
497501
impactBuilder.registerDynamicUse(new ConstrainedDynamicUse(
498502
new Selector.call(
499503
member.memberName, elementMap.getCallStructure(node.arguments)),
@@ -606,7 +610,8 @@ class KernelImpactBuilder extends ir.Visitor {
606610
Object constraint;
607611
DartType receiverType = elementMap.getStaticType(node.receiver);
608612
if (receiverType is InterfaceType) {
609-
constraint = new StrongModeConstraint(receiverType.element);
613+
constraint = new StrongModeConstraint(
614+
commonElements, _nativeBasicData, receiverType.element);
610615
}
611616

612617
if (interfaceTarget is ir.Field ||
@@ -621,7 +626,8 @@ class KernelImpactBuilder extends ir.Visitor {
621626
DartType receiverType =
622627
elementMap.getDartType(interfaceTarget.getterType);
623628
if (receiverType is InterfaceType) {
624-
getterConstraint = new StrongModeConstraint(receiverType.element);
629+
getterConstraint = new StrongModeConstraint(
630+
commonElements, _nativeBasicData, receiverType.element);
625631
}
626632
}
627633

@@ -641,7 +647,8 @@ class KernelImpactBuilder extends ir.Visitor {
641647
Object constraint;
642648
DartType receiverType = elementMap.getStaticType(node.receiver);
643649
if (receiverType is InterfaceType) {
644-
constraint = new StrongModeConstraint(receiverType.element);
650+
constraint = new StrongModeConstraint(
651+
commonElements, _nativeBasicData, receiverType.element);
645652
}
646653
impactBuilder.registerDynamicUse(new ConstrainedDynamicUse(
647654
new Selector.getter(elementMap.getName(node.name)),
@@ -680,7 +687,8 @@ class KernelImpactBuilder extends ir.Visitor {
680687
Object constraint;
681688
DartType receiverType = elementMap.getStaticType(node.receiver);
682689
if (receiverType is InterfaceType) {
683-
constraint = new StrongModeConstraint(receiverType.element);
690+
constraint = new StrongModeConstraint(
691+
commonElements, _nativeBasicData, receiverType.element);
684692
}
685693
impactBuilder.registerDynamicUse(new ConstrainedDynamicUse(
686694
new Selector.setter(elementMap.getName(node.name)),

pkg/compiler/lib/src/universe/world_builder.dart

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,17 @@ class StrongModeWorldConstraints extends UniverseSelectorConstraints {
192192
class StrongModeConstraint {
193193
final ClassEntity cls;
194194

195-
const StrongModeConstraint(this.cls);
195+
factory StrongModeConstraint(CommonElements commonElements,
196+
NativeBasicData nativeBasicData, ClassEntity cls) {
197+
if (nativeBasicData.isJsInteropClass(cls)) {
198+
// We can not tell js-interop classes apart, so we just assume the
199+
// receiver could be any js-interop class.
200+
cls = commonElements.jsJavaScriptObjectClass;
201+
}
202+
return new StrongModeConstraint.internal(cls);
203+
}
204+
205+
const StrongModeConstraint.internal(this.cls);
196206

197207
bool needsNoSuchMethodHandling(Selector selector, World world) => true;
198208

tests/compiler/dart2js/impact/data/jsinterop.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class JsInteropClass {
4444
}
4545

4646
/*strong.element: testJsInteropClass:
47-
dynamic=[JsInteropClass.method(0)],
47+
dynamic=[JavaScriptObject.method(0)],
4848
static=[JsInteropClass.(0)]
4949
*/
5050
testJsInteropClass() => new JsInteropClass().method();
@@ -76,7 +76,7 @@ class GenericClass<T> {
7676
}
7777

7878
/*strong.element: testOptionalGenericFunctionTypeArgument:
79-
dynamic=[GenericClass.method(0)],
79+
dynamic=[JavaScriptObject.method(0)],
8080
static=[GenericClass.(0)]
8181
*/
8282
testOptionalGenericFunctionTypeArgument() => new GenericClass().method();

tests/compiler/dart2js/model/enqueuer_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ main() {}
250250
checkInvariant(enqueuer, elementEnvironment);
251251

252252
Object createConstraint(ClassEntity cls) {
253-
return new StrongModeConstraint(cls);
253+
return new StrongModeConstraint(compiler.frontendStrategy.commonElements,
254+
compiler.frontendStrategy.nativeBasicData, cls);
254255
}
255256

256257
for (Impact impact in impacts) {
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Test that methods implemented (not extended) in js-interop classes are still
6+
// considered live.
7+
8+
@JS()
9+
library anonymous_js_test;
10+
11+
import 'package:js/js.dart';
12+
13+
@JS()
14+
@anonymous
15+
abstract class A {
16+
external factory A();
17+
external String get a;
18+
external set a(String a);
19+
}
20+
21+
@JS()
22+
@anonymous
23+
abstract class B implements A {
24+
external factory B();
25+
external String get b;
26+
external set b(String b);
27+
}
28+
29+
void main() {
30+
final b = B();
31+
// This setter is missing if we don't assume the receiver could be an
32+
// unknown but concrete implementation of A.
33+
b.a = 'Hi';
34+
b.b = 'Hello';
35+
if (b.a != 'Hi') throw 'b.a';
36+
if (b.b != 'Hello') throw 'b.b';
37+
}

0 commit comments

Comments
 (0)