Skip to content

Commit b372b54

Browse files
osa1Commit Queue
authored andcommitted
Reland "[dart2wasm] Don't dartify JS values when returning as void"
This is a reland of commit 4bb05dc Initial commit didn't properly box the `void` return value, so when we cast the return value to `ref #Top` to pass it to a Dart function (as `Object?` or `dynamic`) it caused "illegal cast" traps. In this commit we properly box the `externref` as `JSValue` (a proper Dart class). A new test added passing the return value of a `void` JS return to `print`. Original change's description: > [dart2wasm] Don't dartify JS values when returning as void > > When calling a JS function that returns `void`, avoid dartifying the > result. > > Technically the return value of `void` functions can still be used, by > casting the return type to `Object?` or `dynamic`. However this > shouldn't be done, and `dartifyRaw` overhead just to support this case > which should be extremely rare is too much. > > Any hacky code that uses return values of `void`-returning JS interop > functions can manually dartify the retrun values with `toDart` or > similar. > > Issue: #60357 > Change-Id: Ic370c7cf6eb6982f61f8a07c91e3bb93c5345ac6 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417240 > Reviewed-by: Srujan Gaddam <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> > Commit-Queue: Ömer Ağacan <[email protected]> Change-Id: I264211cca4f6b87e63d68909647975ab96f0b5bd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421080 Commit-Queue: Ömer Ağacan <[email protected]> Reviewed-by: Srujan Gaddam <[email protected]>
1 parent 77f0ed1 commit b372b54

File tree

2 files changed

+48
-28
lines changed

2 files changed

+48
-28
lines changed

pkg/dart2wasm/lib/js/util.dart

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -286,36 +286,39 @@ class CoreTypesUtil {
286286
Expression castInvocationForReturn(
287287
Expression invocation, DartType returnType) {
288288
if (returnType is VoidType) {
289-
// `undefined` may be returned for `void` external members. It, however,
290-
// is an extern ref, and therefore needs to be made a Dart type before
291-
// we can finish the invocation.
292-
return invokeOneArg(dartifyRawTarget, invocation);
293-
} else {
294-
Expression expression;
295-
if (isJSValueType(returnType)) {
296-
// TODO(joshualitt): Expose boxed `JSNull` and `JSUndefined` to Dart
297-
// code after migrating existing users of js interop on Dart2Wasm.
298-
// expression = _createJSValue(invocation);
299-
// Casts are expensive, so we stick to a null-assertion if needed. If
300-
// the nullability can't be determined, cast.
301-
expression = invokeOneArg(jsValueBoxTarget, invocation);
302-
final nullability = returnType.extensionTypeErasure.nullability;
303-
if (nullability == Nullability.nonNullable) {
304-
expression = NullCheck(expression);
305-
} else if (nullability == Nullability.undetermined) {
306-
expression = AsExpression(expression, returnType);
307-
}
308-
} else {
309-
// Because we simply don't have enough information, we leave all JS
310-
// numbers as doubles. However, in cases where we know the user expects
311-
// an `int` we check that the double is an integer, and then insert a
312-
// cast. We also let static interop types flow through without
313-
// conversion, both as arguments, and as the return type.
314-
expression = convertAndCast(
315-
returnType, invokeOneArg(dartifyRawTarget, invocation));
289+
// Technically a `void` return value can still be used, by casting the
290+
// return type to `dynamic` or `Object?`. However this case should be
291+
// extremely rare, and `dartifyRaw` overhead for return values that will
292+
// never be used in practice is too much, so we avoid `dartifyRaw` on
293+
// `void` returns. We still box the `externref` as the value can be passed
294+
// around as a Dart object.
295+
return StaticInvocation(jsValueBoxTarget, Arguments([invocation]));
296+
}
297+
298+
Expression expression;
299+
if (isJSValueType(returnType)) {
300+
// TODO(joshualitt): Expose boxed `JSNull` and `JSUndefined` to Dart
301+
// code after migrating existing users of js interop on Dart2Wasm.
302+
// expression = _createJSValue(invocation);
303+
// Casts are expensive, so we stick to a null-assertion if needed. If
304+
// the nullability can't be determined, cast.
305+
expression = invokeOneArg(jsValueBoxTarget, invocation);
306+
final nullability = returnType.extensionTypeErasure.nullability;
307+
if (nullability == Nullability.nonNullable) {
308+
expression = NullCheck(expression);
309+
} else if (nullability == Nullability.undetermined) {
310+
expression = AsExpression(expression, returnType);
316311
}
317-
return expression;
312+
} else {
313+
// Because we simply don't have enough information, we leave all JS
314+
// numbers as doubles. However, in cases where we know the user expects
315+
// an `int` we check that the double is an integer, and then insert a
316+
// cast. We also let static interop types flow through without
317+
// conversion, both as arguments, and as the return type.
318+
expression = convertAndCast(
319+
returnType, invokeOneArg(dartifyRawTarget, invocation));
318320
}
321+
return expression;
319322
}
320323

321324
// Handles any necessary type conversions. Today this is just for handling the
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) 2025, 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+
// Check that `void` return values can be passed around as Dart objects.
6+
7+
import 'dart:js_interop';
8+
9+
@JS()
10+
external void eval(String code);
11+
12+
void main() {
13+
Object? x = eval('1 + 1') as dynamic;
14+
15+
// It doesn't matter what this prints, it just shouldn't crash.
16+
print(x);
17+
}

0 commit comments

Comments
 (0)