Commit 518f5c0f authored by Manos Koukoutos's avatar Manos Koukoutos Committed by Commit Bot

[wasm] Fix issues concerning type naming

Changes:
- Simplify and generalize ToValueTypeString.
- Fix some error messages in msjunit so that they reflect the underlying
  error better.
- Change 'exn' -> 'exnref' to match exception-handling proposal.

Bug: v8:7581
Change-Id: I264f6c9aa598a57f39d5a4d01399af64db83a2b9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2243214
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68364}
parent dd522782
......@@ -44,7 +44,7 @@ class Simd128;
V(ExternRef, kSystemPointerSizeLog2, ExternRef, TaggedPointer, 'r', \
"externref") \
V(FuncRef, kSystemPointerSizeLog2, FuncRef, TaggedPointer, 'a', "funcref") \
V(ExnRef, kSystemPointerSizeLog2, ExnRef, TaggedPointer, 'e', "exn") \
V(ExnRef, kSystemPointerSizeLog2, ExnRef, TaggedPointer, 'e', "exnref") \
V(Ref, kSystemPointerSizeLog2, Ref, TaggedPointer, '*', "ref") \
V(OptRef, kSystemPointerSizeLog2, OptRef, TaggedPointer, 'o', "optref") \
V(EqRef, kSystemPointerSizeLog2, EqRef, TaggedPointer, 'q', "eqref") \
......
......@@ -1077,13 +1077,15 @@ void WebAssemblyTable(const v8::FunctionCallbackInfo<v8::Value>& args) {
v8::Local<v8::String> string;
if (!value->ToString(context).ToLocal(&string)) return;
auto enabled_features = i::wasm::WasmFeatures::FromFlags();
// The JS api uses 'anyfunc' instead of 'funcref'.
if (string->StringEquals(v8_str(isolate, "anyfunc"))) {
type = i::wasm::kWasmFuncRef;
} else if (enabled_features.has_reftypes() &&
string->StringEquals(v8_str(isolate, "externref"))) {
type = i::wasm::kWasmExternRef;
} else {
thrower.TypeError("Descriptor property 'element' must be 'anyfunc'");
thrower.TypeError(
"Descriptor property 'element' must be a WebAssembly reference type");
return;
}
}
......@@ -1209,6 +1211,7 @@ bool GetValueType(Isolate* isolate, MaybeLocal<Value> maybe,
} else if (enabled_features.has_reftypes() &&
string->StringEquals(v8_str(isolate, "externref"))) {
*type = i::wasm::kWasmExternRef;
// The JS api spec uses 'anyfunc' instead of 'funcref'.
} else if (enabled_features.has_reftypes() &&
string->StringEquals(v8_str(isolate, "anyfunc"))) {
*type = i::wasm::kWasmFuncRef;
......@@ -1264,8 +1267,7 @@ void WebAssemblyGlobal(const v8::FunctionCallbackInfo<v8::Value>& args) {
if (!GetValueType(isolate, maybe, context, &type, enabled_features)) return;
if (type == i::wasm::kWasmStmt) {
thrower.TypeError(
"Descriptor property 'value' must be 'i32', 'i64', 'f32', or "
"'f64'");
"Descriptor property 'value' must be a WebAssembly type");
return;
}
}
......
......@@ -251,46 +251,9 @@ namespace {
// Converts the given {type} into a string representation that can be used in
// reflective functions. Should be kept in sync with the {GetValueType} helper.
Handle<String> ToValueTypeString(Isolate* isolate, ValueType type) {
// TODO(ahaas/jkummerow): This could be as simple as:
// return isolate->factory()->InternalizeUtf8String(type.type_name());
// if we clean up all occurrences of "anyfunc" in favor of "funcref".
Factory* factory = isolate->factory();
Handle<String> string;
switch (type.kind()) {
case i::wasm::ValueType::kI32: {
string = factory->InternalizeUtf8String("i32");
break;
}
case i::wasm::ValueType::kI64: {
string = factory->InternalizeUtf8String("i64");
break;
}
case i::wasm::ValueType::kF32: {
string = factory->InternalizeUtf8String("f32");
break;
}
case i::wasm::ValueType::kF64: {
string = factory->InternalizeUtf8String("f64");
break;
}
case i::wasm::ValueType::kExternRef: {
string = factory->InternalizeUtf8String("externref");
break;
}
case i::wasm::ValueType::kFuncRef: {
string = factory->InternalizeUtf8String("anyfunc");
break;
}
case i::wasm::ValueType::kExnRef: {
string = factory->InternalizeUtf8String("exnref");
break;
}
default:
UNREACHABLE();
}
return string;
return isolate->factory()->InternalizeUtf8String(
type == kWasmFuncRef ? "anyfunc" : type.type_name());
}
} // namespace
Handle<JSObject> GetTypeForFunction(Isolate* isolate, const FunctionSig* sig) {
......@@ -366,8 +329,8 @@ Handle<JSObject> GetTypeForTable(Isolate* isolate, ValueType type,
Handle<String> element;
if (type == kWasmFuncRef) {
// TODO(wasm): We should define the "anyfunc" string in one central place
// and then use that constant everywhere.
// TODO(wasm): We should define the "anyfunc" string in one central
// place and then use that constant everywhere.
element = factory->InternalizeUtf8String("anyfunc");
} else {
DCHECK(WasmFeatures::FromFlags().has_reftypes() && type == kWasmExternRef);
......
......@@ -164,8 +164,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
try {
new WebAssembly.Global(argument);
} catch (e) {
assertContains("'value' must be", e.message);
assertContains("i64", e.message);
assertContains("'value' must be a WebAssembly type", e.message);
}
})();
......
......@@ -582,13 +582,13 @@ assertThrows(
assertThrows(
() => new Table(1), TypeError, 'WebAssembly.Module(): Argument 0 must be a table descriptor');
assertThrows(
() => new Table({initial: 1, element: 1}), TypeError, /must be 'anyfunc'/);
() => new Table({initial: 1, element: 1}), TypeError, /must be a WebAssembly reference type/);
assertThrows(
() => new Table({initial: 1, element: 'any'}), TypeError,
/must be 'anyfunc'/);
/must be a WebAssembly reference type/);
assertThrows(
() => new Table({initial: 1, element: {valueOf() { return 'anyfunc' }}}),
TypeError, /must be 'anyfunc'/);
TypeError, /must be a WebAssembly reference type/);
assertThrows(
() => new Table(
{initial: {valueOf() { throw new Error('here') }}, element: 'anyfunc'}),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment