From 123244977aafee9766177f433e963af51611c287 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 20 Apr 2022 09:03:11 -0700 Subject: [PATCH 1/2] Add a `Type::String` variant This removes the implicit interpretation that `list` is equivalent to `string`, enabling `list` to be equivalent to `&[char]` in Rust, for example. This commit is plumbed throughout all generators with new pseudo-instructions for lifting/lowering strings. --- crates/gen-c/src/lib.rs | 61 ++++----- crates/gen-core/src/lib.rs | 1 + crates/gen-js/src/lib.rs | 129 ++++++++++-------- crates/gen-markdown/src/lib.rs | 2 +- crates/gen-rust-wasm/src/lib.rs | 58 +++++--- crates/gen-rust/src/lib.rs | 41 +++--- crates/gen-spidermonkey/src/lib.rs | 20 +-- crates/gen-wasmtime-py/src/lib.rs | 110 ++++++++------- crates/gen-wasmtime/src/lib.rs | 109 ++++++++++----- crates/parser/src/abi.rs | 106 +++++++++----- crates/parser/src/ast.rs | 3 +- crates/parser/src/ast/resolve.rs | 1 + crates/parser/src/lib.rs | 3 +- crates/parser/src/sizealign.rs | 6 +- crates/parser/tests/all.rs | 1 + .../parser/tests/ui/type-then-eof.wit.result | 8 +- crates/parser/tests/ui/types.wit.result | 4 +- crates/parser/tests/ui/wasi-http.wit.result | 26 ++-- crates/test-helpers/src/lib.rs | 9 +- crates/wasmlink/src/adapter/call.rs | 47 +++++-- crates/wasmlink/src/module.rs | 1 + crates/wit-component/src/decoding.rs | 11 +- crates/wit-component/src/encoding.rs | 9 +- crates/wit-component/src/printing.rs | 14 +- 24 files changed, 448 insertions(+), 332 deletions(-) diff --git a/crates/gen-c/src/lib.rs b/crates/gen-c/src/lib.rs index 79474e7c4..2046e5043 100644 --- a/crates/gen-c/src/lib.rs +++ b/crates/gen-c/src/lib.rs @@ -175,6 +175,7 @@ impl C { TypeDefKind::Record(r) if r.is_flags() => false, TypeDefKind::Record(_) | TypeDefKind::List(_) => true, }, + Type::String => true, _ => false, } } @@ -230,6 +231,11 @@ impl C { self.src.h(&iface.resources[*id].name.to_snake_case()); self.src.h("_t"); } + Type::String => { + self.print_namespace(iface); + self.src.h("string_t"); + self.needs_string = true; + } Type::Id(id) => { let ty = &iface.types[*id]; if let Some(name) = &ty.name { @@ -250,11 +256,6 @@ impl C { self.print_ty_name(iface, &Type::Id(*id)); self.src.h("_t"); } - TypeDefKind::List(Type::Char) => { - self.print_namespace(iface); - self.src.h("string_t"); - self.needs_string = true; - } TypeDefKind::Record(_) | TypeDefKind::List(_) => { self.public_anonymous_types.insert(*id); self.private_anonymous_types.remove(id); @@ -281,6 +282,7 @@ impl C { Type::Float32 => self.src.h("float32"), Type::Float64 => self.src.h("float64"), Type::Handle(id) => self.src.h(&iface.resources[*id].name.to_snake_case()), + Type::String => self.src.h("string"), Type::Id(id) => { let ty = &iface.types[*id]; if let Some(name) = &ty.name { @@ -318,7 +320,6 @@ impl C { unimplemented!(); } } - TypeDefKind::List(Type::Char) => self.src.h("string"), TypeDefKind::List(t) => { self.src.h("list_"); self.print_ty_name(iface, t); @@ -484,14 +485,10 @@ impl C { self.free(iface, t, "&ptr->ptr[i]"); self.src.c("}\n"); } - let (size, align) = if *t == Type::Char { - (1, 1) - } else { - (self.sizes.size(t), self.sizes.align(t)) - }; self.src.c(&format!( "canonical_abi_free(ptr->ptr, ptr->len * {}, {});\n", - size, align, + self.sizes.size(t), + self.sizes.align(t), )); } @@ -524,6 +521,7 @@ impl C { fn owns_anything(&self, iface: &Interface, ty: &Type) -> bool { let id = match ty { Type::Id(id) => *id, + Type::String => return true, Type::Handle(_) => return true, _ => return false, }; @@ -568,6 +566,10 @@ impl Return { fn return_single(&mut self, iface: &Interface, ty: &Type, orig_ty: &Type) { let id = match ty { Type::Id(id) => *id, + Type::String => { + self.retptrs.push(*orig_ty); + return; + } _ => { self.scalar = Some(Scalar::Type(*orig_ty)); return; @@ -799,17 +801,11 @@ impl Generator for C { fn type_list(&mut self, iface: &Interface, id: TypeId, name: &str, ty: &Type, docs: &Docs) { let prev = mem::take(&mut self.src.header); self.docs(docs); - if *ty == Type::Char { - self.src.h("typedef "); - self.print_namespace(iface); - self.src.h("string_t "); - } else { - self.src.h("typedef struct {\n"); - self.print_ty(iface, ty); - self.src.h(" *ptr;\n"); - self.src.h("size_t len;\n"); - self.src.h("} "); - } + self.src.h("typedef struct {\n"); + self.print_ty(iface, ty); + self.src.h(" *ptr;\n"); + self.src.h("size_t len;\n"); + self.src.h("} "); self.print_namespace(iface); self.src.h(&name.to_snake_case()); self.src.h("_t;\n"); @@ -1518,21 +1514,25 @@ impl Bindgen for FunctionBindgen<'_> { results.push(result); } - Instruction::ListCanonLower { .. } => { + Instruction::ListCanonLower { .. } | Instruction::StringLower { .. } => { results.push(format!("(int32_t) ({}).ptr", operands[0])); results.push(format!("(int32_t) ({}).len", operands[0])); } Instruction::ListCanonLift { element, ty, .. } => { let list_name = self.gen.type_string(iface, &Type::Id(*ty)); - let elem_name = match element { - Type::Char => "char".into(), - _ => self.gen.type_string(iface, element), - }; + let elem_name = self.gen.type_string(iface, element); results.push(format!( "({}) {{ ({}*)({}), (size_t)({}) }}", list_name, elem_name, operands[0], operands[1] )); } + Instruction::StringLift { .. } => { + let list_name = self.gen.type_string(iface, &Type::String); + results.push(format!( + "({}) {{ (char*)({}), (size_t)({}) }}", + list_name, operands[0], operands[1] + )); + } Instruction::ListLower { .. } => { let _body = self.blocks.pop().unwrap(); @@ -1543,10 +1543,7 @@ impl Bindgen for FunctionBindgen<'_> { Instruction::ListLift { element, ty, .. } => { let _body = self.blocks.pop().unwrap(); let list_name = self.gen.type_string(iface, &Type::Id(*ty)); - let elem_name = match element { - Type::Char => "char".into(), - _ => self.gen.type_string(iface, element), - }; + let elem_name = self.gen.type_string(iface, element); results.push(format!( "({}) {{ ({}*)({}), (size_t)({}) }}", list_name, elem_name, operands[0], operands[1] diff --git a/crates/gen-core/src/lib.rs b/crates/gen-core/src/lib.rs index 959b99f9b..f3d549541 100644 --- a/crates/gen-core/src/lib.rs +++ b/crates/gen-core/src/lib.rs @@ -213,6 +213,7 @@ impl Types { let mut info = TypeInfo::default(); match ty { Type::Handle(_) => info.has_handle = true, + Type::String => info.has_list = true, Type::Id(id) => return self.type_id_info(iface, *id), _ => {} } diff --git a/crates/gen-js/src/lib.rs b/crates/gen-js/src/lib.rs index b3809957c..4ee765796 100644 --- a/crates/gen-js/src/lib.rs +++ b/crates/gen-js/src/lib.rs @@ -135,6 +135,7 @@ impl Js { Type::Float64 => Some("Float64Array"), Type::Char => None, Type::Handle(_) => None, + Type::String => None, Type::Id(id) => match &iface.types[*id].kind { TypeDefKind::Type(t) => self.array_ty(iface, t), _ => None, @@ -155,6 +156,7 @@ impl Js { Type::U64 | Type::S64 => self.src.ts("bigint"), Type::Char => self.src.ts("string"), Type::Handle(id) => self.src.ts(&iface.resources[*id].name.to_camel_case()), + Type::String => self.src.ts("string"), Type::Id(id) => { let ty = &iface.types[*id]; if let Some(name) = &ty.name { @@ -201,12 +203,8 @@ impl Js { match self.array_ty(iface, ty) { Some(ty) => self.src.ts(ty), None => { - if let Type::Char = ty { - self.src.ts("string"); - } else { - self.print_ty(iface, ty); - self.src.ts("[]"); - } + self.print_ty(iface, ty); + self.src.ts("[]"); } } } @@ -1629,34 +1627,20 @@ impl Bindgen for FunctionBindgen<'_> { self.needs_realloc = Some(realloc.to_string()); let tmp = self.tmp(); - match element { - Type::Char => { - let encode = self.gen.intrinsic(Intrinsic::Utf8Encode); - self.src.js(&format!( - "const ptr{} = {}({}, realloc, memory);\n", - tmp, encode, operands[0], - )); - let encoded_len = self.gen.intrinsic(Intrinsic::Utf8EncodedLen); - self.src - .js(&format!("const len{} = {};\n", tmp, encoded_len)); - } - _ => { - let size = self.gen.sizes.size(element); - let align = self.gen.sizes.align(element); - self.src - .js(&format!("const val{} = {};\n", tmp, operands[0])); - self.src.js(&format!("const len{} = val{0}.length;\n", tmp)); - self.src.js(&format!( - "const ptr{} = realloc(0, 0, {}, len{0} * {});\n", - tmp, align, size, - )); - // TODO: this is the wrong endianness - self.src.js(&format!( - "(new Uint8Array(memory.buffer, ptr{0}, len{0} * {1})).set(new Uint8Array(val{0}.buffer, val{0}.byteOffset, len{0} * {1}));\n", - tmp, size, - )); - } - }; + let size = self.gen.sizes.size(element); + let align = self.gen.sizes.align(element); + self.src + .js(&format!("const val{} = {};\n", tmp, operands[0])); + self.src.js(&format!("const len{} = val{0}.length;\n", tmp)); + self.src.js(&format!( + "const ptr{} = realloc(0, 0, {}, len{0} * {});\n", + tmp, align, size, + )); + // TODO: this is the wrong endianness + self.src.js(&format!( + "(new Uint8Array(memory.buffer, ptr{0}, len{0} * {1})).set(new Uint8Array(val{0}.buffer, val{0}.byteOffset, len{0} * {1}));\n", + tmp, size, + )); results.push(format!("ptr{}", tmp)); results.push(format!("len{}", tmp)); } @@ -1667,31 +1651,15 @@ impl Bindgen for FunctionBindgen<'_> { .js(&format!("const ptr{} = {};\n", tmp, operands[0])); self.src .js(&format!("const len{} = {};\n", tmp, operands[1])); - let (result, align) = match element { - Type::Char => { - let decoder = self.gen.intrinsic(Intrinsic::Utf8Decoder); - ( - format!( - "{}.decode(new Uint8Array(memory.buffer, ptr{}, len{1}))", - decoder, tmp, - ), - 1, - ) - } - _ => { - // TODO: this is the wrong endianness - let array_ty = self.gen.array_ty(iface, element).unwrap(); - ( - format!( - "new {}(memory.buffer.slice(ptr{}, ptr{1} + len{1} * {}))", - array_ty, - tmp, - self.gen.sizes.size(element), - ), - self.gen.sizes.align(element), - ) - } - }; + // TODO: this is the wrong endianness + let array_ty = self.gen.array_ty(iface, element).unwrap(); + let result = format!( + "new {}(memory.buffer.slice(ptr{}, ptr{1} + len{1} * {}))", + array_ty, + tmp, + self.gen.sizes.size(element), + ); + let align = self.gen.sizes.align(element); match free { Some(free) => { self.needs_free = Some(free.to_string()); @@ -1703,6 +1671,49 @@ impl Bindgen for FunctionBindgen<'_> { None => results.push(result), } } + Instruction::StringLower { realloc } => { + // Lowering only happens when we're passing lists into wasm, + // which forces us to always allocate, so this should always be + // `Some`. + let realloc = realloc.unwrap(); + self.gen.needs_get_export = true; + self.needs_memory = true; + self.needs_realloc = Some(realloc.to_string()); + let tmp = self.tmp(); + + let encode = self.gen.intrinsic(Intrinsic::Utf8Encode); + self.src.js(&format!( + "const ptr{} = {}({}, realloc, memory);\n", + tmp, encode, operands[0], + )); + let encoded_len = self.gen.intrinsic(Intrinsic::Utf8EncodedLen); + self.src + .js(&format!("const len{} = {};\n", tmp, encoded_len)); + results.push(format!("ptr{}", tmp)); + results.push(format!("len{}", tmp)); + } + Instruction::StringLift { free } => { + self.needs_memory = true; + let tmp = self.tmp(); + self.src + .js(&format!("const ptr{} = {};\n", tmp, operands[0])); + self.src + .js(&format!("const len{} = {};\n", tmp, operands[1])); + let decoder = self.gen.intrinsic(Intrinsic::Utf8Decoder); + let result = format!( + "{}.decode(new Uint8Array(memory.buffer, ptr{}, len{1}))", + decoder, tmp, + ); + match free { + Some(free) => { + self.needs_free = Some(free.to_string()); + self.src.js(&format!("const list{} = {};\n", tmp, result)); + self.src.js(&format!("free(ptr{}, len{0}, 1);\n", tmp)); + results.push(format!("list{}", tmp)); + } + None => results.push(result), + } + } Instruction::ListLower { element, realloc } => { let realloc = realloc.unwrap(); diff --git a/crates/gen-markdown/src/lib.rs b/crates/gen-markdown/src/lib.rs index cbb7343c3..279c0f1f4 100644 --- a/crates/gen-markdown/src/lib.rs +++ b/crates/gen-markdown/src/lib.rs @@ -46,6 +46,7 @@ impl Markdown { Type::Float32 => self.src.push_str("`float32`"), Type::Float64 => self.src.push_str("`float64`"), Type::Char => self.src.push_str("`char`"), + Type::String => self.src.push_str("`string`"), Type::Handle(id) => { self.src.push_str("handle<"); self.src.push_str(&iface.resources[*id].name); @@ -99,7 +100,6 @@ impl Markdown { unreachable!() } } - TypeDefKind::List(Type::Char) => self.src.push_str("`string`"), TypeDefKind::List(t) => { self.src.push_str("list<"); self.print_ty(iface, t, false); diff --git a/crates/gen-rust-wasm/src/lib.rs b/crates/gen-rust-wasm/src/lib.rs index 11852cbac..55a0c3c64 100644 --- a/crates/gen-rust-wasm/src/lib.rs +++ b/crates/gen-rust-wasm/src/lib.rs @@ -1001,7 +1001,7 @@ impl Bindgen for FunctionBindgen<'_> { results.push(result); } - Instruction::ListCanonLower { element, realloc } => { + Instruction::ListCanonLower { realloc, .. } => { let tmp = self.tmp(); let val = format!("vec{}", tmp); let ptr = format!("ptr{}", tmp); @@ -1009,12 +1009,7 @@ impl Bindgen for FunctionBindgen<'_> { if realloc.is_none() { self.push_str(&format!("let {} = {};\n", val, operands[0])); } else { - let op0 = match element { - Type::Char => { - format!("{}.into_bytes()", operands[0]) - } - _ => operands.pop().unwrap(), - }; + let op0 = operands.pop().unwrap(); self.push_str(&format!("let {} = ({}).into_boxed_slice();\n", val, op0)); } self.push_str(&format!("let {} = {}.as_ptr() as i32;\n", ptr, val)); @@ -1026,7 +1021,7 @@ impl Bindgen for FunctionBindgen<'_> { results.push(len); } - Instruction::ListCanonLift { element, free, .. } => { + Instruction::ListCanonLift { free, .. } => { // This only happens when we're receiving a list from the // outside world, so `free` should always be `Some`. assert!(free.is_some()); @@ -1037,15 +1032,44 @@ impl Bindgen for FunctionBindgen<'_> { "Vec::from_raw_parts({} as *mut _, {1}, {1})", operands[0], len ); - match element { - Type::Char => { - if unchecked { - results.push(format!("String::from_utf8_unchecked({})", result)); - } else { - results.push(format!("String::from_utf8({}).unwrap()", result)); - } - } - _ => results.push(result), + results.push(result); + } + + Instruction::StringLower { realloc } => { + let tmp = self.tmp(); + let val = format!("vec{}", tmp); + let ptr = format!("ptr{}", tmp); + let len = format!("len{}", tmp); + if realloc.is_none() { + self.push_str(&format!("let {} = {};\n", val, operands[0])); + } else { + let op0 = format!("{}.into_bytes()", operands[0]); + self.push_str(&format!("let {} = ({}).into_boxed_slice();\n", val, op0)); + } + self.push_str(&format!("let {} = {}.as_ptr() as i32;\n", ptr, val)); + self.push_str(&format!("let {} = {}.len() as i32;\n", len, val)); + if realloc.is_some() { + self.push_str(&format!("core::mem::forget({});\n", val)); + } + results.push(ptr); + results.push(len); + } + + Instruction::StringLift { free, .. } => { + // This only happens when we're receiving a list from the + // outside world, so `free` should always be `Some`. + assert!(free.is_some()); + let tmp = self.tmp(); + let len = format!("len{}", tmp); + self.push_str(&format!("let {} = {} as usize;\n", len, operands[1])); + let result = format!( + "Vec::from_raw_parts({} as *mut _, {1}, {1})", + operands[0], len + ); + if unchecked { + results.push(format!("String::from_utf8_unchecked({})", result)); + } else { + results.push(format!("String::from_utf8({}).unwrap()", result)); } } diff --git a/crates/gen-rust/src/lib.rs b/crates/gen-rust/src/lib.rs index 1d2915e6a..f8be7766b 100644 --- a/crates/gen-rust/src/lib.rs +++ b/crates/gen-rust/src/lib.rs @@ -208,6 +208,12 @@ pub trait RustGenerator { Type::Float32 => self.push_str("f32"), Type::Float64 => self.push_str("f64"), Type::Char => self.push_str("char"), + Type::String => match mode { + TypeMode::AllBorrowed(lt) | TypeMode::LeafBorrowed(lt) => { + self.print_borrowed_str(lt) + } + TypeMode::Owned | TypeMode::HandlesBorrowed(_) => self.push_str("String"), + }, } } @@ -236,6 +242,7 @@ pub trait RustGenerator { match ty { TypeDefKind::Variant(_) | TypeDefKind::Record(_) | TypeDefKind::List(_) => true, TypeDefKind::Type(Type::Id(t)) => needs_generics(iface, &iface.types[*t].kind), + TypeDefKind::Type(Type::String) => true, TypeDefKind::Type(Type::Handle(_)) => true, _ => false, } @@ -292,32 +299,24 @@ pub trait RustGenerator { } fn print_list(&mut self, iface: &Interface, ty: &Type, mode: TypeMode) { - match ty { - Type::Char => match mode { - TypeMode::AllBorrowed(lt) | TypeMode::LeafBorrowed(lt) => { - self.print_borrowed_str(lt) - } - TypeMode::Owned | TypeMode::HandlesBorrowed(_) => self.push_str("String"), - }, - t => match mode { - TypeMode::AllBorrowed(lt) => { + match mode { + TypeMode::AllBorrowed(lt) => { + self.print_borrowed_slice(iface, false, ty, lt); + } + TypeMode::LeafBorrowed(lt) => { + if iface.all_bits_valid(ty) { self.print_borrowed_slice(iface, false, ty, lt); - } - TypeMode::LeafBorrowed(lt) => { - if iface.all_bits_valid(t) { - self.print_borrowed_slice(iface, false, ty, lt); - } else { - self.push_str("Vec<"); - self.print_ty(iface, ty, mode); - self.push_str(">"); - } - } - TypeMode::HandlesBorrowed(_) | TypeMode::Owned => { + } else { self.push_str("Vec<"); self.print_ty(iface, ty, mode); self.push_str(">"); } - }, + } + TypeMode::HandlesBorrowed(_) | TypeMode::Owned => { + self.push_str("Vec<"); + self.print_ty(iface, ty, mode); + self.push_str(">"); + } } } diff --git a/crates/gen-spidermonkey/src/lib.rs b/crates/gen-spidermonkey/src/lib.rs index ccf3cf97d..7d87f9f00 100644 --- a/crates/gen-spidermonkey/src/lib.rs +++ b/crates/gen-spidermonkey/src/lib.rs @@ -1507,7 +1507,8 @@ impl abi::Bindgen for Bindgen<'_, '_> { abi::Instruction::I32FromOwnedHandle { ty: _ } => todo!(), abi::Instruction::HandleOwnedFromI32 { ty: _ } => todo!(), abi::Instruction::HandleBorrowedFromI32 { ty: _ } => todo!(), - abi::Instruction::ListCanonLower { element, realloc } => { + abi::Instruction::ListCanonLower { .. } => todo!(), + abi::Instruction::StringLower { realloc } => { let js = pop_js(operands); let ptr = self.new_local(wasm_encoder::ValType::I32); let len = self.new_local(wasm_encoder::ValType::I32); @@ -1551,11 +1552,7 @@ impl abi::Bindgen for Bindgen<'_, '_> { // If `realloc` is `None`, then we are responsible for freeing // this pointer after the call. if realloc.is_none() { - self.to_free.push(( - ptr, - len, - u32::try_from(self.gen.sizes.align(element)).unwrap(), - )); + self.to_free.push((ptr, len, 1)); } results.push(Operand::Wasm(ptr)); @@ -1682,13 +1679,8 @@ impl abi::Bindgen for Bindgen<'_, '_> { results.push(Operand::Wasm(ptr)); results.push(Operand::Wasm(length)); } - abi::Instruction::ListCanonLift { - element, - free, - ty: _, - } => { - assert_eq!(**element, Type::Char); - + abi::Instruction::ListCanonLift { .. } => todo!(), + abi::Instruction::StringLift { free } => { let len = pop_wasm(operands); let ptr = pop_wasm(operands); let result = self.next_js(); @@ -1711,7 +1703,7 @@ impl abi::Bindgen for Bindgen<'_, '_> { // [i32] self.inst(Instruction::LocalGet(len)); // [i32 i32] - self.inst(Instruction::I32Const(self.gen.sizes.align(element) as _)); + self.inst(Instruction::I32Const(1)); // [i32 i32 i32] self.inst(Instruction::Call(self.gen.spidermonkey_import(free))); // [] diff --git a/crates/gen-wasmtime-py/src/lib.rs b/crates/gen-wasmtime-py/src/lib.rs index 277192a97..ace6c1015 100644 --- a/crates/gen-wasmtime-py/src/lib.rs +++ b/crates/gen-wasmtime-py/src/lib.rs @@ -380,6 +380,7 @@ impl WasmtimePy { | Type::S64 => self.src.push_str("int"), Type::Float32 | Type::Float64 => self.src.push_str("float"), Type::Char => self.src.push_str("str"), + Type::String => self.src.push_str("str"), Type::Handle(id) => { // In general we want to use quotes around this to support // forward-references (such as a method on a resource returning @@ -460,7 +461,6 @@ impl WasmtimePy { fn print_list(&mut self, iface: &Interface, element: &Type) { match element { - Type::Char => self.src.push_str("str"), Type::U8 => self.src.push_str("bytes"), t => { self.pyimport("typing", "List"); @@ -503,6 +503,7 @@ impl WasmtimePy { Type::Float64 => Some("c_double"), Type::Char => None, Type::Handle(_) => None, + Type::String => None, Type::Id(id) => match &iface.types[*id].kind { TypeDefKind::Type(t) => self.array_ty(iface, t), _ => None, @@ -1720,25 +1721,14 @@ impl Bindgen for FunctionBindgen<'_> { let ptr = self.locals.tmp("ptr"); let len = self.locals.tmp("len"); - match element { - Type::Char => { - self.gen.needs_encode_utf8 = true; - self.src.push_str(&format!( - "{}, {} = _encode_utf8({}, realloc, memory, caller)\n", - ptr, len, operands[0], - )); - } - _ => { - let array_ty = self.gen.array_ty(iface, element).unwrap(); - self.gen.needs_list_canon_lower = true; - let size = self.gen.sizes.size(element); - let align = self.gen.sizes.align(element); - self.src.push_str(&format!( - "{}, {} = _list_canon_lower({}, ctypes.{}, {}, {}, realloc, memory, caller)\n", - ptr, len, operands[0], array_ty, size, align, - )); - } - }; + let array_ty = self.gen.array_ty(iface, element).unwrap(); + self.gen.needs_list_canon_lower = true; + let size = self.gen.sizes.size(element); + let align = self.gen.sizes.align(element); + self.src.push_str(&format!( + "{}, {} = _list_canon_lower({}, ctypes.{}, {}, {}, realloc, memory, caller)\n", + ptr, len, operands[0], array_ty, size, align, + )); results.push(ptr); results.push(len); } @@ -1748,35 +1738,25 @@ impl Bindgen for FunctionBindgen<'_> { let len = self.locals.tmp("len"); self.src.push_str(&format!("{} = {}\n", ptr, operands[0])); self.src.push_str(&format!("{} = {}\n", len, operands[1])); - let (result, align) = match element { - Type::Char => { - self.gen.needs_decode_utf8 = true; - (format!("_decode_utf8(memory, caller, {}, {})", ptr, len), 1) - } + let array_ty = self.gen.array_ty(iface, element).unwrap(); + self.gen.needs_list_canon_lift = true; + let lift = format!( + "_list_canon_lift({}, {}, {}, ctypes.{}, memory, caller)", + ptr, + len, + self.gen.sizes.size(element), + array_ty, + ); + let pyty = match element { + Type::U8 => "bytes".to_string(), _ => { - let array_ty = self.gen.array_ty(iface, element).unwrap(); - self.gen.needs_list_canon_lift = true; - let lift = format!( - "_list_canon_lift({}, {}, {}, ctypes.{}, memory, caller)", - ptr, - len, - self.gen.sizes.size(element), - array_ty, - ); - let pyty = match element { - Type::U8 => "bytes".to_string(), - _ => { - self.gen.pyimport("typing", "List"); - format!("List[{}]", self.gen.type_string(iface, element)) - } - }; - self.gen.pyimport("typing", "cast"); - ( - format!("cast({}, {})", pyty, lift), - self.gen.sizes.align(element), - ) + self.gen.pyimport("typing", "List"); + format!("List[{}]", self.gen.type_string(iface, element)) } }; + self.gen.pyimport("typing", "cast"); + let result = format!("cast({}, {})", pyty, lift); + let align = self.gen.sizes.align(element); match free { Some(free) => { self.needs_free = Some(free.to_string()); @@ -1789,6 +1769,44 @@ impl Bindgen for FunctionBindgen<'_> { None => results.push(result), } } + Instruction::StringLower { realloc } => { + // Lowering only happens when we're passing lists into wasm, + // which forces us to always allocate, so this should always be + // `Some`. + let realloc = realloc.unwrap(); + self.needs_memory = true; + self.needs_realloc = Some(realloc.to_string()); + + let ptr = self.locals.tmp("ptr"); + let len = self.locals.tmp("len"); + self.gen.needs_encode_utf8 = true; + self.src.push_str(&format!( + "{}, {} = _encode_utf8({}, realloc, memory, caller)\n", + ptr, len, operands[0], + )); + results.push(ptr); + results.push(len); + } + Instruction::StringLift { free, .. } => { + self.needs_memory = true; + let ptr = self.locals.tmp("ptr"); + let len = self.locals.tmp("len"); + self.src.push_str(&format!("{} = {}\n", ptr, operands[0])); + self.src.push_str(&format!("{} = {}\n", len, operands[1])); + self.gen.needs_decode_utf8 = true; + let result = format!("_decode_utf8(memory, caller, {}, {})", ptr, len); + match free { + Some(free) => { + self.needs_free = Some(free.to_string()); + let list = self.locals.tmp("list"); + self.src.push_str(&format!("{} = {}\n", list, result)); + self.src + .push_str(&format!("free(caller, {}, {}, 1)\n", ptr, len)); + results.push(list); + } + None => results.push(result), + } + } Instruction::ListLower { element, realloc } => { let base = self.payloads.pop().unwrap(); diff --git a/crates/gen-wasmtime/src/lib.rs b/crates/gen-wasmtime/src/lib.rs index 7df7d3f12..bb930a388 100644 --- a/crates/gen-wasmtime/src/lib.rs +++ b/crates/gen-wasmtime/src/lib.rs @@ -1616,17 +1616,10 @@ impl Bindgen for FunctionBindgen<'_> { // Lowering only happens when we're passing lists into wasm, // which forces us to always allocate, so this should always be // `Some`. - // - // Note that the size of a list of `char` is 1 because it's - // encoded as utf-8, otherwise it's just normal contiguous array - // elements. let realloc = realloc.unwrap(); self.needs_functions .insert(realloc.to_string(), NeededFunction::Realloc); - let (size, align) = match element { - Type::Char => (1, 1), - _ => (self.gen.sizes.size(element), self.gen.sizes.align(element)), - }; + let (size, align) = (self.gen.sizes.size(element), self.gen.sizes.align(element)); // Store the operand into a temporary... let tmp = self.tmp(); @@ -1643,10 +1636,7 @@ impl Bindgen for FunctionBindgen<'_> { // ... and then copy over the result. let mem = self.memory_src(); - self.push_str(&format!( - "{}.store_many({}, {}.as_ref())?;\n", - mem, ptr, val - )); + self.push_str(&format!("{}.store_many({}, &{})?;\n", mem, ptr, val)); self.gen.needs_raw_mem = true; self.needs_memory = true; results.push(ptr); @@ -1659,14 +1649,8 @@ impl Bindgen for FunctionBindgen<'_> { self.gen.needs_copy_slice = true; self.needs_functions .insert(free.to_string(), NeededFunction::Free); - let (stringify, align, el_size) = match element { - Type::Char => (true, 1, 1), - _ => ( - false, - self.sizes().align(element), - self.sizes().size(element), - ), - }; + let (align, el_size) = + (self.sizes().align(element), self.sizes().size(element)); let tmp = self.tmp(); self.push_str(&format!("let ptr{} = {};\n", tmp, operands[0])); self.push_str(&format!("let len{} = {};\n", tmp, operands[1])); @@ -1687,26 +1671,83 @@ impl Bindgen for FunctionBindgen<'_> { // already verified that multiplied size fits i32 format!("(ptr{tmp}, len{tmp} * {}, {})", el_size, align, tmp = tmp), ); - if stringify { - results.push(format!( - "String::from_utf8(data{}) - .map_err(|_| wasmtime::Trap::new(\"invalid utf-8\"))?", - tmp, - )); - } else { - results.push(format!("data{}", tmp)); - } + results.push(format!("data{}", tmp)); + } + None => { + self.needs_borrow_checker = true; + let tmp = self.tmp(); + self.push_str(&format!("let ptr{} = {};\n", tmp, operands[0])); + self.push_str(&format!("let len{} = {};\n", tmp, operands[1])); + let slice = format!("_bc.slice(ptr{0}, len{0})?", tmp); + results.push(slice); + } + }, + + Instruction::StringLower { realloc } => { + // see above for this unwrap + let realloc = realloc.unwrap(); + self.needs_functions + .insert(realloc.to_string(), NeededFunction::Realloc); + + // Store the operand into a temporary... + let tmp = self.tmp(); + let val = format!("vec{}", tmp); + self.push_str(&format!("let {} = {};\n", val, operands[0])); + + // ... and then realloc space for the result in the guest module + let ptr = format!("ptr{}", tmp); + self.push_str(&format!("let {} = ", ptr)); + self.call_intrinsic(realloc, format!("(0, 0, 1, {}.len() as i32)", val)); + + // ... and then copy over the result. + let mem = self.memory_src(); + self.push_str(&format!( + "{}.store_many({}, {}.as_bytes())?;\n", + mem, ptr, val + )); + self.gen.needs_raw_mem = true; + self.needs_memory = true; + results.push(ptr); + results.push(format!("{}.len() as i32", val)); + } + + Instruction::StringLift { free } => match free { + Some(free) => { + self.needs_memory = true; + self.gen.needs_copy_slice = true; + self.needs_functions + .insert(free.to_string(), NeededFunction::Free); + let tmp = self.tmp(); + self.push_str(&format!("let ptr{} = {};\n", tmp, operands[0])); + self.push_str(&format!("let len{} = {};\n", tmp, operands[1])); + self.push_str(&format!( + " + let data{tmp} = copy_slice( + &mut caller, + memory, + ptr{tmp}, len{tmp}, 1, + )?; + ", + tmp = tmp, + )); + self.call_intrinsic( + free, + // we use normal multiplication here as copy_slice has + // already verified that multiplied size fits i32 + format!("(ptr{tmp}, len{tmp}, 1)", tmp = tmp), + ); + results.push(format!( + "String::from_utf8(data{}) + .map_err(|_| wasmtime::Trap::new(\"invalid utf-8\"))?", + tmp, + )); } None => { self.needs_borrow_checker = true; - let method = match element { - Type::Char => "slice_str", - _ => "slice", - }; let tmp = self.tmp(); self.push_str(&format!("let ptr{} = {};\n", tmp, operands[0])); self.push_str(&format!("let len{} = {};\n", tmp, operands[1])); - let slice = format!("_bc.{}(ptr{1}, len{1})?", method, tmp); + let slice = format!("_bc.slice_str(ptr{0}, len{0})?", tmp); results.push(slice); } }, diff --git a/crates/parser/src/abi.rs b/crates/parser/src/abi.rs index 9b1e99103..84c712c25 100644 --- a/crates/parser/src/abi.rs +++ b/crates/parser/src/abi.rs @@ -389,6 +389,11 @@ def_instruction! { realloc: Option<&'a str>, } : [1] => [2], + /// Same as `ListCanonLower`, but used for strings + StringLower { + realloc: Option<&'a str>, + } : [1] => [2], + /// Lowers a list where the element's layout in the native language is /// not expected to match the canonical ABI definition of interface /// types. @@ -432,6 +437,11 @@ def_instruction! { ty: TypeId, } : [2] => [1], + /// Same as `ListCanonLift`, but used for strings + StringLift { + free: Option<&'a str>, + } : [2] => [1], + /// Lifts a list which into an interface types value. /// /// This will consume two `i32` values from the stack, a pointer and a @@ -849,6 +859,10 @@ impl Interface { Type::U64 | Type::S64 => result.push(WasmType::I64), Type::Float32 => result.push(WasmType::F32), Type::Float64 => result.push(WasmType::F64), + Type::String => { + result.push(WasmType::I32); + result.push(WasmType::I32); + } Type::Id(id) => match &self.types[*id].kind { TypeDefKind::Type(t) => self.push_wasm(variant, t, result), @@ -1334,18 +1348,15 @@ impl<'a, B: Bindgen> Generator<'a, B> { self.emit(&I32FromOwnedHandle { ty }); } } + Type::String => { + let realloc = self.list_realloc(); + self.emit(&StringLower { realloc }); + } Type::Id(id) => match &self.iface.types[id].kind { TypeDefKind::Type(t) => self.lower(t), TypeDefKind::List(element) => { - // Lowering parameters calling a wasm import means - // we don't need to pass ownership, but we pass - // ownership in all other cases. - let realloc = match (self.variant, self.lift_lower) { - (AbiVariant::GuestImport, LiftLower::LowerArgsLiftResults) => None, - _ => Some("canonical_abi_realloc"), - }; - if self.is_char(element) || self.bindgen.is_list_canonical(self.iface, element) - { + let realloc = self.list_realloc(); + if self.bindgen.is_list_canonical(self.iface, element) { self.emit(&ListCanonLower { element, realloc }); } else { self.push_block(); @@ -1445,6 +1456,16 @@ impl<'a, B: Bindgen> Generator<'a, B> { } } + fn list_realloc(&self) -> Option<&'static str> { + // Lowering parameters calling a wasm import means + // we don't need to pass ownership, but we pass + // ownership in all other cases. + match (self.variant, self.lift_lower) { + (AbiVariant::GuestImport, LiftLower::LowerArgsLiftResults) => None, + _ => Some("canonical_abi_realloc"), + } + } + fn prep_return_pointer(&mut self, sig: &WasmSignature, results: &[(String, Type)]) { drop(results); // FIXME: update to the new canonical abi and use this // If a return pointer was automatically injected into this function @@ -1487,16 +1508,14 @@ impl<'a, B: Bindgen> Generator<'a, B> { self.emit(&HandleOwnedFromI32 { ty }); } } + Type::String => { + let free = self.list_free(); + self.emit(&StringLift { free }); + } Type::Id(id) => match &self.iface.types[id].kind { TypeDefKind::Type(t) => self.lift(t), TypeDefKind::List(element) => { - // Lifting the arguments of a defined import means that, if - // possible, the caller still retains ownership and we don't - // free anything. - let free = match (self.variant, self.lift_lower) { - (AbiVariant::GuestImport, LiftLower::LiftArgsLowerResults) => None, - _ => Some("canonical_abi_free"), - }; + let free = self.list_free(); if self.is_char(element) || self.bindgen.is_list_canonical(self.iface, element) { self.emit(&ListCanonLift { @@ -1595,6 +1614,16 @@ impl<'a, B: Bindgen> Generator<'a, B> { } } + fn list_free(&self) -> Option<&'static str> { + // Lifting the arguments of a defined import means that, if + // possible, the caller still retains ownership and we don't + // free anything. + match (self.variant, self.lift_lower) { + (AbiVariant::GuestImport, LiftLower::LiftArgsLowerResults) => None, + _ => Some("canonical_abi_free"), + } + } + fn write_to_memory(&mut self, ty: &Type, addr: B::Operand, offset: i32) { use Instruction::*; @@ -1609,20 +1638,11 @@ impl<'a, B: Bindgen> Generator<'a, B> { Type::U64 | Type::S64 => self.lower_and_emit(ty, addr, &I64Store { offset }), Type::Float32 => self.lower_and_emit(ty, addr, &F32Store { offset }), Type::Float64 => self.lower_and_emit(ty, addr, &F64Store { offset }), + Type::String => self.write_list_to_memory(ty, addr, offset), Type::Id(id) => match &self.iface.types[id].kind { TypeDefKind::Type(t) => self.write_to_memory(t, addr, offset), - - // After lowering the list there's two i32 values on the stack - // which we write into memory, writing the pointer into the low address - // and the length into the high address. - TypeDefKind::List(_) => { - self.lower(ty); - self.stack.push(addr.clone()); - self.emit(&I32Store { offset: offset + 4 }); - self.stack.push(addr); - self.emit(&I32Store { offset }); - } + TypeDefKind::List(_) => self.write_list_to_memory(ty, addr, offset), TypeDefKind::Record(r) if r.is_flags() => { self.lower(ty); @@ -1701,6 +1721,17 @@ impl<'a, B: Bindgen> Generator<'a, B> { } } + fn write_list_to_memory(&mut self, ty: &Type, addr: B::Operand, offset: i32) { + // After lowering the list there's two i32 values on the stack + // which we write into memory, writing the pointer into the low address + // and the length into the high address. + self.lower(ty); + self.stack.push(addr.clone()); + self.emit(&Instruction::I32Store { offset: offset + 4 }); + self.stack.push(addr); + self.emit(&Instruction::I32Store { offset }); + } + fn lower_and_emit(&mut self, ty: &Type, addr: B::Operand, instr: &Instruction) { self.lower(ty); self.stack.push(addr); @@ -1721,19 +1752,12 @@ impl<'a, B: Bindgen> Generator<'a, B> { Type::U64 | Type::S64 => self.emit_and_lift(ty, addr, &I64Load { offset }), Type::Float32 => self.emit_and_lift(ty, addr, &F32Load { offset }), Type::Float64 => self.emit_and_lift(ty, addr, &F64Load { offset }), + Type::String => self.read_list_from_memory(ty, addr, offset), Type::Id(id) => match &self.iface.types[id].kind { TypeDefKind::Type(t) => self.read_from_memory(t, addr, offset), - // Read the pointer/len and then perform the standard lifting - // proceses. - TypeDefKind::List(_) => { - self.stack.push(addr.clone()); - self.emit(&I32Load { offset }); - self.stack.push(addr); - self.emit(&I32Load { offset: offset + 4 }); - self.lift(ty); - } + TypeDefKind::List(_) => self.read_list_from_memory(ty, addr, offset), TypeDefKind::Record(r) if r.is_flags() => { match self.iface.flags_repr(r) { @@ -1803,6 +1827,16 @@ impl<'a, B: Bindgen> Generator<'a, B> { } } + fn read_list_from_memory(&mut self, ty: &Type, addr: B::Operand, offset: i32) { + // Read the pointer/len and then perform the standard lifting + // proceses. + self.stack.push(addr.clone()); + self.emit(&Instruction::I32Load { offset }); + self.stack.push(addr); + self.emit(&Instruction::I32Load { offset: offset + 4 }); + self.lift(ty); + } + fn emit_and_lift(&mut self, ty: &Type, addr: B::Operand, instr: &Instruction) { self.stack.push(addr); self.emit(instr); diff --git a/crates/parser/src/ast.rs b/crates/parser/src/ast.rs index 88aba7720..9c2b498e7 100644 --- a/crates/parser/src/ast.rs +++ b/crates/parser/src/ast.rs @@ -84,6 +84,7 @@ enum Type<'a> { Float32, Float64, Char, + String, Handle(Id<'a>), Name(Id<'a>), List(Box>), @@ -524,7 +525,7 @@ impl<'a> Type<'a> { } Some((_span, Token::Bool)) => Ok(Type::bool()), - Some((_span, Token::String_)) => Ok(Type::List(Box::new(Type::Char))), + Some((_span, Token::String_)) => Ok(Type::String), // list Some((_span, Token::List)) => { diff --git a/crates/parser/src/ast/resolve.rs b/crates/parser/src/ast/resolve.rs index 312a29a79..ddca45296 100644 --- a/crates/parser/src/ast/resolve.rs +++ b/crates/parser/src/ast/resolve.rs @@ -319,6 +319,7 @@ impl Resolver { super::Type::Float32 => TypeDefKind::Type(Type::Float32), super::Type::Float64 => TypeDefKind::Type(Type::Float64), super::Type::Char => TypeDefKind::Type(Type::Char), + super::Type::String => TypeDefKind::Type(Type::String), super::Type::Handle(resource) => { let id = match self.resource_lookup.get(&*resource.name) { Some(id) => *id, diff --git a/crates/parser/src/lib.rs b/crates/parser/src/lib.rs index b2b2a33bc..24f9c51ec 100644 --- a/crates/parser/src/lib.rs +++ b/crates/parser/src/lib.rs @@ -64,6 +64,7 @@ pub enum Type { Float32, Float64, Char, + String, Handle(ResourceId), Id(TypeId), } @@ -433,7 +434,7 @@ impl Interface { | Type::Float32 | Type::Float64 => true, - Type::Char | Type::Handle(_) => false, + Type::Char | Type::Handle(_) | Type::String => false, Type::Id(id) => match &self.types[*id].kind { TypeDefKind::List(_) | TypeDefKind::Variant(_) => false, diff --git a/crates/parser/src/sizealign.rs b/crates/parser/src/sizealign.rs index 7b242321d..0edc77273 100644 --- a/crates/parser/src/sizealign.rs +++ b/crates/parser/src/sizealign.rs @@ -61,7 +61,7 @@ impl SizeAlign { Type::U8 | Type::S8 => 1, Type::U16 | Type::S16 => 2, Type::U32 | Type::S32 | Type::Float32 | Type::Char | Type::Handle(_) => 4, - Type::U64 | Type::S64 | Type::Float64 => 8, + Type::U64 | Type::S64 | Type::Float64 | Type::String => 8, Type::Id(id) => self.map[id.index()].0, } } @@ -70,7 +70,9 @@ impl SizeAlign { match ty { Type::U8 | Type::S8 => 1, Type::U16 | Type::S16 => 2, - Type::U32 | Type::S32 | Type::Float32 | Type::Char | Type::Handle(_) => 4, + Type::U32 | Type::S32 | Type::Float32 | Type::Char | Type::Handle(_) | Type::String => { + 4 + } Type::U64 | Type::S64 | Type::Float64 => 8, Type::Id(id) => self.map[id.index()].1, } diff --git a/crates/parser/tests/all.rs b/crates/parser/tests/all.rs index 6a13d8877..56c9d3548 100644 --- a/crates/parser/tests/all.rs +++ b/crates/parser/tests/all.rs @@ -293,6 +293,7 @@ fn to_json(i: &Interface) -> String { Type::Float32 => format!("float32"), Type::Float64 => format!("float64"), Type::Char => format!("char"), + Type::String => format!("string"), Type::Handle(resource) => format!("handle-{}", resource.index()), Type::Id(id) => format!("type-{}", id.index()), } diff --git a/crates/parser/tests/ui/type-then-eof.wit.result b/crates/parser/tests/ui/type-then-eof.wit.result index 53d37ede0..625b858e2 100644 --- a/crates/parser/tests/ui/type-then-eof.wit.result +++ b/crates/parser/tests/ui/type-then-eof.wit.result @@ -1,16 +1,10 @@ { - "types": [ - { - "idx": 0, - "list": "char" - } - ], "functions": [ { "name": "foo", "params": [], "results": [ - "type-0" + "string" ] } ] diff --git a/crates/parser/tests/ui/types.wit.result b/crates/parser/tests/ui/types.wit.result index d9a0993c5..2b39531c7 100644 --- a/crates/parser/tests/ui/types.wit.result +++ b/crates/parser/tests/ui/types.wit.result @@ -72,7 +72,7 @@ { "idx": 13, "name": "t13", - "list": "char" + "primitive": "string" }, { "idx": 14, @@ -518,7 +518,7 @@ { "idx": 45, "name": "t44", - "list": "char" + "primitive": "string" }, { "idx": 46, diff --git a/crates/parser/tests/ui/wasi-http.wit.result b/crates/parser/tests/ui/wasi-http.wit.result index 61c8910eb..df69b2a9d 100644 --- a/crates/parser/tests/ui/wasi-http.wit.result +++ b/crates/parser/tests/ui/wasi-http.wit.result @@ -64,10 +64,6 @@ }, { "idx": 6, - "list": "char" - }, - { - "idx": 7, "variant": { "cases": [ [ @@ -76,17 +72,17 @@ ], [ "some", - "type-6" + "string" ] ] } }, { - "idx": 8, + "idx": 7, "list": "u8" }, { - "idx": 9, + "idx": 8, "variant": { "cases": [ [ @@ -124,7 +120,7 @@ "handle-0" ], "results": [ - "type-6" + "string" ] }, { @@ -176,37 +172,37 @@ "name": "headers::get", "params": [ "handle-2", - "type-6" + "string" ], "results": [ - "type-7" + "type-6" ] }, { "name": "body::read", "params": [ "handle-3", - "type-8" + "type-7" ], "results": [ - "type-9" + "type-8" ] }, { "name": "body::write", "params": [ "handle-3", - "type-8" + "type-7" ], "results": [ - "type-9" + "type-8" ] }, { "name": "maybe-number", "params": [], "results": [ - "type-9" + "type-8" ] } ], diff --git a/crates/test-helpers/src/lib.rs b/crates/test-helpers/src/lib.rs index 106f44132..6c0eef044 100644 --- a/crates/test-helpers/src/lib.rs +++ b/crates/test-helpers/src/lib.rs @@ -155,6 +155,7 @@ pub fn codegen_rust_wasm_export(input: TokenStream) -> TokenStream { Type::Float32 => quote::quote! { f32 }, Type::Float64 => quote::quote! { f64 }, Type::Char => quote::quote! { char }, + Type::String => quote::quote! { String }, Type::Handle(resource) => { let name = quote::format_ident!("{}", iface.resources[resource].name.to_camel_case()); @@ -178,12 +179,8 @@ pub fn codegen_rust_wasm_export(input: TokenStream) -> TokenStream { match &ty.kind { TypeDefKind::Type(t) => quote_ty(param, iface, t), TypeDefKind::List(t) => { - if *t == Type::Char { - quote::quote! { String } - } else { - let t = quote_ty(param, iface, t); - quote::quote! { Vec<#t> } - } + let t = quote_ty(param, iface, t); + quote::quote! { Vec<#t> } } TypeDefKind::Record(r) => { let fields = r.fields.iter().map(|f| quote_ty(param, iface, &f.ty)); diff --git a/crates/wasmlink/src/adapter/call.rs b/crates/wasmlink/src/adapter/call.rs index c9058bc3d..41096407e 100644 --- a/crates/wasmlink/src/adapter/call.rs +++ b/crates/wasmlink/src/adapter/call.rs @@ -183,17 +183,6 @@ impl Operand<'_> { } } -fn is_char(interface: &WitInterface, ty: &Type) -> bool { - match ty { - Type::Char => true, - Type::Id(id) => match &interface.types[*id].kind { - TypeDefKind::Type(t) => is_char(interface, t), - _ => false, - }, - _ => false, - } -} - #[derive(Debug, Clone, Copy)] enum PushMode { Params, @@ -452,7 +441,7 @@ impl<'a> CallAdapter<'a> { let len = params.next().unwrap(); let mut element_operands = Vec::new(); - if !interface.all_bits_valid(element) && !is_char(interface, element) { + if !interface.all_bits_valid(element) { Self::push_element_operands( interface, sizes, @@ -556,6 +545,26 @@ impl<'a> CallAdapter<'a> { } } }, + Type::String => { + let addr = params.next().unwrap(); + let len = params.next().unwrap(); + + // Every list copied needs a destination local (and a source for + // retptr) + *locals_count += match mode { + PushMode::Params => 1, + PushMode::RetPtr => 2, + PushMode::Return => unreachable!(), + }; + + operands.push(Operand::List { + addr: mode.create_value_ref(addr), + len: mode.create_value_ref(len), + element_size: 1, // UTF-8 + element_alignment: 1, + operands: Vec::new(), + }); + } Type::Handle(id) => { let addr = params.next().unwrap(); @@ -598,7 +607,7 @@ impl<'a> CallAdapter<'a> { ), TypeDefKind::List(element) => { let mut element_operands = Vec::new(); - if !interface.all_bits_valid(element) && !is_char(interface, element) { + if !interface.all_bits_valid(element) { Self::push_element_operands( interface, sizes, @@ -680,6 +689,18 @@ impl<'a> CallAdapter<'a> { } } }, + Type::String => { + // Every list copied needs a source and destination local + *locals_count += 2; + + operands.push(Operand::List { + addr: ValueRef::ElementOffset(offset), + len: ValueRef::ElementOffset(offset + 4), + element_size: 1, // UTF-8 + element_alignment: 1, + operands: Vec::new(), + }); + } Type::Handle(id) => { // Params need to be cloned, so add a local *locals_count += match mode { diff --git a/crates/wasmlink/src/module.rs b/crates/wasmlink/src/module.rs index a48bdaac0..cfa806502 100644 --- a/crates/wasmlink/src/module.rs +++ b/crates/wasmlink/src/module.rs @@ -44,6 +44,7 @@ fn has_list(interface: &WitInterface, ty: &WitType) -> bool { use wit_parser::{Type, TypeDefKind}; match ty { + Type::String => true, Type::Id(id) => match &interface.types[*id].kind { TypeDefKind::List(_) => true, TypeDefKind::Type(t) => has_list(interface, t), diff --git a/crates/wit-component/src/decoding.rs b/crates/wit-component/src/decoding.rs index 45530fa67..e5448a62f 100644 --- a/crates/wit-component/src/decoding.rs +++ b/crates/wit-component/src/decoding.rs @@ -178,7 +178,6 @@ pub struct InterfaceDecoder<'a> { type_map: HashMap, name_map: HashMap, bool_ty: Option, - string_ty: Option, } impl<'a> InterfaceDecoder<'a> { @@ -190,7 +189,6 @@ impl<'a> InterfaceDecoder<'a> { name_map: HashMap::new(), type_map: HashMap::new(), bool_ty: None, - string_ty: None, } } @@ -344,7 +342,7 @@ impl<'a> InterfaceDecoder<'a> { PrimitiveInterfaceType::Float32 => Type::Float32, PrimitiveInterfaceType::Float64 => Type::Float64, PrimitiveInterfaceType::Char => Type::Char, - PrimitiveInterfaceType::String => self.string(), + PrimitiveInterfaceType::String => Type::String, }) } @@ -630,11 +628,4 @@ impl<'a> InterfaceDecoder<'a> { } Type::Id(self.bool_ty.unwrap()) } - - fn string(&mut self) -> Type { - if self.string_ty.is_none() { - self.string_ty = Some(self.alloc_type(None, TypeDefKind::List(Type::Char))); - } - Type::Id(self.string_ty.unwrap()) - } } diff --git a/crates/wit-component/src/encoding.rs b/crates/wit-component/src/encoding.rs index 95301d94b..2331815da 100644 --- a/crates/wit-component/src/encoding.rs +++ b/crates/wit-component/src/encoding.rs @@ -426,6 +426,7 @@ impl<'a> TypeEncoder<'a> { Type::Float32 => InterfaceTypeRef::Primitive(PrimitiveInterfaceType::Float32), Type::Float64 => InterfaceTypeRef::Primitive(PrimitiveInterfaceType::Float64), Type::Char => InterfaceTypeRef::Primitive(PrimitiveInterfaceType::Char), + Type::String => InterfaceTypeRef::Primitive(PrimitiveInterfaceType::String), Type::Id(id) => { let ty = &interface.types[*id]; let key = TypeDefKey::borrow(interface, &interface.types[*id]); @@ -435,9 +436,6 @@ impl<'a> TypeEncoder<'a> { let mut encoded = match &ty.kind { TypeDefKind::Record(r) => self.encode_record(interface, instance, r)?, TypeDefKind::Variant(v) => self.encode_variant(interface, instance, v)?, - TypeDefKind::List(Type::Char) => { - InterfaceTypeRef::Primitive(PrimitiveInterfaceType::String) - } TypeDefKind::List(ty) => { let ty = self.encode_type(interface, instance, ty)?; let index = self.types.len(); @@ -664,10 +662,6 @@ impl RequiredOptions { TypeDefKind::Variant(v) => { Self::for_types(interface, v.cases.iter().filter_map(|c| c.ty.as_ref())) } - TypeDefKind::List(Type::Char) => { - // Strings need the encoding option - Self::Encoding - } TypeDefKind::List(t) => { // Lists need at least the `into` option, but may require // the encoding option if there's a string somewhere in the @@ -676,6 +670,7 @@ impl RequiredOptions { } TypeDefKind::Type(t) => Self::for_type(interface, t), }, + Type::String => Self::Encoding, _ => Self::None, } } diff --git a/crates/wit-component/src/printing.rs b/crates/wit-component/src/printing.rs index d0ae142b9..baeffd6ff 100644 --- a/crates/wit-component/src/printing.rs +++ b/crates/wit-component/src/printing.rs @@ -57,6 +57,7 @@ impl InterfacePrinter { Type::Float32 => self.output.push_str("float32"), Type::Float64 => self.output.push_str("float64"), Type::Char => self.output.push_str("char"), + Type::String => self.output.push_str("string"), Type::Id(id) => { let ty = &interface.types[*id]; @@ -73,13 +74,9 @@ impl InterfacePrinter { self.print_variant_type(interface, v)?; } TypeDefKind::List(ty) => { - if let Type::Char = ty { - self.output.push_str("string"); - } else { - self.output.push_str("list<"); - self.print_type_name(interface, ty)?; - self.output.push('>'); - } + self.output.push_str("list<"); + self.print_type_name(interface, ty)?; + self.output.push('>'); } TypeDefKind::Type(ty) => self.print_type_name(interface, ty)?, } @@ -151,7 +148,8 @@ impl InterfacePrinter { | Type::S64 | Type::Float32 | Type::Float64 - | Type::Char => return Ok(()), + | Type::Char + | Type::String => return Ok(()), Type::Id(id) => { if !self.declared.insert(*id) { From 6098e708ea8d81a08c398262746b02c858b9a036 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 21 Apr 2022 09:37:58 -0500 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Peter Huene --- crates/gen-js/src/lib.rs | 2 +- crates/gen-rust-wasm/src/lib.rs | 2 +- crates/gen-wasmtime-py/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/gen-js/src/lib.rs b/crates/gen-js/src/lib.rs index 4ee765796..a5bbba401 100644 --- a/crates/gen-js/src/lib.rs +++ b/crates/gen-js/src/lib.rs @@ -1672,7 +1672,7 @@ impl Bindgen for FunctionBindgen<'_> { } } Instruction::StringLower { realloc } => { - // Lowering only happens when we're passing lists into wasm, + // Lowering only happens when we're passing strings into wasm, // which forces us to always allocate, so this should always be // `Some`. let realloc = realloc.unwrap(); diff --git a/crates/gen-rust-wasm/src/lib.rs b/crates/gen-rust-wasm/src/lib.rs index 55a0c3c64..033c7c57d 100644 --- a/crates/gen-rust-wasm/src/lib.rs +++ b/crates/gen-rust-wasm/src/lib.rs @@ -1056,7 +1056,7 @@ impl Bindgen for FunctionBindgen<'_> { } Instruction::StringLift { free, .. } => { - // This only happens when we're receiving a list from the + // This only happens when we're receiving a string from the // outside world, so `free` should always be `Some`. assert!(free.is_some()); let tmp = self.tmp(); diff --git a/crates/gen-wasmtime-py/src/lib.rs b/crates/gen-wasmtime-py/src/lib.rs index ace6c1015..5dc61c2ad 100644 --- a/crates/gen-wasmtime-py/src/lib.rs +++ b/crates/gen-wasmtime-py/src/lib.rs @@ -1770,7 +1770,7 @@ impl Bindgen for FunctionBindgen<'_> { } } Instruction::StringLower { realloc } => { - // Lowering only happens when we're passing lists into wasm, + // Lowering only happens when we're passing strings into wasm, // which forces us to always allocate, so this should always be // `Some`. let realloc = realloc.unwrap();