Skip to content

Commit d4adfcc

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
[dart:js_interop/ddc] Lower static interop using call-site and omit tear-offs
Since tear-offs of static interop members are disallowed, we can avoid modifying external procedures and just modify the invocation instead. This allows us to avoid emitting external procedures and remove unnecessary tear-offs in DDC. It also results in more concise calling code. Call-site lowerings are implemented using closures that take args from an existing invocation, and return a new inlined invocation. Change-Id: I7585d8db0378c4058ad23d452e7f47ed960b194a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303742 Reviewed-by: Nicholas Shahan <[email protected]> Commit-Queue: Srujan Gaddam <[email protected]> Reviewed-by: Joshua Litt <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent c98ec26 commit d4adfcc

File tree

15 files changed

+472
-503
lines changed

15 files changed

+472
-503
lines changed

‎pkg/_js_interop_checks/lib/js_interop_checks.dart

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class JsInteropChecks extends RecursiveVisitor {
102102
/// types.
103103
static const Iterable<String> _customStaticInteropImplementations = [
104104
'js_interop',
105+
'js_interop_unsafe',
105106
];
106107

107108
/// Libraries that cannot be used when [_nonStrictModeIsAllowed] is false.
@@ -144,6 +145,14 @@ class JsInteropChecks extends RecursiveVisitor {
144145
_staticTypeContext = StatefulStaticTypeContext.stacked(
145146
TypeEnvironment(_coreTypes, hierarchy));
146147

148+
/// Verifies given [member] is an external extension member on a static
149+
/// interop type that needs custom behavior.
150+
static bool isAllowedCustomStaticInteropImplementation(Member member) {
151+
Uri uri = member.enclosingLibrary.importUri;
152+
return uri.isScheme('dart') &&
153+
_customStaticInteropImplementations.contains(uri.path);
154+
}
155+
147156
/// Extract all native class names from the [component].
148157
///
149158
/// Returns a map from the name to the underlying Class node. This is a
@@ -351,7 +360,7 @@ class JsInteropChecks extends RecursiveVisitor {
351360
if (annotatable != null) {
352361
// If a @staticInterop member, check that it uses no type parameters.
353362
if (hasStaticInteropAnnotation(annotatable) &&
354-
!_isAllowedCustomStaticInteropImplementation(node)) {
363+
!isAllowedCustomStaticInteropImplementation(node)) {
355364
_checkStaticInteropMemberUsesNoTypeParameters(node);
356365
}
357366
// We do not support external extension members with the 'static'
@@ -579,15 +588,6 @@ class JsInteropChecks extends RecursiveVisitor {
579588

580589
// JS interop member checks
581590

582-
/// Verifies given [member] is an external extension member on a static
583-
/// interop type that needs custom behavior.
584-
bool _isAllowedCustomStaticInteropImplementation(Member member) {
585-
Uri uri = member.enclosingLibrary.importUri;
586-
return uri.isScheme('dart') &&
587-
_customStaticInteropImplementations.contains(uri.path) ||
588-
_allowedNativeTestPatterns.any((pattern) => uri.path.contains(pattern));
589-
}
590-
591591
/// Verifies given [member] is one of the allowed usages of external:
592592
/// a dart low level library, a foreign helper, a native test,
593593
/// or a from environment constructor.

‎pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart

Lines changed: 223 additions & 256 deletions
Large diffs are not rendered by default.

‎pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import 'dart:convert';
77
import 'dart:io' as io;
88
import 'dart:math' show max, min;
99

10+
import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'
11+
show InlineExtensionIndex;
1012
import 'package:_js_interop_checks/src/transformations/static_interop_class_eraser.dart'
1113
show eraseStaticInteropTypesForJSCompilers;
1214
import 'package:collection/collection.dart'
@@ -273,6 +275,12 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
273275
/// Maps uri strings in asserts and elsewhere to hoisted identifiers.
274276
var _uriContainer = ModuleItemContainer<String>.asArray('I');
275277

278+
/// Index of inline and extension members in order to filter static interop
279+
/// members.
280+
// TODO(srujzs): Is there some way to share this from the js_util_optimizer to
281+
// avoid having to recompute?
282+
final _inlineExtensionIndex = InlineExtensionIndex();
283+
276284
final Class _jsArrayClass;
277285
final Class _privateSymbolClass;
278286
final Class _linkedHashMapImplClass;
@@ -934,6 +942,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
934942
var nonExternalMethods = <js_ast.Method>[];
935943
for (var procedure in c.procedures) {
936944
if (procedure.isExternal) continue;
945+
// Don't emit tear-offs for @staticInterop members as they're disallowed.
946+
if (_isStaticInteropTearOff(procedure)) continue;
937947
if (procedure.isFactory && !procedure.isRedirectingFactory) {
938948
// Skip redirecting factories (they've already been resolved).
939949
var factory = _emitFactoryConstructor(procedure);
@@ -2985,7 +2995,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
29852995

29862996
void _emitLibraryProcedures(Library library) {
29872997
var procedures = library.procedures
2988-
.where((p) => !p.isExternal && !p.isAbstract)
2998+
.where((p) =>
2999+
!p.isExternal && !p.isAbstract && !_isStaticInteropTearOff(p))
29893000
.toList();
29903001
moduleItems.addAll(procedures
29913002
.where((p) => !p.isAccessor)
@@ -2994,6 +3005,44 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
29943005
_emitLibraryAccessors(procedures.where((p) => p.isAccessor).toList());
29953006
}
29963007

3008+
/// Check whether [p] is a tear-off for an external or synthetic static
3009+
/// interop member.
3010+
///
3011+
/// Users are disallowed from using these tear-offs, so we should avoid
3012+
/// emitting them.
3013+
bool _isStaticInteropTearOff(Procedure p) {
3014+
final extensionMember =
3015+
_inlineExtensionIndex.getExtensionMemberForTearOff(p);
3016+
if (extensionMember != null && extensionMember.asProcedure.isExternal) {
3017+
return true;
3018+
}
3019+
final inlineMember = _inlineExtensionIndex.getInlineMemberForTearOff(p);
3020+
if (inlineMember != null && inlineMember.asProcedure.isExternal) {
3021+
return true;
3022+
}
3023+
final enclosingClass = p.enclosingClass;
3024+
if (enclosingClass != null && isStaticInteropType(enclosingClass)) {
3025+
// @staticInterop types can't use generative constructors, so we only
3026+
// check for tear-offs of factories. The one exception is a tear-off of a
3027+
// default constructor, which is disallowed on @staticInterop classes.
3028+
final factoryName = extractConstructorNameFromTearOff(p.name);
3029+
if (factoryName != null) {
3030+
if (factoryName.isEmpty &&
3031+
enclosingClass.constructors.any((constructor) =>
3032+
constructor.isSynthetic && constructor.name.text.isEmpty)) {
3033+
return true;
3034+
}
3035+
if (enclosingClass.procedures.any((procedure) =>
3036+
procedure.isFactory &&
3037+
procedure.isExternal &&
3038+
procedure.name.text == factoryName)) {
3039+
return true;
3040+
}
3041+
}
3042+
}
3043+
return false;
3044+
}
3045+
29973046
void _emitLibraryAccessors(Iterable<Procedure> accessors) {
29983047
if (accessors.isEmpty) return;
29993048
moduleItems.add(runtimeStatement('copyProperties(#, { # })', [

‎pkg/front_end/testcases/dart2js/inline_class/external.dart.strong.transformed.expect

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -33,57 +33,43 @@ inline class B /* declaredRepresentationType = self::A */ {
3333
constructor named = self::B|named;
3434
tearoff named = self::B|_#named#tearOff;
3535
}
36-
static method B|(self::A a) → self::B
37-
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), a);
36+
external static method B|(self::A a) → self::B;
3837
static method B|_#new#tearOff(self::A a) → self::B
39-
return self::B|(a);
40-
static method B|named(core::int i) → self::B
41-
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), i);
38+
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), a);
39+
external static method B|named(core::int i) → self::B;
4240
static method B|_#named#tearOff(core::int i) → self::B
43-
return self::B|named(i);
44-
static method B|get#field(lowered self::A #this) → self::A
45-
return js_2::getProperty<self::A>(#this, "field");
46-
static method B|set#field(lowered self::A #this, self::A #externalFieldValue) → void
47-
return js_2::_setPropertyUnchecked<self::A>(#this, "field", #externalFieldValue);
48-
static method B|method(lowered final self::B #this) → self::A
49-
return js_2::_callMethodUnchecked0<self::A>(#this, "method");
41+
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), i);
42+
external static method B|get#field(lowered self::A #this) → self::A;
43+
external static method B|set#field(lowered self::A #this, self::A #externalFieldValue) → void;
44+
external static method B|method(lowered final self::B #this) → self::A;
5045
static method B|get#method(lowered final self::B #this) → () → self::A
51-
return () → self::A => self::B|method(#this);
52-
static method B|genericMethod<T extends core::Object? = dynamic>(lowered final self::B #this, self::B|genericMethod::T% t) → self::B|genericMethod::T%
53-
return js_2::callMethod<self::B|genericMethod::T%>(#this, "genericMethod", <dynamic>[t]);
46+
return () → self::A => js_2::_callMethodUnchecked0<self::A>(#this, "method");
47+
external static method B|genericMethod<T extends core::Object? = dynamic>(lowered final self::B #this, self::B|genericMethod::T% t) → self::B|genericMethod::T%;
5448
static method B|get#genericMethod(lowered final self::B #this) → <T extends core::Object? = dynamic>(T%) → T%
55-
return <T extends core::Object? = dynamic>(T% t) → T% => self::B|genericMethod<T%>(#this, t);
56-
static method B|get#getter(lowered final self::B #this) → self::B
57-
return js_2::getProperty<self::B>(#this, "getter");
58-
static method B|set#setter(lowered final self::B #this, self::B b) → void
59-
return js_2::setProperty<self::B>(#this, "setter", b);
60-
static get B|staticField() → self::A
61-
return js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField");
62-
static set B|staticField(self::A #externalFieldValue) → void
63-
return js_2::_setPropertyUnchecked<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField", #externalFieldValue);
64-
static method B|staticMethod() → self::A
65-
return js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticMethod");
66-
static method B|staticGenericMethod<T extends core::Object? = dynamic>(self::B|staticGenericMethod::T% t) → self::B|staticGenericMethod::T%
67-
return js_2::callMethod<self::B|staticGenericMethod::T%>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGenericMethod", <dynamic>[t]);
68-
static get B|staticGetter() → self::B
69-
return js_2::getProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGetter");
70-
static set B|staticSetter(self::B b) → void
71-
return js_2::setProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticSetter", b);
49+
return <T extends core::Object? = dynamic>(T% t) → T% => js_2::callMethod<T%>(#this, "genericMethod", <dynamic>[t]);
50+
external static method B|get#getter(lowered final self::B #this) → self::B;
51+
external static method B|set#setter(lowered final self::B #this, self::B b) → void;
52+
external static get B|staticField() → self::A;
53+
external static set B|staticField(self::A #externalFieldValue) → void;
54+
external static method B|staticMethod() → self::A;
55+
external static method B|staticGenericMethod<T extends core::Object? = dynamic>(self::B|staticGenericMethod::T% t) → self::B|staticGenericMethod::T%;
56+
external static get B|staticGetter() → self::B;
57+
external static set B|staticSetter(self::B b) → void;
7258
static method method(self::A a) → void {
73-
self::B b1 = self::B|(a);
74-
self::B b2 = self::B|named(0);
75-
a = self::B|get#field(b1);
76-
self::B|set#field(b1, a);
77-
a = self::B|method(b1);
78-
b2 = self::B|genericMethod<self::B>(b2, b2);
79-
b1 = self::B|get#getter(b2);
80-
self::B|set#setter(b1, b2);
81-
a = self::B|staticField;
82-
self::B|staticField = a;
83-
a = self::B|staticMethod();
84-
b2 = self::B|staticGenericMethod<self::B>(b2);
85-
b1 = self::B|staticGetter;
86-
self::B|staticSetter = b2;
59+
self::B b1 = js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), a);
60+
self::B b2 = js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), 0);
61+
a = js_2::getProperty<self::A>(b1, "field");
62+
js_2::setProperty<self::A>(b1, "field", a);
63+
a = js_2::_callMethodUnchecked0<self::A>(b1, "method");
64+
b2 = js_2::callMethod<self::B>(b2, "genericMethod", <dynamic>[b2]);
65+
b1 = js_2::getProperty<self::B>(b2, "getter");
66+
js_2::setProperty<self::B>(b1, "setter", b2);
67+
a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField");
68+
js_2::setProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField", a);
69+
a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticMethod");
70+
b2 = js_2::callMethod<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGenericMethod", <dynamic>[b2]);
71+
b1 = js_2::getProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGetter");
72+
js_2::setProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticSetter", b2);
8773
}
8874

8975
constants {

‎pkg/front_end/testcases/dart2js/inline_class/external.dart.weak.transformed.expect

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -33,57 +33,43 @@ inline class B /* declaredRepresentationType = self::A */ {
3333
constructor named = self::B|named;
3434
tearoff named = self::B|_#named#tearOff;
3535
}
36-
static method B|(self::A a) → self::B
37-
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), a);
36+
external static method B|(self::A a) → self::B;
3837
static method B|_#new#tearOff(self::A a) → self::B
39-
return self::B|(a);
40-
static method B|named(core::int i) → self::B
41-
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), i);
38+
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), a);
39+
external static method B|named(core::int i) → self::B;
4240
static method B|_#named#tearOff(core::int i) → self::B
43-
return self::B|named(i);
44-
static method B|get#field(lowered self::A #this) → self::A
45-
return js_2::getProperty<self::A>(#this, "field");
46-
static method B|set#field(lowered self::A #this, self::A #externalFieldValue) → void
47-
return js_2::_setPropertyUnchecked<self::A>(#this, "field", #externalFieldValue);
48-
static method B|method(lowered final self::B #this) → self::A
49-
return js_2::_callMethodUnchecked0<self::A>(#this, "method");
41+
return js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), i);
42+
external static method B|get#field(lowered self::A #this) → self::A;
43+
external static method B|set#field(lowered self::A #this, self::A #externalFieldValue) → void;
44+
external static method B|method(lowered final self::B #this) → self::A;
5045
static method B|get#method(lowered final self::B #this) → () → self::A
51-
return () → self::A => self::B|method(#this);
52-
static method B|genericMethod<T extends core::Object? = dynamic>(lowered final self::B #this, self::B|genericMethod::T% t) → self::B|genericMethod::T%
53-
return js_2::callMethod<self::B|genericMethod::T%>(#this, "genericMethod", <dynamic>[t]);
46+
return () → self::A => js_2::_callMethodUnchecked0<self::A>(#this, "method");
47+
external static method B|genericMethod<T extends core::Object? = dynamic>(lowered final self::B #this, self::B|genericMethod::T% t) → self::B|genericMethod::T%;
5448
static method B|get#genericMethod(lowered final self::B #this) → <T extends core::Object? = dynamic>(T%) → T%
55-
return <T extends core::Object? = dynamic>(T% t) → T% => self::B|genericMethod<T%>(#this, t);
56-
static method B|get#getter(lowered final self::B #this) → self::B
57-
return js_2::getProperty<self::B>(#this, "getter");
58-
static method B|set#setter(lowered final self::B #this, self::B b) → void
59-
return js_2::setProperty<self::B>(#this, "setter", b);
60-
static get B|staticField() → self::A
61-
return js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField");
62-
static set B|staticField(self::A #externalFieldValue) → void
63-
return js_2::_setPropertyUnchecked<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField", #externalFieldValue);
64-
static method B|staticMethod() → self::A
65-
return js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticMethod");
66-
static method B|staticGenericMethod<T extends core::Object? = dynamic>(self::B|staticGenericMethod::T% t) → self::B|staticGenericMethod::T%
67-
return js_2::callMethod<self::B|staticGenericMethod::T%>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGenericMethod", <dynamic>[t]);
68-
static get B|staticGetter() → self::B
69-
return js_2::getProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGetter");
70-
static set B|staticSetter(self::B b) → void
71-
return js_2::setProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticSetter", b);
49+
return <T extends core::Object? = dynamic>(T% t) → T% => js_2::callMethod<T%>(#this, "genericMethod", <dynamic>[t]);
50+
external static method B|get#getter(lowered final self::B #this) → self::B;
51+
external static method B|set#setter(lowered final self::B #this, self::B b) → void;
52+
external static get B|staticField() → self::A;
53+
external static set B|staticField(self::A #externalFieldValue) → void;
54+
external static method B|staticMethod() → self::A;
55+
external static method B|staticGenericMethod<T extends core::Object? = dynamic>(self::B|staticGenericMethod::T% t) → self::B|staticGenericMethod::T%;
56+
external static get B|staticGetter() → self::B;
57+
external static set B|staticSetter(self::B b) → void;
7258
static method method(self::A a) → void {
73-
self::B b1 = self::B|(a);
74-
self::B b2 = self::B|named(0);
75-
a = self::B|get#field(b1);
76-
self::B|set#field(b1, a);
77-
a = self::B|method(b1);
78-
b2 = self::B|genericMethod<self::B>(b2, b2);
79-
b1 = self::B|get#getter(b2);
80-
self::B|set#setter(b1, b2);
81-
a = self::B|staticField;
82-
self::B|staticField = a;
83-
a = self::B|staticMethod();
84-
b2 = self::B|staticGenericMethod<self::B>(b2);
85-
b1 = self::B|staticGetter;
86-
self::B|staticSetter = b2;
59+
self::B b1 = js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), a);
60+
self::B b2 = js_2::_callConstructorUnchecked1<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), 0);
61+
a = js_2::getProperty<self::A>(b1, "field");
62+
js_2::setProperty<self::A>(b1, "field", a);
63+
a = js_2::_callMethodUnchecked0<self::A>(b1, "method");
64+
b2 = js_2::callMethod<self::B>(b2, "genericMethod", <dynamic>[b2]);
65+
b1 = js_2::getProperty<self::B>(b2, "getter");
66+
js_2::setProperty<self::B>(b1, "setter", b2);
67+
a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField");
68+
js_2::setProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticField", a);
69+
a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticMethod");
70+
b2 = js_2::callMethod<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGenericMethod", <dynamic>[b2]);
71+
b1 = js_2::getProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticGetter");
72+
js_2::setProperty<self::B>(js_2::_getPropertyTrustType<core::Object>(js_2::globalThis, "B"), "staticSetter", b2);
8773
}
8874

8975
constants {

0 commit comments

Comments
 (0)