Skip to content

Commit

Permalink
[dart2wasm] Avoid implicitly internalizing WasmExternRef.
Browse files Browse the repository at this point in the history
CoreLibraryReviewExempt: Wasm only changes.
Change-Id: I0e0a6cddffa1c7f2d7c74779f480dd10bf509c1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279268
Commit-Queue: Joshua Litt <joshualitt@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
  • Loading branch information
joshualitt authored and Commit Queue committed Feb 10, 2023
1 parent d4a9f2b commit 1309036
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 34 deletions.
10 changes: 10 additions & 0 deletions pkg/dart2wasm/lib/intrinsics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,10 @@ class Intrinsifier {
codeGen.wrap(value, w.RefType.extern(nullable: true));
b.extern_internalize();
return w.RefType.any(nullable: true);
case "_wasmExternRefIsNull":
codeGen.wrap(value, w.RefType.extern(nullable: true));
b.ref_is_null();
return w.NumType.i32;
}
}

Expand Down Expand Up @@ -1746,6 +1750,12 @@ class Intrinsifier {
return true;
}

if (member.enclosingClass == translator.wasmExternRefClass &&
name == "nullRef") {
b.ref_null(w.HeapType.noextern);
return true;
}

return false;
}
}
2 changes: 1 addition & 1 deletion pkg/dart2wasm/lib/js_runtime_blob.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const jsRuntimeBlobPart2 = r'''
// Initialize async bridge.
asyncBridge = new WebAssembly.Function(
{parameters: ['externref', 'externref'], results: ['externref']},
{parameters: ['anyref', 'anyref'], results: ['externref']},
dartInstance.exports.$asyncBridge,
{promising: 'first'});
return dartInstance;
Expand Down
16 changes: 14 additions & 2 deletions pkg/dart2wasm/lib/js_runtime_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class _JSLowerer extends Transformer {
final Procedure _jsifyRawTarget;
final Procedure _isDartFunctionWrappedTarget;
final Procedure _wrapDartFunctionTarget;
final Procedure _jsObjectFromDartObjectTarget;
final Procedure _jsValueBoxTarget;
final Procedure _jsValueUnboxTarget;
final Procedure _allowInteropTarget;
Expand Down Expand Up @@ -151,6 +152,8 @@ class _JSLowerer extends Transformer {
.getTopLevelProcedure('dart:_js_helper', '_isDartFunctionWrapped'),
_wrapDartFunctionTarget = _coreTypes.index
.getTopLevelProcedure('dart:_js_helper', '_wrapDartFunction'),
_jsObjectFromDartObjectTarget = _coreTypes.index
.getTopLevelProcedure('dart:_js_helper', 'jsObjectFromDartObject'),
_jsValueConstructor = _coreTypes.index
.getClass('dart:_js_helper', 'JSValue')
.constructors
Expand Down Expand Up @@ -606,7 +609,11 @@ class _JSLowerer extends Transformer {
Arguments([
VariableGet(v),
StaticInvocation(
jsWrapperFunction, Arguments([VariableGet(v)])),
jsWrapperFunction,
Arguments([
StaticInvocation(_jsObjectFromDartObjectTarget,
Arguments([VariableGet(v)]))
])),
], types: [
type
])),
Expand All @@ -625,7 +632,12 @@ class _JSLowerer extends Transformer {
return ConstructorInvocation(
_jsValueConstructor,
Arguments([
StaticInvocation(jsWrapperFunction, Arguments([argument]))
StaticInvocation(
jsWrapperFunction,
Arguments([
StaticInvocation(
_jsObjectFromDartObjectTarget, Arguments([argument]))
]))
]));
}

Expand Down
35 changes: 14 additions & 21 deletions pkg/dart2wasm/lib/translator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ class Translator with KernelNodes {

w.DefinedFunction generateGetMain(Procedure mainFunction) {
w.DefinedFunction getMain = m.addFunction(
m.addFunctionType(const [], const [w.RefType.extern(nullable: true)]));
m.addFunctionType(const [], const [w.RefType.any(nullable: true)]));
constants.instantiateConstant(getMain, getMain.body,
StaticTearOffConstant(mainFunction), getMain.type.outputs.single);
getMain.body.end();
Expand Down Expand Up @@ -568,17 +568,22 @@ class Translator with KernelNodes {
}

/// Translate a Dart type as it should appear on parameters and returns of
/// imported and exported functions. The only reference types allowed here
/// for JS interop are `externref` and `funcref`.
///
/// imported and exported functions. All wasm types are allowed on the interop
/// boundary, but in order to be compatible with the `--closed-world` mode of
/// Binaryen, we coerce all reference types to abstract reference types
/// (`anyref`, `funcref` or `externref`).
/// This function can be called before the class info is built.
w.ValueType translateExternalType(DartType type) {
bool isPotentiallyNullable = type.isPotentiallyNullable;
if (type is InterfaceType) {
Class cls = type.classNode;
if (cls == wasmFuncRefClass || cls == wasmFunctionClass) {
return w.RefType.func(nullable: true);
return w.RefType.func(nullable: isPotentiallyNullable);
}
if (cls == wasmExternRefClass) {
return w.RefType.extern(nullable: isPotentiallyNullable);
}
if (!type.isPotentiallyNullable) {
if (!isPotentiallyNullable) {
w.StorageType? builtin = builtinTypes[cls];
if (builtin != null && builtin.isPrimitive) {
return builtin as w.ValueType;
Expand All @@ -588,7 +593,9 @@ class Translator with KernelNodes {
}
}
}
return w.RefType.extern(nullable: true);
// TODO(joshualitt): We'd like to use the potential nullability here too,
// but unfortunately this seems to break things.
return w.RefType.any(nullable: true);
}

w.DefinedGlobal makeFunctionRef(w.BaseFunction f) {
Expand Down Expand Up @@ -773,16 +780,6 @@ class Translator with KernelNodes {
}
}

bool fromIsExtern = from.isSubtypeOf(w.RefType.extern(nullable: true));
bool toIsExtern = to.isSubtypeOf(w.RefType.extern(nullable: true));
if (fromIsExtern && !toIsExtern) {
b.extern_internalize();
from = w.RefType.any(nullable: from.nullable);
}
if (!fromIsExtern && toIsExtern) {
to = w.RefType.any(nullable: to.nullable);
}

if (!from.isSubtypeOf(to)) {
if (from is w.RefType && to is w.RefType) {
if (from.withNullability(false).isSubtypeOf(to)) {
Expand Down Expand Up @@ -813,10 +810,6 @@ class Translator with KernelNodes {
throw "Conversion between non-reference types";
}
}

if (!fromIsExtern && toIsExtern) {
b.extern_externalize();
}
}

w.FunctionType signatureFor(Reference target) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/wasm_builder/lib/src/types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class RefType extends ValueType {
const RefType.common({required bool nullable})
: this._(HeapType.common, nullable);

/// A (possibly nullable) reference to the `any` heap type.
/// A (possibly nullable) reference to the `extern` heap type.
const RefType.extern({required bool nullable})
: this._(HeapType.extern, nullable);

Expand Down
4 changes: 2 additions & 2 deletions sdk/lib/_internal/wasm/lib/async_patch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void _callAsyncBridge(WasmStructRef args, Completer<Object?> completer) =>
"(args, completer) => asyncBridge(args, completer)", args, completer);

@pragma("wasm:export", "\$asyncBridge")
WasmAnyRef? _asyncBridge(
WasmExternRef? _asyncBridge(
WasmExternRef? stack, WasmStructRef args, Completer<Object?> completer) {
try {
Object? result = _asyncBridge2(args, stack);
Expand Down Expand Up @@ -112,7 +112,7 @@ Object? _awaitHelper(Object? operand, WasmExternRef? stack) {

Object? _futurePromise(WasmExternRef? stack, Future<Object?> future) =>
JS<Object?>("""new WebAssembly.Function(
{parameters: ['externref', 'externref'], results: ['externref']},
{parameters: ['externref', 'anyref'], results: ['anyref']},
function(future) {
return new Promise(function (resolve, reject) {
dartInstance.exports.\$awaitCallback(future, resolve);
Expand Down
10 changes: 5 additions & 5 deletions sdk/lib/_internal/wasm/lib/js_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ extension ObjectToJS on Object {
}

// For now both `null` and `undefined` in JS map to `null` in Dart.
bool isDartNull(WasmExternRef? ref) => ref == null || isJSUndefined(ref);
bool isDartNull(WasmExternRef? ref) => ref.isNull || isJSUndefined(ref);

// Extensions for [JSArray] and [JSObject].
// TODO(joshualitt): Rewrite using JS types.
Expand Down Expand Up @@ -328,7 +328,7 @@ WasmExternRef? jsArrayBufferFromDartByteBuffer(ByteBuffer buffer) {

WasmExternRef? jsifyRaw(Object? object) {
if (object == null) {
return null;
return WasmExternRef.nullRef;
} else if (object is bool) {
return toJSBoolean(object);
} else if (object is Function) {
Expand Down Expand Up @@ -370,10 +370,10 @@ WasmExternRef? jsifyRaw(Object? object) {
}
}

bool isWasmGCStruct(WasmExternRef ref) => ref.internalize().isObject;
bool isWasmGCStruct(WasmExternRef? ref) => ref.internalize()?.isObject ?? false;

Object? dartifyRaw(WasmExternRef? ref) {
if (ref == null) {
if (ref.isNull) {
return null;
} else if (isJSUndefined(ref)) {
// TODO(joshualitt): Introduce a `JSUndefined` type.
Expand Down Expand Up @@ -413,7 +413,7 @@ Object? dartifyRaw(WasmExternRef? ref) {
} else if (isWasmGCStruct(ref)) {
return jsObjectToDartObject(ref);
} else {
return JSValue(ref);
return JSValue.box(ref);
}
}

Expand Down
6 changes: 6 additions & 0 deletions sdk/lib/wasm/wasm_types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,23 @@ extension ExternalizeNullable on WasmAnyRef? {
/// The Wasm `externref` type.
@pragma("wasm:entry-point")
class WasmExternRef extends _WasmBase {
// To avoid conflating the null externref with Dart's null, we provide a
// special getter for the null externref.
external static WasmExternRef? get nullRef;

WasmAnyRef internalize() => _internalizeNonNullable(this);
}

extension InternalizeNullable on WasmExternRef? {
bool get isNull => _wasmExternRefIsNull(this);
WasmAnyRef? internalize() => _internalizeNullable(this);
}

external WasmExternRef _externalizeNonNullable(WasmAnyRef ref);
external WasmExternRef? _externalizeNullable(WasmAnyRef? ref);
external WasmAnyRef _internalizeNonNullable(WasmExternRef ref);
external WasmAnyRef? _internalizeNullable(WasmExternRef? ref);
external bool _wasmExternRefIsNull(WasmExternRef? ref);

/// The Wasm `funcref` type.
@pragma("wasm:entry-point")
Expand Down
5 changes: 3 additions & 2 deletions tests/web/wasm/wasm_types_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test() {
Object dartObject1 = "1";
Object dartObject2 = true;
Object dartObject3 = Object();
WasmAnyRef jsObject1 = createObject(null)!.internalize();
WasmAnyRef jsObject1 = createObject(WasmExternRef.nullRef)!.internalize();

// A JS object is not a Dart object.
Expect.isFalse(jsObject1.isObject);
Expand Down Expand Up @@ -86,7 +86,8 @@ test() {

// Create a typed function reference from an import and call it.
var createObjectFun = WasmFunction.fromFunction(createObject);
WasmAnyRef jsObject3 = createObjectFun.call(null).internalize()!;
WasmAnyRef jsObject3 =
createObjectFun.call(WasmExternRef.nullRef).internalize()!;
Expect.isFalse(jsObject3.isObject);

Expect.equals(3, funCount);
Expand Down

0 comments on commit 1309036

Please sign in to comment.