Skip to content

Commit 3bedcdf

Browse files
rakudramaCommit Queue
authored and
Commit Queue
committed
[js_interop] Use synthesized getter for static getters
The advantages of this approach: - Annotations on the original getter are preserved, allowing any dart2js `@pragma` to be honored. - An optimizing compiler (or user via a pragma) can decide to inline or not inline the getter, giving more control over code size. Change-Id: I719b41640b75634fe00f6b87f95ebca1801cd159 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/418880 Reviewed-by: Srujan Gaddam <[email protected]> Reviewed-by: Paul Berry <[email protected]> Commit-Queue: Stephen Adams <[email protected]>
1 parent 1540a33 commit 3bedcdf

File tree

2 files changed

+84
-7
lines changed

2 files changed

+84
-7
lines changed

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

+75-1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class JsUtilOptimizer extends Transformer {
6464
final Map<Member, _InvocationBuilder?> _externalInvocationBuilders = {};
6565
final StatefulStaticTypeContext _staticTypeContext;
6666

67+
final bool isDart2JS;
68+
6769
/// Dynamic members in js_util that interop allowed.
6870
static const List<String> _allowedInteropJsUtilMembers = [
6971
'callConstructor',
@@ -78,7 +80,7 @@ class JsUtilOptimizer extends Transformer {
7880
this._coreTypes,
7981
ClassHierarchy hierarchy,
8082
this._extensionIndex, {
81-
required bool isDart2JS,
83+
required this.isDart2JS,
8284
}) : _callMethodTarget = _coreTypes.index.getTopLevelProcedure(
8385
'dart:js_util',
8486
'callMethod',
@@ -212,6 +214,68 @@ class JsUtilOptimizer extends Transformer {
212214
return node;
213215
}
214216

217+
@override
218+
TreeNode visitProcedure(Procedure node) {
219+
if (!isDart2JS) return defaultMember(node);
220+
221+
if (node.isExternal &&
222+
node.isStatic &&
223+
!JsInteropChecks.isPatchedMember(node)) {
224+
if (node.kind == ProcedureKind.Getter) {
225+
final dottedPrefix = _getDottedPrefixForStaticallyResolvableMember(
226+
node,
227+
);
228+
if (dottedPrefix != null) {
229+
// Convert the external getter into a Dart getter with a body of the
230+
// form
231+
//
232+
// => getPropertyTrustType<dynamic>(
233+
// staticInteropGlobalContext(),
234+
// "name"
235+
// ) as <type>;
236+
//
237+
// The type argument is `<dynamic>` so the downcast can be removed
238+
// under `-O3`.
239+
240+
final receiver = _getObjectOffGlobalContext(
241+
node,
242+
dottedPrefix.isEmpty ? [] : dottedPrefix.split('.'),
243+
);
244+
245+
final shouldTrustType =
246+
node.enclosingClass != null &&
247+
hasTrustTypesAnnotation(node.enclosingClass!);
248+
249+
Expression expression = StaticInvocation(
250+
_getPropertyTrustTypeTarget,
251+
Arguments(
252+
[receiver, StringLiteral(_getMemberJSName(node))],
253+
types: [DynamicType()],
254+
),
255+
)..fileOffset = node.fileOffset;
256+
257+
if (!shouldTrustType) {
258+
expression =
259+
AsExpression(expression, node.getterType)
260+
..isTypeError = true
261+
..isForDynamic = true
262+
..fileOffset = node.fileOffset;
263+
}
264+
265+
final statement = ReturnStatement(expression)
266+
..fileOffset = node.fileOffset;
267+
268+
node.function.body = statement;
269+
node.isExternal = false;
270+
} else {
271+
// Leave the static getter member as `external`.
272+
}
273+
}
274+
}
275+
276+
return defaultMember(node);
277+
}
278+
215279
/// Given a static interop procedure [node], return a
216280
/// [_InvocationBuilder] that will create new [StaticInvocation]s that
217281
/// replace calls to [node].
@@ -244,6 +308,16 @@ class JsUtilOptimizer extends Transformer {
244308
node.enclosingClass != null &&
245309
hasTrustTypesAnnotation(node.enclosingClass!);
246310
if (_extensionIndex.isGetter(node)) {
311+
if (isDart2JS) {
312+
// Top-level and static external getters are given a Dart body so
313+
// that annotations can remain attached. The call to the getter
314+
// remains in place. The dart2js optimizer may do inlining.
315+
return null;
316+
}
317+
// For DDC, the call to the getter is replaced with the lowered code
318+
// in this transformer.
319+
// TODO(http://dartbug.com/60505): Consider also using a synthesized
320+
// getter and leaving the call in place.
247321
return _getExternalGetterInvocationBuilder(
248322
node,
249323
shouldTrustType,

pkg/front_end/testcases/dart2js/extension_types/external.dart.strong.transformed.expect

+9-6
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,16 @@ external static extension-type-member method B|get#getter(lowered final self::B%
7070
external static extension-type-member method B|set#setter(lowered final self::B% /* erasure=self::A, declared=! */ #this, self::B% /* erasure=self::A, declared=! */ b) → void;
7171
external static extension-type-member method B|get#property(lowered final self::B% /* erasure=self::A, declared=! */ #this) → self::B% /* erasure=self::A, declared=! */;
7272
external static extension-type-member method B|set#property(lowered final self::B% /* erasure=self::A, declared=! */ #this, self::B% /* erasure=self::A, declared=! */ b) → void;
73-
external static extension-type-member get B|staticField() → self::A;
73+
static extension-type-member get B|staticField() → self::A
74+
return js_2::_getPropertyTrustType<dynamic>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField") as{TypeError,ForDynamic} self::A;
7475
external static extension-type-member set B|staticField(synthesized self::A #externalFieldValue) → void;
7576
external static extension-type-member method B|staticMethod() → self::A;
7677
external static extension-type-member method B|staticGenericMethod<T extends self::B% /* erasure=self::A, declared=! */>(self::B|staticGenericMethod::T% t) → self::B|staticGenericMethod::T%;
77-
external static extension-type-member get B|staticGetter() → self::B% /* erasure=self::A, declared=! */;
78+
static extension-type-member get B|staticGetter() → self::B% /* erasure=self::A, declared=! */
79+
return js_2::_getPropertyTrustType<dynamic>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGetter") as{TypeError,ForDynamic} self::B% /* erasure=self::A, declared=! */;
7880
external static extension-type-member set B|staticSetter(self::B% /* erasure=self::A, declared=! */ b) → void;
79-
external static extension-type-member get B|staticProperty() → self::B% /* erasure=self::A, declared=! */;
81+
static extension-type-member get B|staticProperty() → self::B% /* erasure=self::A, declared=! */
82+
return js_2::_getPropertyTrustType<dynamic>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty") as{TypeError,ForDynamic} self::B% /* erasure=self::A, declared=! */;
8083
external static extension-type-member set B|staticProperty(self::B% /* erasure=self::A, declared=! */ b) → void;
8184
static method method(self::A a) → void {
8285
self::B% /* erasure=self::A, declared=! */ b1 = js_2::_callConstructorUnchecked1<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), a);
@@ -88,13 +91,13 @@ static method method(self::A a) → void {
8891
b1 = js_2::getProperty<self::B% /* erasure=self::A, declared=! */>(b2, "getter");
8992
js_2::_setPropertyUnchecked<self::B% /* erasure=self::A, declared=! */>(b1, "setter", b2);
9093
js_2::_setPropertyUnchecked<self::B% /* erasure=self::A, declared=! */>(b1, "property", js_2::getProperty<self::B% /* erasure=self::A, declared=! */>(b2, "property"));
91-
a = js_2::getProperty<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField");
94+
a = self::B|staticField;
9295
js_2::_setPropertyUnchecked<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticField", a);
9396
a = js_2::_callMethodUnchecked0<self::A>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticMethod");
9497
b2 = js_2::_callMethodUnchecked1<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGenericMethod", b2);
95-
b1 = js_2::getProperty<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticGetter");
98+
b1 = self::B|staticGetter;
9699
js_2::_setPropertyUnchecked<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticSetter", b2);
97-
js_2::_setPropertyUnchecked<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty", js_2::getProperty<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty"));
100+
js_2::_setPropertyUnchecked<self::B% /* erasure=self::A, declared=! */>(js_2::_getPropertyTrustType<core::Object>(_js2::staticInteropGlobalContext, "B"), "staticProperty", self::B|staticProperty);
98101
}
99102

100103
constants {

0 commit comments

Comments
 (0)