Skip to content

Commit

Permalink
[dart2wasm/ddc/dart2js] Lower Function.toJS and JSExportedDartFunctio…
Browse files Browse the repository at this point in the history 
…n.toDart

Lowers these extension methods to some helper functions instead of
allowInterop to improve performance and get consistent semantics.

Specifically:
- Function.toJS no longer gives you the same JSFunction when calling
toJS on the same function multiple times in the JS compilers.
- Adds fast calling syntax for functions with 0-5 args in the JS
compilers.
- Allows additional args to be passed to converted functions that
are omitted in the JS compilers.
- The static type now determines the number of args that can be
passed to the JS function in the JS compilers.
- Fixes an issue in dart2wasm where if too few arguments are
passed, the call may succeed due to conversion of undefined to
null.
- Adds throws when a user tries to wrap a JS function that
returned from Function.toJS in the JS compilers.

Closes #55515
Addresses #48186

CoreLibraryReviewExempt: Changes to docs only in API surface.
Change-Id: I41d864dc5e02b597d9f1c16c88e3c04872f28225
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368065
Reviewed-by: Nicholas Shahan <[email protected]>
Reviewed-by: Stephen Adams <[email protected]>
Commit-Queue: Srujan Gaddam <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
  • Loading branch information
@srujzs
srujzs authored and Commit Queue committed Jun 26, 2024
1 parent 4b88698 commit 1c534e5
Show file tree
Hide file tree
Showing 15 changed files with 1,114 additions and 109 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,19 @@
`ExternalDartReferenceToObject` and `ObjectToExternalDartReference` are now
extensions on `T` and `ExternalDartReference<T>`, respectively, where `T
extends Object?`. See [#55342][] and [#55536][] for more details.
- Fixes some consistency issues with `Function.toJS` across all compilers.
Specifically, calling `Function.toJS` on the same function gives you a new JS
function (see issue [#55515][]), the max number of args that are passed to the
JS function is determined by the static type of the Dart function, and extra
args are dropped when passed to the JS function in all compilers (see
[#48186][]).

[#55508]: https://github.com/dart-lang/sdk/issues/55508
[#55267]: https://github.com/dart-lang/sdk/issues/55267
[#55342]: https://github.com/dart-lang/sdk/issues/55342
[#55536]: https://github.com/dart-lang/sdk/issues/55536
[#55515]: https://github.com/dart-lang/sdk/issues/55515
[#48186]: https://github.com/dart-lang/sdk/issues/48186

### Tools

Expand Down
61 changes: 61 additions & 0 deletions pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,14 @@ class JsUtilOptimizer extends Transformer {
final List<Procedure> _callConstructorUncheckedTargets;
final CloneVisitorNotMembers _cloner = CloneVisitorWithMembers();
final Map<Member, _InvocationBuilder?> _externalInvocationBuilders = {};
final Procedure _functionToJSTarget;
final List<Procedure> _functionToJSTargets;
final Procedure _functionToJSNTarget;
final Procedure _getPropertyTarget;
final Procedure _getPropertyTrustTypeTarget;
final Procedure _globalContextTarget;
final Procedure _jsExportedDartFunctionToDartTarget;
final Procedure _jsFunctionToDart;
final InterfaceType _objectType;
final Procedure _setPropertyTarget;
final Procedure _setPropertyUncheckedTarget;
Expand Down Expand Up @@ -88,12 +93,25 @@ class JsUtilOptimizer extends Transformer {
5,
(i) => _coreTypes.index.getTopLevelProcedure(
'dart:js_util', '_callConstructorUnchecked$i')),
_functionToJSTarget = _coreTypes.index.getTopLevelProcedure(
'dart:js_interop', 'FunctionToJSExportedDartFunction|get#toJS'),
_functionToJSTargets = List<Procedure>.generate(
6,
(i) => _coreTypes.index
.getTopLevelProcedure('dart:js_util', '_functionToJS$i')),
_functionToJSNTarget = _coreTypes.index
.getTopLevelProcedure('dart:js_util', '_functionToJSN'),
_getPropertyTarget = _coreTypes.index
.getTopLevelProcedure('dart:js_util', 'getProperty'),
_getPropertyTrustTypeTarget = _coreTypes.index
.getTopLevelProcedure('dart:js_util', '_getPropertyTrustType'),
_globalContextTarget = _coreTypes.index.getTopLevelProcedure(
'dart:_js_helper', 'get:staticInteropGlobalContext'),
_jsExportedDartFunctionToDartTarget = _coreTypes.index
.getTopLevelProcedure('dart:js_interop',
'JSExportedDartFunctionToFunction|get#toDart'),
_jsFunctionToDart = _coreTypes.index
.getTopLevelProcedure('dart:js_util', '_jsFunctionToDart'),
_objectType = hierarchy.coreTypes.objectNonNullableRawType,
_setPropertyTarget = _coreTypes.index
.getTopLevelProcedure('dart:js_util', 'setProperty'),
Expand Down Expand Up @@ -480,6 +498,10 @@ class JsUtilOptimizer extends Transformer {
invocation = _lowerCallConstructor(node);
// TODO(srujzs): Delete the `isPatchedMember` check once
// https://github.com/dart-lang/sdk/issues/53367 is resolved.
} else if (target == _functionToJSTarget) {
invocation = _lowerFunctionToJS(node);
} else if (target == _jsExportedDartFunctionToDartTarget) {
invocation = _lowerJSExportedDartFunctionToDart(node);
} else if (target.isExternal && !JsInteropChecks.isPatchedMember(target)) {
final builder = _externalInvocationBuilders.putIfAbsent(
target, () => _getExternalInvocationBuilder(target));
Expand Down Expand Up @@ -662,6 +684,45 @@ class JsUtilOptimizer extends Transformer {
..parent = nodeParent;
}

/// For the given `dart:js_interop` `Function.toJS` invocation [node], returns
/// an invocation of `_functionToJSX` with the given `Function` argument,
/// where X is the number of the positional arguments.
///
/// If the number of the positional arguments is larger than 5, returns an
/// invocation of `_functionToJSN` instead.
StaticInvocation _lowerFunctionToJS(StaticInvocation node) {
// JS interop checks assert that the static type is available, and that
// there are no named arguments or type arguments.
final function = node.arguments.positional.single;
final functionType =
function.getStaticType(_staticTypeContext) as FunctionType;
final argumentsLength = functionType.positionalParameters.length;
Procedure target;
Arguments arguments;
if (argumentsLength < _functionToJSTargets.length) {
target = _functionToJSTargets[argumentsLength];
arguments = Arguments([function]);
} else {
target = _functionToJSNTarget;
arguments = Arguments([function, IntLiteral(argumentsLength)]);
}
return StaticInvocation(
target, arguments..fileOffset = node.arguments.fileOffset)
..fileOffset = node.fileOffset
..parent = node.parent;
}

/// For the given `dart:js_interop` `JSExportedDartFunction.toDart` invocation
/// [node], returns an invocation of `_jsFunctionToDart` with the given
/// `JSExportedDartFunction` argument.
StaticInvocation _lowerJSExportedDartFunctionToDart(StaticInvocation node) =>
StaticInvocation(
_jsFunctionToDart,
Arguments([node.arguments.positional[0]])
..fileOffset = node.arguments.fileOffset)
..fileOffset = node.fileOffset
..parent = node.parent;

/// Returns whether the given [node] is guaranteed to be allowed to interop
/// with JS.
///
Expand Down
104 changes: 51 additions & 53 deletions pkg/dart2wasm/lib/js/callback_specializer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ class CallbackSpecializer {
CallbackSpecializer(
this._staticTypeContext, this._util, this._methodCollector);

bool _needsArgumentsLength(FunctionType type) =>
type.requiredParameterCount < type.positionalParameters.length;

Statement _generateDispatchCase(
FunctionType function,
VariableDeclaration callbackVariable,
Expand Down Expand Up @@ -54,12 +51,11 @@ class CallbackSpecializer {
/// Creates a callback trampoline for the given [function].
///
/// This callback trampoline expects a Dart callback as its first argument,
/// then an integer value(double type) indicating the position of the last
/// defined argument(only for callbacks that take optional parameters),
/// followed by all of the arguments to the Dart callback as JS objects. The
/// trampoline will `dartifyRaw` or box all incoming JS objects and then cast
/// them to their appropriate types, dispatch, and then `jsifyRaw` or box any
/// returned value.
/// then an integer value(double type) indicating the number of arguments
/// passed, followed by all of the arguments to the Dart callback as JS
/// objects. The trampoline will `dartifyRaw` or box all incoming JS objects
/// and then cast them to their appropriate types, dispatch, and then
/// `jsifyRaw` or box any returned value.
///
/// Returns a [String] function name representing the name of the wrapping
/// function.
Expand All @@ -69,20 +65,18 @@ class CallbackSpecializer {
// arguments will be JS objects. The generated wrapper will cast each
// argument to the correct type. The first argument to this function will
// be the Dart callback, which will be cast to the supplied [FunctionType]
// before being invoked. If the callback takes optional parameters then, the
// second argument will be a `double` indicating the last defined argument.
// before being invoked. The second argument will be a `double` indicating
// the number of arguments passed.
int parameterId = 1;
final callbackVariable = VariableDeclaration('callback',
type: _util.nonNullableObjectType, isSynthesized: true);
VariableDeclaration? argumentsLength;
if (_needsArgumentsLength(function)) {
argumentsLength = VariableDeclaration('argumentsLength',
type: _util.coreTypes.doubleNonNullableRawType, isSynthesized: true);
}
final argumentsLength = VariableDeclaration('argumentsLength',
type: _util.coreTypes.doubleNonNullableRawType, isSynthesized: true);

// Initialize variable declarations.
List<VariableDeclaration> positionalParameters = [];
for (int j = 0; j < function.positionalParameters.length; j++) {
final positionalParametersLength = function.positionalParameters.length;
for (int j = 0; j < positionalParametersLength; j++) {
positionalParameters.add(VariableDeclaration('x${parameterId++}',
type: _util.nullableWasmExternRefType, isSynthesized: true));
}
Expand All @@ -91,26 +85,42 @@ class CallbackSpecializer {
// find the last defined argument in JS, that is the last argument which was
// explicitly passed by the user, and then we dispatch to a Dart function
// with the right number of arguments.
//
// First we handle cases where some or all arguments are undefined.
// TODO(joshualitt): Consider using a switch instead.
List<Statement> dispatchCases = [];
for (int i = function.requiredParameterCount + 1;
i <= function.positionalParameters.length;
i++) {
// If more arguments were passed than there are parameters, ignore the extra
// arguments.
dispatchCases.add(IfStatement(
_util.variableGreaterThanOrEqualToConstant(
argumentsLength, IntConstant(positionalParametersLength)),
_generateDispatchCase(function, callbackVariable, positionalParameters,
positionalParametersLength,
boxExternRef: boxExternRef),
null));
// TODO(srujzs): Consider using a switch instead.
for (int i = positionalParametersLength - 1;
i >= function.requiredParameterCount;
i--) {
dispatchCases.add(IfStatement(
_util.variableCheckConstant(
argumentsLength!, DoubleConstant(i.toDouble())),
argumentsLength, DoubleConstant(i.toDouble())),
_generateDispatchCase(
function, callbackVariable, positionalParameters, i,
boxExternRef: boxExternRef),
null));
}

// Finally handle the case where only required parameters are passed.
dispatchCases.add(_generateDispatchCase(function, callbackVariable,
positionalParameters, function.requiredParameterCount,
boxExternRef: boxExternRef));
// Throw since we have too few arguments. Alternatively, we can continue
// checking lengths and try to call the callback, which will then throw, but
// that's unnecessary extra code. Note that we can't exclude this and assume
// the last dispatch case will catch this. Since arguments that are not
// passed are `undefined` and `undefined` gets converted to `null`, they may
// be treated as valid `null` arguments to the Dart function even though
// they were never passed.
dispatchCases.add(ExpressionStatement(Throw(StringConcatenation([
StringLiteral('Too few arguments passed. '
'Expected ${function.requiredParameterCount} or more, got '),
invokeMethod(argumentsLength, _util.numToIntTarget),
StringLiteral(' instead.')
]))));
Statement functionTrampolineBody = Block(dispatchCases);

// Create a new procedure for the callback trampoline. This procedure will
Expand All @@ -124,8 +134,9 @@ class CallbackSpecializer {
FunctionNode(functionTrampolineBody,
positionalParameters: [
callbackVariable,
if (argumentsLength != null) argumentsLength
].followedBy(positionalParameters).toList(),
argumentsLength,
...positionalParameters
],
returnType: _util.nullableWasmExternRefType)
..fileOffset = node.fileOffset,
node.fileUri,
Expand All @@ -144,11 +155,7 @@ class CallbackSpecializer {
jsParameters.add('x$i');
}
String jsParametersString = jsParameters.join(',');
String dartArguments = 'f';
bool needsArguments = _needsArgumentsLength(type);
if (needsArguments) {
dartArguments = '$dartArguments,arguments.length';
}
String dartArguments = 'f,arguments.length';
if (jsParameters.isNotEmpty) {
dartArguments = '$dartArguments,$jsParametersString';
}
Expand All @@ -171,24 +178,12 @@ class CallbackSpecializer {
// Create JS method.
// Note: We have to use a regular function for the inner closure in some
// cases because we need access to `arguments`.
if (needsArguments) {
_methodCollector.addMethod(
dartProcedure,
jsMethodName,
"f => finalizeWrapper(f, function($jsParametersString) {"
" return dartInstance.exports.$functionTrampolineName($dartArguments) "
"})");
} else {
if (parametersNeedParens(jsParameters)) {
jsParametersString = '($jsParametersString)';
}
_methodCollector.addMethod(
dartProcedure,
jsMethodName,
"f => "
"finalizeWrapper(f,$jsParametersString => "
"dartInstance.exports.$functionTrampolineName($dartArguments))");
}
_methodCollector.addMethod(
dartProcedure,
jsMethodName,
"f => finalizeWrapper(f, function($jsParametersString) {"
" return dartInstance.exports.$functionTrampolineName($dartArguments) "
"})");

return dartProcedure;
}
Expand All @@ -207,6 +202,9 @@ class CallbackSpecializer {
/// these Dart functions flow to JS, they are replaced by their wrappers. If
/// the wrapper should ever flow back into Dart then it will be replaced by
/// the original Dart function.
// TODO(srujzs): It looks like there's no more code that references this
// function anymore in dart2wasm. Should we delete this lowering and related
// code?
Expression allowInterop(StaticInvocation staticInvocation) {
final argument = staticInvocation.arguments.positional.single;
final type = argument.getStaticType(_staticTypeContext) as FunctionType;
Expand Down
14 changes: 14 additions & 0 deletions pkg/dart2wasm/lib/js/util.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class CoreTypesUtil {
final Procedure allowInteropTarget;
final Procedure dartifyRawTarget;
final Procedure functionToJSTarget;
final Procedure greaterThanOrEqualToTarget;
final Procedure inlineJSTarget;
final Procedure isDartFunctionWrappedTarget;
final Procedure jsifyRawTarget;
Expand All @@ -33,6 +34,8 @@ class CoreTypesUtil {
.getTopLevelProcedure('dart:_js_helper', 'dartifyRaw'),
functionToJSTarget = coreTypes.index.getTopLevelProcedure(
'dart:js_interop', 'FunctionToJSExportedDartFunction|get#toJS'),
greaterThanOrEqualToTarget =
coreTypes.index.getProcedure('dart:core', 'num', '>='),
inlineJSTarget =
coreTypes.index.getTopLevelProcedure('dart:_js_helper', 'JS'),
isDartFunctionWrappedTarget = coreTypes.index
Expand Down Expand Up @@ -94,6 +97,17 @@ class CoreTypesUtil {
StaticInvocation(coreTypes.identicalProcedure,
Arguments([VariableGet(variable), ConstantExpression(constant)]));

Expression variableGreaterThanOrEqualToConstant(
VariableDeclaration variable, Constant constant) =>
InstanceInvocation(
InstanceAccessKind.Instance,
VariableGet(variable),
greaterThanOrEqualToTarget.name,
Arguments([ConstantExpression(constant)]),
interfaceTarget: greaterThanOrEqualToTarget,
functionType: greaterThanOrEqualToTarget.getterType as FunctionType,
);

/// Cast the [invocation] if needed to conform to the expected [returnType].
Expression castInvocationForReturn(
Expression invocation, DartType returnType) {
Expand Down
Loading

0 comments on commit 1c534e5

Please sign in to comment.