Skip to content

Commit

Permalink
[dart:js_interop] Add ExternalDartReference
Browse files Browse the repository at this point in the history
Closes #55187

Adds a faster way for users to pass opaque Dart values to
JS without the need for boxing like in JSBoxedDartObject.
This does mean, however, that this new type can't be a JS type,
and therefore cannot have interop members declared on it.
Refactors existing code to handle that distinction.

CoreLibraryReviewExempt: Backend-specific library that's been reviewed by both dart2wasm and JS compiler teams.
Change-Id: Ia86f1fe3476512fc0e5f382e05739713b687f092
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358224
Reviewed-by: Sigmund Cherem <sigmund@google.com>
  • Loading branch information
srujzs authored and Commit Queue committed Mar 26, 2024
1 parent bcf64bd commit 1232260
Show file tree
Hide file tree
Showing 18 changed files with 275 additions and 45 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Expand Up @@ -46,10 +46,15 @@ advantage of these improvements, set your package's
- Fixes an issue with several comparison operators in `JSAnyOperatorExtension`
that were declared to return `JSBoolean` but really returned `bool`. This led
to runtime errors when trying to use the return values. The implementation now
returns a `JSBoolean` to align with the interface. See issues [#55024] for
returns a `JSBoolean` to align with the interface. See issue [#55024] for
more details.
- Added `ExternalDartReference` and related conversion functions
`toExternalReference` and `toDartObject`. This is a faster alternative to
`JSBoxedDartObject`, but with fewer safety guarantees and fewer
interoperability capabilities. See [#55187] for more details.

[#55024]: https://github.com/dart-lang/sdk/issues/55024
[#55187]: https://github.com/dart-lang/sdk/issues/55187

### Tools

Expand Down
8 changes: 4 additions & 4 deletions pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
Expand Up @@ -10698,7 +10698,7 @@ const Template<Message Function(String string2)>
problemMessageTemplate:
r"""External JS interop member contains invalid types in its function signature: '#string2'.""",
correctionMessageTemplate:
r"""Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
r"""Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
withArguments:
_withArgumentsJsInteropStaticInteropExternalFunctionTypeViolation,
);
Expand All @@ -10719,7 +10719,7 @@ Message _withArgumentsJsInteropStaticInteropExternalFunctionTypeViolation(
problemMessage:
"""External JS interop member contains invalid types in its function signature: '${string2}'.""",
correctionMessage:
"""Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
"""Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
arguments: {
'string2': string2,
},
Expand Down Expand Up @@ -10938,7 +10938,7 @@ const Template<Message Function(String string2)>
problemMessageTemplate:
r"""Function converted via 'toJS' contains invalid types in its function signature: '#string2'.""",
correctionMessageTemplate:
r"""Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
r"""Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
withArguments: _withArgumentsJsInteropStaticInteropToJSFunctionTypeViolation,
);

Expand All @@ -10958,7 +10958,7 @@ Message _withArgumentsJsInteropStaticInteropToJSFunctionTypeViolation(
problemMessage:
"""Function converted via 'toJS' contains invalid types in its function signature: '${string2}'.""",
correctionMessage:
"""Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
"""Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
arguments: {
'string2': string2,
},
Expand Down
7 changes: 5 additions & 2 deletions pkg/_js_interop_checks/lib/js_interop_checks.dart
Expand Up @@ -927,6 +927,9 @@ class JsInteropChecks extends RecursiveVisitor {
// If it can be used as a representation type of an interop extension type,
// it is okay to be used on an external member.
if (extensionIndex.isAllowedRepresentationType(type)) return true;
// ExternalDartReference is allowed on interop members even though it's not
// an interop type.
if (extensionIndex.isExternalDartReferenceType(type)) return true;
if (type is InterfaceType) {
final cls = type.classNode;
// Primitive types are okay.
Expand All @@ -939,8 +942,8 @@ class JsInteropChecks extends RecursiveVisitor {
}
} else if (type is ExtensionType) {
// Extension types that wrap other allowed types are also okay. Interop
// extension types are handled above, so this is essentially for extension
// types on primitives.
// extension types and ExternalDartReference are handled above, so this is
// essentially for extension types on primitives.
return _isAllowedExternalType(type.extensionTypeErasure);
}
return false;
Expand Down
Expand Up @@ -697,6 +697,7 @@ class ExtensionIndex {
final Map<Reference, ExtensionTypeMemberDescriptor>
_extensionTypeMemberIndex = {};
final Map<Reference, Reference> _extensionTypeTearOffIndex = {};
final Set<Reference> _externalDartReferences = {};
final Class? _javaScriptObject;
final Set<Library> _processedExtensionLibraries = {};
final Set<Library> _processedExtensionTypeLibraries = {};
Expand Down Expand Up @@ -1013,5 +1014,23 @@ class ExtensionIndex {
}

bool isJSType(ExtensionTypeDeclaration decl) =>
decl.enclosingLibrary.importUri.toString() == 'dart:js_interop';
decl.enclosingLibrary.importUri.toString() == 'dart:js_interop' &&
decl.name.startsWith('JS');

bool isExternalDartReference(ExtensionTypeDeclaration decl) =>
decl.enclosingLibrary.importUri.toString() == 'dart:js_interop' &&
decl.name == 'ExternalDartReference';

bool isExternalDartReferenceType(DartType type) {
if (type is ExtensionType) {
final decl = type.extensionTypeDeclaration;
if (_externalDartReferences.contains(decl.reference)) return true;
if (isExternalDartReference(decl) ||
isExternalDartReferenceType(decl.declaredRepresentationType)) {
_externalDartReferences.add(decl.reference);
return true;
}
}
return false;
}
}
11 changes: 4 additions & 7 deletions pkg/dart2wasm/lib/js/callback_specializer.dart
@@ -1,8 +1,7 @@
// 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.
import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'
show ExtensionIndex;

import 'package:kernel/ast.dart';
import 'package:kernel/type_environment.dart';

Expand All @@ -14,10 +13,9 @@ class CallbackSpecializer {
final StatefulStaticTypeContext _staticTypeContext;
final MethodCollector _methodCollector;
final CoreTypesUtil _util;
final ExtensionIndex _extensionIndex;

CallbackSpecializer(this._staticTypeContext, this._util,
this._methodCollector, this._extensionIndex);
CallbackSpecializer(
this._staticTypeContext, this._util, this._methodCollector);

bool _needsArgumentsLength(FunctionType type) =>
type.requiredParameterCount < type.positionalParameters.length;
Expand All @@ -33,8 +31,7 @@ class CallbackSpecializer {
DartType callbackParameterType = function.positionalParameters[i];
Expression expression;
VariableGet v = VariableGet(positionalParameters[i]);
if (_extensionIndex.isStaticInteropType(callbackParameterType) &&
boxExternRef) {
if (_util.isJSValueType(callbackParameterType) && boxExternRef) {
expression = _createJSValue(v);
if (!callbackParameterType.isPotentiallyNullable) {
expression = NullCheck(expression);
Expand Down
4 changes: 2 additions & 2 deletions pkg/dart2wasm/lib/js/interop_transformer.dart
Expand Up @@ -36,8 +36,8 @@ class InteropTransformer extends Transformer {

InteropTransformer._(this._staticTypeContext, this._util,
this._methodCollector, extensionIndex)
: _callbackSpecializer = CallbackSpecializer(
_staticTypeContext, _util, _methodCollector, extensionIndex),
: _callbackSpecializer =
CallbackSpecializer(_staticTypeContext, _util, _methodCollector),
_inlineExpander =
InlineExpander(_staticTypeContext, _util, _methodCollector),
_interopSpecializerFactory = InteropSpecializerFactory(
Expand Down
11 changes: 7 additions & 4 deletions pkg/dart2wasm/lib/js/util.dart
Expand Up @@ -68,9 +68,12 @@ class CoreTypesUtil {
wasmExternRefClass.getThisType(coreTypes, Nullability.nullable);

Procedure jsifyTarget(DartType type) =>
_extensionIndex.isStaticInteropType(type)
? jsValueUnboxTarget
: jsifyRawTarget;
isJSValueType(type) ? jsValueUnboxTarget : jsifyRawTarget;

/// Return whether [type] erases to a `JSValue`.
bool isJSValueType(DartType type) =>
_extensionIndex.isStaticInteropType(type) ||
_extensionIndex.isExternalDartReferenceType(type);

void annotateProcedure(
Procedure procedure, String pragmaOptionString, AnnotationType type) {
Expand Down Expand Up @@ -101,7 +104,7 @@ class CoreTypesUtil {
return invokeOneArg(dartifyRawTarget, invocation);
} else {
Expression expression;
if (_extensionIndex.isStaticInteropType(returnType)) {
if (isJSValueType(returnType)) {
// TODO(joshualitt): Expose boxed `JSNull` and `JSUndefined` to Dart
// code after migrating existing users of js interop on Dart2Wasm.
// expression = _createJSValue(invocation);
Expand Down
Expand Up @@ -4723,7 +4723,7 @@ const Template<Message Function(DartType _type, bool isNonNullableByDefault)>
problemMessageTemplate:
r"""External JS interop member contains an invalid type: '#type'.""",
correctionMessageTemplate:
r"""Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
r"""Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
withArguments:
_withArgumentsJsInteropStaticInteropExternalAccessorTypeViolation,
);
Expand All @@ -4747,7 +4747,7 @@ Message _withArgumentsJsInteropStaticInteropExternalAccessorTypeViolation(
"""External JS interop member contains an invalid type: '${type}'.""" +
labeler.originMessages,
correctionMessage:
"""Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
"""Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type.""",
arguments: {
'type': _type,
},
Expand Down
6 changes: 3 additions & 3 deletions pkg/front_end/messages.yaml
Expand Up @@ -5932,15 +5932,15 @@ JsInteropNonStaticWithStaticInteropSupertype:
# to avoid duplication?
JsInteropStaticInteropExternalAccessorTypeViolation:
problemMessage: "External JS interop member contains an invalid type: '#type'."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type."

JsInteropStaticInteropExternalFunctionTypeViolation:
problemMessage: "External JS interop member contains invalid types in its function signature: '#string2'."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type."

JsInteropStaticInteropToJSFunctionTypeViolation:
problemMessage: "Function converted via 'toJS' contains invalid types in its function signature: '#string2'."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', ExternalDartReference, void, bool, num, double, int, String, extension types that erase to one of these types, '@staticInterop' types, 'dart:html' types when compiling to JS, or a type parameter that is a subtype of a valid non-primitive type."

JsInteropStaticInteropGenerativeConstructor:
problemMessage: "`@staticInterop` classes should not contain any generative constructors."
Expand Down
3 changes: 2 additions & 1 deletion pkg/front_end/test/spell_checking_list_messages.txt
Expand Up @@ -54,6 +54,7 @@ erase
exhaustively
exportable
extensiontype
externaldartreference
f
ffi
finality
Expand Down Expand Up @@ -84,8 +85,8 @@ loadlibrary
macro
member(s)
migrate
modifier
mocking
modifier
n
name.#name
name.stack
Expand Down
16 changes: 16 additions & 0 deletions sdk/lib/_internal/js_shared/lib/js_interop_patch.dart
Expand Up @@ -118,6 +118,22 @@ extension ObjectToJSBoxedDartObject on Object {
}
}

/// [ExternalDartReference] <-> [Object]
@patch
extension ExternalDartReferenceToObject on ExternalDartReference {
@patch
@pragma('dart2js:prefer-inline')
Object get toDartObject => this;
}

@patch
extension ObjectToExternalDartReference on Object {
@patch
@pragma('dart2js:prefer-inline')
ExternalDartReference get toExternalReference =>
this as ExternalDartReference;
}

/// [JSPromise] -> [Future].
@patch
extension JSPromiseToFuture<T extends JSAny?> on JSPromise<T> {
Expand Down
8 changes: 5 additions & 3 deletions sdk/lib/_internal/js_shared/lib/js_types.dart
Expand Up @@ -64,7 +64,9 @@ typedef JSSymbolRepType = interceptors.JavaScriptSymbol;

typedef JSBigIntRepType = interceptors.JavaScriptBigInt;

/// [JSVoid] is just a typedef for [void]. While we could just use
/// `JSUndefined`, in the future we may be able to use this to elide `return`s
/// in JS trampolines.
// While this type is not a JS type, it is here for convenience so we don't need
// to create a new shared library.
typedef ExternalDartReferenceRepType = Object;

// JSVoid is just a typedef for void.
typedef JSVoidRepType = void;
15 changes: 15 additions & 0 deletions sdk/lib/_internal/wasm/lib/js_interop_patch.dart
Expand Up @@ -136,6 +136,21 @@ extension ObjectToJSBoxedDartObject on Object {
}
}

/// [ExternalDartReference] <-> [Object]
@patch
extension ExternalDartReferenceToObject on ExternalDartReference {
@patch
Object get toDartObject =>
jsObjectToDartObject((this as JSValue).toExternRef);
}

@patch
extension ObjectToExternalDartReference on Object {
@patch
ExternalDartReference get toExternalReference =>
_boxNonNullable<ExternalDartReference>(jsObjectFromDartObject(this));
}

/// [JSPromise] -> [Future].
@patch
extension JSPromiseToFuture<T extends JSAny?> on JSPromise<T> {
Expand Down
9 changes: 6 additions & 3 deletions sdk/lib/_internal/wasm/lib/js_types.dart
Expand Up @@ -75,7 +75,10 @@ typedef JSSymbolRepType = js.JSValue;

typedef JSBigIntRepType = js.JSValue;

/// [JSVoid] is just a typedef for [void]. While we could just use
/// `JSUndefined`, in the future we may be able to use this to elide `return`s
/// in JS trampolines.
// While this type is not a JS type, it is here for convenience so we don't need
// to create a new shared library.
typedef ExternalDartReferenceRepType = js.JSValue;

// JSVoid is just a typedef for void. While we could just use JSUndefined, in
// the future we may be able to use this to elide `return`s in JS trampolines.
typedef JSVoidRepType = void;

0 comments on commit 1232260

Please sign in to comment.