Reland "[dart2wasm] Don't dartify JS values when returning as void"
This is a reland of commit 4bb05dc562
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: https://github.com/dart-lang/sdk/issues/60357
> Change-Id: Ic370c7cf6eb6982f61f8a07c91e3bb93c5345ac6
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417240
> Reviewed-by: Srujan Gaddam <srujzs@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Commit-Queue: Ömer Ağacan <omersa@google.com>
Change-Id: I264211cca4f6b87e63d68909647975ab96f0b5bd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421080
Commit-Queue: Ömer Ağacan <omersa@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
This commit is contained in:
committed by
Commit Queue
parent
77f0ed1c7a
commit
b372b54ecb
@@ -286,36 +286,39 @@ class CoreTypesUtil {
|
||||
Expression castInvocationForReturn(
|
||||
Expression invocation, DartType returnType) {
|
||||
if (returnType is VoidType) {
|
||||
// `undefined` may be returned for `void` external members. It, however,
|
||||
// is an extern ref, and therefore needs to be made a Dart type before
|
||||
// we can finish the invocation.
|
||||
return invokeOneArg(dartifyRawTarget, invocation);
|
||||
} else {
|
||||
Expression expression;
|
||||
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);
|
||||
// Casts are expensive, so we stick to a null-assertion if needed. If
|
||||
// the nullability can't be determined, cast.
|
||||
expression = invokeOneArg(jsValueBoxTarget, invocation);
|
||||
final nullability = returnType.extensionTypeErasure.nullability;
|
||||
if (nullability == Nullability.nonNullable) {
|
||||
expression = NullCheck(expression);
|
||||
} else if (nullability == Nullability.undetermined) {
|
||||
expression = AsExpression(expression, returnType);
|
||||
}
|
||||
} else {
|
||||
// Because we simply don't have enough information, we leave all JS
|
||||
// numbers as doubles. However, in cases where we know the user expects
|
||||
// an `int` we check that the double is an integer, and then insert a
|
||||
// cast. We also let static interop types flow through without
|
||||
// conversion, both as arguments, and as the return type.
|
||||
expression = convertAndCast(
|
||||
returnType, invokeOneArg(dartifyRawTarget, invocation));
|
||||
}
|
||||
return expression;
|
||||
// Technically a `void` return value can still be used, by casting the
|
||||
// return type to `dynamic` or `Object?`. However this case should be
|
||||
// extremely rare, and `dartifyRaw` overhead for return values that will
|
||||
// never be used in practice is too much, so we avoid `dartifyRaw` on
|
||||
// `void` returns. We still box the `externref` as the value can be passed
|
||||
// around as a Dart object.
|
||||
return StaticInvocation(jsValueBoxTarget, Arguments([invocation]));
|
||||
}
|
||||
|
||||
Expression expression;
|
||||
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);
|
||||
// Casts are expensive, so we stick to a null-assertion if needed. If
|
||||
// the nullability can't be determined, cast.
|
||||
expression = invokeOneArg(jsValueBoxTarget, invocation);
|
||||
final nullability = returnType.extensionTypeErasure.nullability;
|
||||
if (nullability == Nullability.nonNullable) {
|
||||
expression = NullCheck(expression);
|
||||
} else if (nullability == Nullability.undetermined) {
|
||||
expression = AsExpression(expression, returnType);
|
||||
}
|
||||
} else {
|
||||
// Because we simply don't have enough information, we leave all JS
|
||||
// numbers as doubles. However, in cases where we know the user expects
|
||||
// an `int` we check that the double is an integer, and then insert a
|
||||
// cast. We also let static interop types flow through without
|
||||
// conversion, both as arguments, and as the return type.
|
||||
expression = convertAndCast(
|
||||
returnType, invokeOneArg(dartifyRawTarget, invocation));
|
||||
}
|
||||
return expression;
|
||||
}
|
||||
|
||||
// Handles any necessary type conversions. Today this is just for handling the
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
// Copyright (c) 2025, 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.
|
||||
|
||||
// Check that `void` return values can be passed around as Dart objects.
|
||||
|
||||
import 'dart:js_interop';
|
||||
|
||||
@JS()
|
||||
external void eval(String code);
|
||||
|
||||
void main() {
|
||||
Object? x = eval('1 + 1') as dynamic;
|
||||
|
||||
// It doesn't matter what this prints, it just shouldn't crash.
|
||||
print(x);
|
||||
}
|
||||
Reference in New Issue
Block a user