Skip to content

Commit 1dccb82

Browse files
derekxu16Commit Queue
authored and
Commit Queue
committed
Revert "[dart2wasm] Don't dartify JS values when returning as void"
This reverts commit 4bb05dc. Reason for revert: suspected to be blocking rolls into Flutter, e.g. flutter/flutter#166618 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]> Issue: #60357 Change-Id: I27e0a7355eeee94d1dc02fba0c9569b53954d1c5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420780 Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Ömer Ağacan <[email protected]> Reviewed-by: Srujan Gaddam <[email protected]> Commit-Queue: Derek Xu <[email protected]>
1 parent 17558d5 commit 1dccb82

File tree

1 file changed

+28
-33
lines changed

1 file changed

+28
-33
lines changed

pkg/dart2wasm/lib/js/util.dart

+28-33
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ class CoreTypesUtil {
6565
final Procedure jsifyJSArrayBufferImpl; // JS ByteBuffer
6666
final Procedure jsArrayBufferFromDartByteBuffer; // Wasm ByteBuffer
6767
final Procedure jsifyFunction;
68-
final Procedure internalizeNullable;
6968

7069
// Classes used in type tests for the converters.
7170
final Class jsInt8ArrayImplClass;
@@ -192,8 +191,6 @@ class CoreTypesUtil {
192191
'dart:_js_helper', 'jsArrayBufferFromDartByteBuffer'),
193192
jsifyFunction = coreTypes.index
194193
.getTopLevelProcedure('dart:_js_helper', 'jsifyFunction'),
195-
internalizeNullable = coreTypes.index
196-
.getTopLevelProcedure('dart:_wasm', '_internalizeNullable'),
197194
jsInt8ArrayImplClass =
198195
coreTypes.index.getClass('dart:_js_types', 'JSInt8ArrayImpl'),
199196
jsUint8ArrayImplClass =
@@ -289,38 +286,36 @@ class CoreTypesUtil {
289286
Expression castInvocationForReturn(
290287
Expression invocation, DartType returnType) {
291288
if (returnType is VoidType) {
292-
// Technically a `void` return value can still be used, by casting the
293-
// return type to `dynamic` or `Object?`. However this case should be
294-
// extremely rare, and `dartifyRaw` overhead for return values that will
295-
// never be used in practice is too much, so we avoid `dartifyRaw` on
296-
// `void` returns.
297-
return StaticInvocation(internalizeNullable, Arguments([invocation]));
298-
}
299-
300-
Expression expression;
301-
if (isJSValueType(returnType)) {
302-
// TODO(joshualitt): Expose boxed `JSNull` and `JSUndefined` to Dart
303-
// code after migrating existing users of js interop on Dart2Wasm.
304-
// expression = _createJSValue(invocation);
305-
// Casts are expensive, so we stick to a null-assertion if needed. If
306-
// the nullability can't be determined, cast.
307-
expression = invokeOneArg(jsValueBoxTarget, invocation);
308-
final nullability = returnType.extensionTypeErasure.nullability;
309-
if (nullability == Nullability.nonNullable) {
310-
expression = NullCheck(expression);
311-
} else if (nullability == Nullability.undetermined) {
312-
expression = AsExpression(expression, returnType);
313-
}
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);
314293
} else {
315-
// Because we simply don't have enough information, we leave all JS
316-
// numbers as doubles. However, in cases where we know the user expects
317-
// an `int` we check that the double is an integer, and then insert a
318-
// cast. We also let static interop types flow through without
319-
// conversion, both as arguments, and as the return type.
320-
expression = convertAndCast(
321-
returnType, invokeOneArg(dartifyRawTarget, invocation));
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));
316+
}
317+
return expression;
322318
}
323-
return expression;
324319
}
325320

326321
// Handles any necessary type conversions. Today this is just for handling the

0 commit comments

Comments
 (0)