diff --git a/example/js/lib/api/FixedDecimalFormatterOptions.mjs b/example/js/lib/api/FixedDecimalFormatterOptions.mjs index 517eae22f..a26479f7a 100644 --- a/example/js/lib/api/FixedDecimalFormatterOptions.mjs +++ b/example/js/lib/api/FixedDecimalFormatterOptions.mjs @@ -33,11 +33,15 @@ export class FixedDecimalFormatterOptions { // Return this struct in FFI function friendly format. // Returns an array that can be expanded with spread syntax (...) + // JS structs need to be generated with or without padding depending on whether they are being passed as aggregates or splatted out into fields. + // Most of the time this is known beforehand: large structs (>2 scalar fields) always get padding, and structs passed directly in parameters omit padding + // if they are small. However small structs within large structs also get padding, and we signal that by setting forcePadding. _intoFFI( functionCleanupArena, - appendArrayMap + appendArrayMap, + forcePadding ) { - return [this.#groupingStrategy.ffiValue, this.#someOtherConfig] + return [this.#groupingStrategy.ffiValue, this.#someOtherConfig, ...diplomatRuntime.maybePaddingFields(forcePadding, 3 /* x i8 */)] } // This struct contains borrowed fields, so this takes in a list of diff --git a/example/js/lib/api/diplomat-runtime.mjs b/example/js/lib/api/diplomat-runtime.mjs index c77c87515..95b1c719d 100644 --- a/example/js/lib/api/diplomat-runtime.mjs +++ b/example/js/lib/api/diplomat-runtime.mjs @@ -59,6 +59,18 @@ export function enumDiscriminant(wasm, ptr) { return (new Int32Array(wasm.memory.buffer, ptr, 1))[0] } +/** + * Return an array of paddingCount zeroes to be spread into a function call + * if needsPaddingFields is true, else empty + */ +export function maybePaddingFields(needsPaddingFields, paddingCount) { + if (needsPaddingFields) { + return Array(paddingCount).fill(0);; + } else { + return []; + } +} + /** * A wrapper around a slice of WASM memory that can be freed manually or * automatically by the garbage collector. diff --git a/feature_tests/js/api/BigStructWithStuff.d.ts b/feature_tests/js/api/BigStructWithStuff.d.ts new file mode 100644 index 000000000..e66cc2012 --- /dev/null +++ b/feature_tests/js/api/BigStructWithStuff.d.ts @@ -0,0 +1,27 @@ +// generated by diplomat-tool +import type { ScalarPairWithPadding } from "./ScalarPairWithPadding" +import type { pointer, codepoint } from "./diplomat-runtime.d.ts"; + + +/** Testing JS-specific layout/padding behavior +*/ +export class BigStructWithStuff { + + get first() : number; + set first(value: number); + + get second() : number; + set second(value: number); + + get third() : number; + set third(value: number); + + get fourth() : ScalarPairWithPadding; + set fourth(value: ScalarPairWithPadding); + + get fifth() : number; + set fifth(value: number); + constructor(first: number, second: number, third: number, fourth: ScalarPairWithPadding, fifth: number); + + assertValue(extraVal: number): void; +} \ No newline at end of file diff --git a/feature_tests/js/api/BigStructWithStuff.mjs b/feature_tests/js/api/BigStructWithStuff.mjs new file mode 100644 index 000000000..75d347acd --- /dev/null +++ b/feature_tests/js/api/BigStructWithStuff.mjs @@ -0,0 +1,101 @@ +// generated by diplomat-tool +import { ScalarPairWithPadding } from "./ScalarPairWithPadding.mjs" +import wasm from "./diplomat-wasm.mjs"; +import * as diplomatRuntime from "./diplomat-runtime.mjs"; + + +/** Testing JS-specific layout/padding behavior +*/ +export class BigStructWithStuff { + + #first; + get first() { + return this.#first; + } + set first(value) { + this.#first = value; + } + + #second; + get second() { + return this.#second; + } + set second(value) { + this.#second = value; + } + + #third; + get third() { + return this.#third; + } + set third(value) { + this.#third = value; + } + + #fourth; + get fourth() { + return this.#fourth; + } + set fourth(value) { + this.#fourth = value; + } + + #fifth; + get fifth() { + return this.#fifth; + } + set fifth(value) { + this.#fifth = value; + } + constructor() { + if (arguments.length > 0 && arguments[0] === diplomatRuntime.internalConstructor) { + this.#fromFFI(...Array.prototype.slice.call(arguments, 1)); + } else { + + this.#first = arguments[0]; + this.#second = arguments[1]; + this.#third = arguments[2]; + this.#fourth = arguments[3]; + this.#fifth = arguments[4]; + } + } + + // Return this struct in FFI function friendly format. + // Returns an array that can be expanded with spread syntax (...) + + _intoFFI( + functionCleanupArena, + appendArrayMap + ) { + return [this.#first, /* [1 x i8] padding */ 0 /* end padding */, this.#second, this.#third, /* [1 x i16] padding */ 0 /* end padding */, ...this.#fourth._intoFFI(functionCleanupArena, {}, true), this.#fifth, /* [3 x i8] padding */ 0, 0, 0 /* end padding */] + } + + // This struct contains borrowed fields, so this takes in a list of + // "edges" corresponding to where each lifetime's data may have been borrowed from + // and passes it down to individual fields containing the borrow. + // This method does not attempt to handle any dependencies between lifetimes, the caller + // should handle this when constructing edge arrays. + #fromFFI(ptr) { + const firstDeref = (new Uint8Array(wasm.memory.buffer, ptr, 1))[0]; + this.#first = firstDeref; + const secondDeref = (new Uint16Array(wasm.memory.buffer, ptr + 2, 1))[0]; + this.#second = secondDeref; + const thirdDeref = (new Uint16Array(wasm.memory.buffer, ptr + 4, 1))[0]; + this.#third = thirdDeref; + const fourthDeref = ptr + 8; + this.#fourth = new ScalarPairWithPadding(diplomatRuntime.internalConstructor, fourthDeref); + const fifthDeref = (new Uint8Array(wasm.memory.buffer, ptr + 16, 1))[0]; + this.#fifth = fifthDeref; + } + + assertValue(extraVal) { + let functionCleanupArena = new diplomatRuntime.CleanupArena(); + wasm.BigStructWithStuff_assert_value(...this._intoFFI(), extraVal); + + try {} + + finally { + functionCleanupArena.free(); + } + } +} \ No newline at end of file diff --git a/feature_tests/js/api/ImportedStruct.mjs b/feature_tests/js/api/ImportedStruct.mjs index 1afd98bc2..48faf6eba 100644 --- a/feature_tests/js/api/ImportedStruct.mjs +++ b/feature_tests/js/api/ImportedStruct.mjs @@ -33,11 +33,15 @@ export class ImportedStruct { // Return this struct in FFI function friendly format. // Returns an array that can be expanded with spread syntax (...) + // JS structs need to be generated with or without padding depending on whether they are being passed as aggregates or splatted out into fields. + // Most of the time this is known beforehand: large structs (>2 scalar fields) always get padding, and structs passed directly in parameters omit padding + // if they are small. However small structs within large structs also get padding, and we signal that by setting forcePadding. _intoFFI( functionCleanupArena, - appendArrayMap + appendArrayMap, + forcePadding ) { - return [this.#foo.ffiValue, this.#count] + return [this.#foo.ffiValue, this.#count, ...diplomatRuntime.maybePaddingFields(forcePadding, 3 /* x i8 */)] } // This struct contains borrowed fields, so this takes in a list of diff --git a/feature_tests/js/api/MyStruct.mjs b/feature_tests/js/api/MyStruct.mjs index 103f2def0..1c2eb813d 100644 --- a/feature_tests/js/api/MyStruct.mjs +++ b/feature_tests/js/api/MyStruct.mjs @@ -83,7 +83,7 @@ export class MyStruct { functionCleanupArena, appendArrayMap ) { - return [this.#a, this.#b, this.#c,/* Padding for c */ 0, 0, 0, 0, 0 /* End Padding */, this.#d, this.#e, this.#f, this.#g.ffiValue] + return [this.#a, this.#b, this.#c, /* [5 x i8] padding */ 0, 0, 0, 0, 0 /* end padding */, this.#d, this.#e, this.#f, this.#g.ffiValue, /* [1 x i32] padding */ 0 /* end padding */] } // This struct contains borrowed fields, so this takes in a list of diff --git a/feature_tests/js/api/ScalarPairWithPadding.d.ts b/feature_tests/js/api/ScalarPairWithPadding.d.ts new file mode 100644 index 000000000..414c5f1ab --- /dev/null +++ b/feature_tests/js/api/ScalarPairWithPadding.d.ts @@ -0,0 +1,17 @@ +// generated by diplomat-tool +import type { pointer, codepoint } from "./diplomat-runtime.d.ts"; + + +/** Testing JS-specific layout/padding behavior +*/ +export class ScalarPairWithPadding { + + get first() : number; + set first(value: number); + + get second() : number; + set second(value: number); + constructor(first: number, second: number); + + assertValue(): void; +} \ No newline at end of file diff --git a/feature_tests/js/api/ScalarPairWithPadding.mjs b/feature_tests/js/api/ScalarPairWithPadding.mjs new file mode 100644 index 000000000..52726a5f6 --- /dev/null +++ b/feature_tests/js/api/ScalarPairWithPadding.mjs @@ -0,0 +1,71 @@ +// generated by diplomat-tool +import wasm from "./diplomat-wasm.mjs"; +import * as diplomatRuntime from "./diplomat-runtime.mjs"; + + +/** Testing JS-specific layout/padding behavior +*/ +export class ScalarPairWithPadding { + + #first; + get first() { + return this.#first; + } + set first(value) { + this.#first = value; + } + + #second; + get second() { + return this.#second; + } + set second(value) { + this.#second = value; + } + constructor() { + if (arguments.length > 0 && arguments[0] === diplomatRuntime.internalConstructor) { + this.#fromFFI(...Array.prototype.slice.call(arguments, 1)); + } else { + + this.#first = arguments[0]; + this.#second = arguments[1]; + } + } + + // Return this struct in FFI function friendly format. + // Returns an array that can be expanded with spread syntax (...) + + // JS structs need to be generated with or without padding depending on whether they are being passed as aggregates or splatted out into fields. + // Most of the time this is known beforehand: large structs (>2 scalar fields) always get padding, and structs passed directly in parameters omit padding + // if they are small. However small structs within large structs also get padding, and we signal that by setting forcePadding. + _intoFFI( + functionCleanupArena, + appendArrayMap, + forcePadding + ) { + return [this.#first, ...diplomatRuntime.maybePaddingFields(forcePadding, 3 /* x i8 */), this.#second] + } + + // This struct contains borrowed fields, so this takes in a list of + // "edges" corresponding to where each lifetime's data may have been borrowed from + // and passes it down to individual fields containing the borrow. + // This method does not attempt to handle any dependencies between lifetimes, the caller + // should handle this when constructing edge arrays. + #fromFFI(ptr) { + const firstDeref = (new Uint8Array(wasm.memory.buffer, ptr, 1))[0]; + this.#first = firstDeref; + const secondDeref = (new Uint32Array(wasm.memory.buffer, ptr + 4, 1))[0]; + this.#second = secondDeref; + } + + assertValue() { + let functionCleanupArena = new diplomatRuntime.CleanupArena(); + wasm.ScalarPairWithPadding_assert_value(...this._intoFFI()); + + try {} + + finally { + functionCleanupArena.free(); + } + } +} \ No newline at end of file diff --git a/feature_tests/js/api/diplomat-runtime.mjs b/feature_tests/js/api/diplomat-runtime.mjs index c77c87515..95b1c719d 100644 --- a/feature_tests/js/api/diplomat-runtime.mjs +++ b/feature_tests/js/api/diplomat-runtime.mjs @@ -59,6 +59,18 @@ export function enumDiscriminant(wasm, ptr) { return (new Int32Array(wasm.memory.buffer, ptr, 1))[0] } +/** + * Return an array of paddingCount zeroes to be spread into a function call + * if needsPaddingFields is true, else empty + */ +export function maybePaddingFields(needsPaddingFields, paddingCount) { + if (needsPaddingFields) { + return Array(paddingCount).fill(0);; + } else { + return []; + } +} + /** * A wrapper around a slice of WASM memory that can be freed manually or * automatically by the garbage collector. diff --git a/feature_tests/js/api/index.d.ts b/feature_tests/js/api/index.d.ts index 957b8a62f..6105033d2 100644 --- a/feature_tests/js/api/index.d.ts +++ b/feature_tests/js/api/index.d.ts @@ -14,6 +14,8 @@ export { NestedBorrowedFields } from "./NestedBorrowedFields" export { ErrorStruct } from "./ErrorStruct" +export { BigStructWithStuff } from "./BigStructWithStuff" + export { CyclicStructA } from "./CyclicStructA" export { CyclicStructB } from "./CyclicStructB" @@ -22,6 +24,8 @@ export { MyStruct } from "./MyStruct" export { MyZst } from "./MyZst" +export { ScalarPairWithPadding } from "./ScalarPairWithPadding" + export { OptionStruct } from "./OptionStruct" export { AttrOpaque1Renamed } from "./AttrOpaque1Renamed" diff --git a/feature_tests/js/api/index.mjs b/feature_tests/js/api/index.mjs index ce0584ccc..c0b968bde 100644 --- a/feature_tests/js/api/index.mjs +++ b/feature_tests/js/api/index.mjs @@ -12,6 +12,8 @@ export { NestedBorrowedFields } from "./NestedBorrowedFields.mjs" export { ErrorStruct } from "./ErrorStruct.mjs" +export { BigStructWithStuff } from "./BigStructWithStuff.mjs" + export { CyclicStructA } from "./CyclicStructA.mjs" export { CyclicStructB } from "./CyclicStructB.mjs" @@ -20,6 +22,8 @@ export { MyStruct } from "./MyStruct.mjs" export { MyZst } from "./MyZst.mjs" +export { ScalarPairWithPadding } from "./ScalarPairWithPadding.mjs" + export { OptionStruct } from "./OptionStruct.mjs" export { AttrOpaque1Renamed } from "./AttrOpaque1Renamed.mjs" diff --git a/feature_tests/js/test/struct.mjs b/feature_tests/js/test/struct.mjs index a3c3a7669..86fbcd30e 100644 --- a/feature_tests/js/test/struct.mjs +++ b/feature_tests/js/test/struct.mjs @@ -1,5 +1,5 @@ import test from 'ava'; -import { MyEnum, MyStruct } from "diplomat-wasm-js-feature-tests"; +import { MyEnum, MyStruct, ScalarPairWithPadding, BigStructWithStuff } from "diplomat-wasm-js-feature-tests"; test("Verify invariants of struct", t => { const s = MyStruct.new_("hello"); @@ -16,4 +16,16 @@ test("Verify invariants of struct", t => { test("Test struct creation", t => { const s = new MyStruct(17, true, 209, 1234n, 5991, '餐'.codePointAt(0), MyEnum.B); t.is(s.intoA(), 17); -}); \ No newline at end of file +}); + +test("Test struct layout: scalar pair layout", t => { + const s = new ScalarPairWithPadding(122, 414); + s.assertValue(); + t.is(true, true); // Ava doesn't like tests without assertions +}); + +test("Test struct layout: complex struct with multiple padding types and contained scalar pair", t => { + const s = new BigStructWithStuff(101, 505, 9345, new ScalarPairWithPadding(122, 414), 99); + s.assertValue(853); + t.is(true, true); // Ava doesn't like tests without assertions +}); diff --git a/feature_tests/src/structs.rs b/feature_tests/src/structs.rs index 8aaf17537..8e4979083 100644 --- a/feature_tests/src/structs.rs +++ b/feature_tests/src/structs.rs @@ -218,6 +218,44 @@ pub mod ffi { Default::default() } } + + /// Testing JS-specific layout/padding behavior + #[diplomat::attr(not(js), disable)] + pub struct ScalarPairWithPadding { + pub first: u8, + // Padding: [3 x u8] + pub second: u32, + } + + impl ScalarPairWithPadding { + pub fn assert_value(self) { + assert_eq!(self.first, 122); + assert_eq!(self.second, 414); + } + } + + /// Testing JS-specific layout/padding behavior + #[diplomat::attr(not(js), disable)] + pub struct BigStructWithStuff { + pub first: u8, + // Padding: [1 x u8] + pub second: u16, + pub third: u16, + // Padding: [1 x u16] + pub fourth: ScalarPairWithPadding, + pub fifth: u8, + } + + impl BigStructWithStuff { + pub fn assert_value(self, extra_val: u16) { + assert_eq!(self.first, 101); + assert_eq!(self.second, 505); + assert_eq!(self.third, 9345); + self.fourth.assert_value(); + assert_eq!(self.fifth, 99); + assert_eq!(extra_val, 853); + } + } } #[allow(unused)] diff --git a/tool/src/js/layout.rs b/tool/src/js/layout.rs index 9918fbdea..fb8365aad 100644 --- a/tool/src/js/layout.rs +++ b/tool/src/js/layout.rs @@ -11,38 +11,98 @@ use diplomat_core::hir::{ // TODO(#58): support non-32-bit platforms use u32 as usize_target; +pub struct StructFieldLayout { + /// The offset of this field in the struct + pub offset: usize, + /// The number of padding fields needed after this field + pub padding_count: usize, + /// The size of the padding field + pub padding_size: usize, + /// The number of scalar (integer primitive) fields in this field, transitively. Does not count padding fields. + pub scalar_count: usize, +} + +pub struct StructFieldsInfo { + /// Layout details for individual fields + pub fields: Vec, + /// The layout of the struct overall + pub struct_layout: Layout, + /// The number of scalar (integer primitive) fields in this struct, transitively. Does not count padding fields. + pub scalar_count: usize, +} /// Given a struct, calculate where each of its fields is in memory. /// /// ([`Vec`], _) is the list of offsets that each field is at in memory. /// (_, [`Layout`]) represents the [`Layout`] of our structure, in full. -pub fn struct_offsets_size_max_align<'a, P: hir::TyPosition + 'a>( +pub fn struct_field_info<'a, P: hir::TyPosition + 'a>( types: impl Iterator>, tcx: &'a TypeContext, -) -> (Vec, Layout) { +) -> StructFieldsInfo { let mut max_align = 0; let mut next_offset = 0; - let mut offsets = vec![]; + let mut fields: Vec = vec![]; + let mut scalar_count = 0; let types = types.collect::>(); if types.is_empty() { - return (vec![], unit_size_alignment()); + return StructFieldsInfo { + fields: vec![], + struct_layout: unit_size_alignment(), + scalar_count: 0, + }; } + let mut prev_align = 1; for typ in types { - let size_align = type_size_alignment(typ, tcx); + let (size_align, field_scalars) = type_size_alignment_and_scalar_count(typ, tcx); + scalar_count += field_scalars; let size = size_align.size(); let align = size_align.align(); max_align = max(max_align, align); let padding = (align - (next_offset % align)) % align; next_offset += padding; - offsets.push(next_offset); + + // Tack padding on to previous field + // + // We don't know until we see the next field if padding is needed, but padding + // belongs to the field before it, not the field after it (since there can be padding at the end, but never + // padding at the beginning) + if padding != 0 { + assert!(padding % prev_align == 0, "Needed padding {padding} must be a perfect multiple of the previous field alignment {prev_align}"); + let fields_len = fields.len(); + assert!( + fields_len != 0, + "Padding can only be found after first field!" + ); + + fields[fields_len - 1].padding_count = padding / prev_align; + fields[fields_len - 1].padding_size = prev_align; + } + + fields.push(StructFieldLayout { + offset: next_offset, + padding_count: 0, + padding_size: 1, + scalar_count: field_scalars, + }); + prev_align = align; next_offset += size; } - ( - offsets, - Layout::from_size_align(next_offset, max_align).unwrap(), - ) + + // Structs can have padding at the end, too + if next_offset % max_align != 0 { + let fields_len = fields.len(); + let padding = (max_align - (next_offset % max_align)) % max_align; + fields[fields_len - 1].padding_count = padding / prev_align; + fields[fields_len - 1].padding_size = prev_align; + } + + StructFieldsInfo { + fields, + struct_layout: Layout::from_size_align(next_offset, max_align).unwrap(), + scalar_count, + } } pub fn opaque_size_alignment() -> Layout { @@ -57,24 +117,32 @@ pub fn unit_size_alignment() -> Layout { /// Get the [`Layout`] for a specific type. pub fn type_size_alignment(typ: &Type

, tcx: &TypeContext) -> Layout { + type_size_alignment_and_scalar_count(typ, tcx).0 +} + +/// Get the [`Layout`] for a specific type, as well as the number of scalar fields it contains +pub fn type_size_alignment_and_scalar_count( + typ: &Type

, + tcx: &TypeContext, +) -> (Layout, usize) { match typ { // repr(C) fieldless enums use the default platform representation: isize - Type::Enum(..) => Layout::new::(), - Type::Opaque(..) => opaque_size_alignment(), - Type::Slice(..) => Layout::new::<(usize_target, usize_target)>(), - Type::Primitive(p) => primitive_size_alignment(*p), + Type::Enum(..) => (Layout::new::(), 1), + Type::Opaque(..) => (opaque_size_alignment(), 1), + Type::Slice(..) => (Layout::new::<(usize_target, usize_target)>(), 2), + Type::Primitive(p) => (primitive_size_alignment(*p), 1), Type::Struct(struct_path) => { let def = tcx.resolve_type(struct_path.id()); - let (_, size_max_align) = match def { + let info = match def { hir::TypeDef::OutStruct(out_struct) => { - struct_offsets_size_max_align(out_struct.fields.iter().map(|f| &f.ty), tcx) + struct_field_info(out_struct.fields.iter().map(|f| &f.ty), tcx) } hir::TypeDef::Struct(struct_def) => { - struct_offsets_size_max_align(struct_def.fields.iter().map(|f| &f.ty), tcx) + struct_field_info(struct_def.fields.iter().map(|f| &f.ty), tcx) } _ => panic!("Should be a struct TypeDef."), }; - size_max_align + (info.struct_layout, info.scalar_count) } _ => unreachable!("Unknown AST/HIR variant {:?}", typ), } diff --git a/tool/src/js/mod.rs b/tool/src/js/mod.rs index 771e2687a..dfc7cfea5 100644 --- a/tool/src/js/mod.rs +++ b/tool/src/js/mod.rs @@ -91,10 +91,11 @@ pub(crate) fn run<'tcx>( let _guard = errors.set_context_ty(type_def.name().as_str().into()); - let name = formatter.fmt_type_name(id); + let type_name = formatter.fmt_type_name(id); let context = TyGenContext { tcx, + type_name, formatter: &formatter, errors: &errors, imports: RefCell::new(BTreeSet::new()), @@ -135,38 +136,35 @@ pub(crate) fn run<'tcx>( methods_info.special_methods.typescript = ts; let contents = match type_def { - TypeDef::Enum(e) => context.gen_enum(ts, &name, e, &methods_info), - TypeDef::Opaque(o) => context.gen_opaque(ts, &name, o, &methods_info), + TypeDef::Enum(e) => context.gen_enum(ts, e, &methods_info), + TypeDef::Opaque(o) => context.gen_opaque(ts, o, &methods_info), TypeDef::Struct(s) => { - context.gen_struct(ts, &name, s, &fields.clone().unwrap(), &methods_info, false) + let (fields, needs_force_padding) = fields.clone().unwrap(); + context.gen_struct(ts, s, &fields, &methods_info, false, needs_force_padding) + } + TypeDef::OutStruct(s) => { + let (fields, needs_force_padding) = fields_out.clone().unwrap(); + context.gen_struct(ts, s, &fields, &methods_info, true, needs_force_padding) } - TypeDef::OutStruct(s) => context.gen_struct( - ts, - &name, - s, - &fields_out.clone().unwrap(), - &methods_info, - true, - ), _ => unreachable!("HIR/AST variant {:?} is unknown.", type_def), }; - let file_name = formatter.fmt_file_name(&name, &file_type); + let file_name = formatter.fmt_file_name(&context.type_name, &file_type); // Remove our self reference: - context.remove_import(name.clone().into()); + context.remove_import(context.type_name.clone().into()); files.add_file(file_name, context.generate_base(ts, contents)); } exports.push( formatter - .fmt_export_statement(&name, false, "./".into()) + .fmt_export_statement(&context.type_name, false, "./".into()) .into(), ); ts_exports.push( formatter - .fmt_export_statement(&name, true, "./".into()) + .fmt_export_statement(&context.type_name, true, "./".into()) .into(), ) } diff --git a/tool/src/js/type_generation/converter.rs b/tool/src/js/type_generation/converter.rs index 6f071e39e..7ba03c57e 100644 --- a/tool/src/js/type_generation/converter.rs +++ b/tool/src/js/type_generation/converter.rs @@ -19,6 +19,21 @@ fn is_contiguous_enum(ty: &hir::EnumDef) -> bool { .all(|(i, v)| i as isize == v.discriminant) } +/// The Rust-Wasm ABI currently treats structs with 1 or 2 scalar fields different from +/// structs with more ("large" structs). Structs with 1 or 2 scalar fields are passed in as consecutive fields, +/// whereas larger structs are passed in as an array of fields *including padding*. This choice is typically at the struct +/// level, however a small struct found within a large struct will also need to care about padding. +#[derive(Copy, Clone, Default, PartialEq, Eq)] +pub(super) enum ForcePaddingStatus { + /// Don't force padding. For structs found in arguments + #[default] + NoForce, + /// Force padding. For small structs found as fields in large structs + Force, + /// Force padding if the caller forces padding. For small structs found as fields in small structs. + PassThrough, +} + /// Context about a struct being borrowed when doing js-to-c conversions /// Borrowed from dart implementation. pub(super) struct StructBorrowContext<'tcx> { @@ -552,14 +567,19 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { js_name: Cow<'tcx, str>, struct_borrow_info: Option<&StructBorrowContext<'tcx>>, alloc: Option<&str>, + // Whether to force padding + force_padding: ForcePaddingStatus, ) -> Cow<'tcx, str> { match *ty { Type::Primitive(..) => js_name.clone(), Type::Opaque(ref op) if op.is_optional() => format!("{js_name}.ffiValue ?? 0").into(), Type::Enum(..) | Type::Opaque(..) => format!("{js_name}.ffiValue").into(), - Type::Struct(..) => { - self.gen_js_to_c_for_struct_type(js_name, struct_borrow_info, alloc.unwrap()) - } + Type::Struct(..) => self.gen_js_to_c_for_struct_type( + js_name, + struct_borrow_info, + alloc.unwrap(), + force_padding, + ), Type::Slice(slice) => { if let Some(hir::MaybeStatic::Static) = slice.lifetime() { panic!("'static not supported for JS backend.") @@ -605,6 +625,7 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { js_name: Cow<'tcx, str>, struct_borrow_info: Option<&StructBorrowContext<'tcx>>, allocator: &str, + force_padding: ForcePaddingStatus, ) -> Cow<'tcx, str> { let mut params = String::new(); if let Some(info) = struct_borrow_info { @@ -629,7 +650,12 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { write!(&mut params, "],").unwrap(); } } - format!("...{js_name}._intoFFI({allocator}, {{{params}}})").into() + let force_padding = match force_padding { + ForcePaddingStatus::NoForce => "", + ForcePaddingStatus::Force => ", true", + ForcePaddingStatus::PassThrough => ", forcePadding", + }; + format!("...{js_name}._intoFFI({allocator}, {{{params}}}{force_padding})").into() } // #endregion } diff --git a/tool/src/js/type_generation/mod.rs b/tool/src/js/type_generation/mod.rs index 260e36ad1..e3475f224 100644 --- a/tool/src/js/type_generation/mod.rs +++ b/tool/src/js/type_generation/mod.rs @@ -20,13 +20,14 @@ use super::formatter::JSFormatter; use crate::ErrorStore; mod converter; -use converter::StructBorrowContext; +use converter::{ForcePaddingStatus, StructBorrowContext}; /// Represents context for generating a Javascript class. /// /// Given an enum, opaque, struct, etc. (anything from [`hir::TypeDef`] that JS supports), this handles creation of the associated `.mjs`` files. pub(super) struct TyGenContext<'ctx, 'tcx> { pub tcx: &'tcx TypeContext, + pub type_name: Cow<'tcx, str>, pub formatter: &'ctx JSFormatter<'tcx>, pub errors: &'ctx ErrorStore<'tcx, String>, /// Imports, stored as a type name. Imports are fully resolved in [`TyGenContext::generate_base`], with a call to [`JSFormatter::fmt_import_statement`]. @@ -82,8 +83,6 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { &self, typescript: bool, - type_name: &str, - enum_def: &'tcx EnumDef, methods: &MethodsInfo, ) -> String { @@ -103,7 +102,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { ImplTemplate { enum_def, formatter: self.formatter, - type_name, + type_name: &self.type_name, typescript, doc_str: self.formatter.fmt_docs(&enum_def.docs), @@ -119,8 +118,6 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { &self, typescript: bool, - type_name: &str, - opaque_def: &'tcx OpaqueDef, methods: &MethodsInfo, ) -> String { @@ -141,7 +138,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { } ImplTemplate { - type_name, + type_name: &self.type_name, typescript, lifetimes: &opaque_def.lifetimes, @@ -155,28 +152,29 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { .unwrap() } - /// Generate a list of [`FieldInfo`] to be used in [`Self::gen_struct`]. We separate this step out for two reasons: + /// Generate a list of [`FieldInfo`] to be used in [`Self::gen_struct`]. + /// + /// Also returns a boolean of whether the forcePadding argument is needed. + /// + /// We separate this step out for two reasons: /// /// 1. It allows re-use between `.d.ts` and `.mjs` files. /// 2. Clarity. pub(super) fn generate_fields( &self, struct_def: &'tcx hir::StructDef

, - ) -> Vec> { - let (offsets, _) = crate::js::layout::struct_offsets_size_max_align( - struct_def.fields.iter().map(|f| &f.ty), - self.tcx, - ); + ) -> (Vec>, bool) { + let struct_field_info = + crate::js::layout::struct_field_info(struct_def.fields.iter().map(|f| &f.ty), self.tcx); + let mut needs_force_padding = false; let fields = struct_def.fields.iter().enumerate() - .map(|field_enumerator| { - let (i, field) = field_enumerator; - + .map(|(i, field)| { let field_name = self.formatter.fmt_param_name(field.name.as_str()); let js_type_name = self.gen_js_type_str(&field.ty); - let c_to_js_deref = self.gen_c_to_js_deref_for_type(&field.ty, "ptr".into(), offsets[i]); + let c_to_js_deref = self.gen_c_to_js_deref_for_type(&field.ty, "ptr".into(), struct_field_info.fields[i].offset); let c_to_js = self.gen_c_to_js_for_type( &field.ty, @@ -217,35 +215,57 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { None }; - let field_layout = crate::js::layout::type_size_alignment(&field.ty, self.tcx); + let padding = struct_field_info.fields[i].padding_count; - let curr_offset = offsets[i]; - let next_offset = if i < offsets.len() - 1 { - offsets[i + 1] - } else { - curr_offset + field_layout.size() - }; - let padding = next_offset - curr_offset - field_layout.size(); + let force_padding = match (struct_field_info.fields[i].scalar_count, struct_field_info.scalar_count) { + // There's no padding needed + (0 | 1, _) => ForcePaddingStatus::NoForce, + // Non-structs don't care + _ if !matches!(&field.ty, &hir::Type::Struct(_)) => ForcePaddingStatus::NoForce, + // 2-field struct contained in 2-field struct, caller decides + (2, 2) => { + needs_force_padding = true; + ForcePaddingStatus::PassThrough + } + // Outer struct has > 3 fields, always pad + (2, 3..) => ForcePaddingStatus::Force, + // Larger fields will always have padding anyway + _ => ForcePaddingStatus::NoForce + + }; - let js_to_c = format!("{}{}", - self.gen_js_to_c_for_type(&field.ty, format!("this.#{}", field_name.clone()).into(), maybe_struct_borrow_info.as_ref(), alloc.as_deref()), - if padding > 0 { - let mut out = format!(",/* Padding for {} */ ", field.name); + let maybe_padding_after = if padding > 0 { + let padding_size_str = match struct_field_info.fields[i].padding_size { + 1 => "i8", + 2 => "i16", + 4 => "i32", + 8 => "i64", + other => unreachable!("Found unknown padding size {other}") + }; + if struct_field_info.scalar_count == 2 { + // For structs with 2 scalar fields, we pass down whether or not padding is needed from the caller + needs_force_padding = true; + format!(", ...diplomatRuntime.maybePaddingFields(forcePadding, {padding} /* x {padding_size_str} */)") + } else { + let mut out = format!(", /* [{padding} x {padding_size_str}] padding */ "); for i in 0..padding { if i < padding - 1 { write!(out, "0, ").unwrap(); } else { - write!(out, "0 /* End Padding */").unwrap(); + write!(out, "0 /* end padding */").unwrap(); } } out - } else { - "".into() } - ); + + } else { + "".into() + }; + let js_to_c = self.gen_js_to_c_for_type(&field.ty, format!("this.#{}", field_name.clone()).into(), maybe_struct_borrow_info.as_ref(), alloc.as_deref(), force_padding); + let js_to_c = format!("{js_to_c}{maybe_padding_after}"); FieldInfo { field_name, @@ -257,7 +277,8 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { maybe_struct_borrow_info: maybe_struct_borrow_info.map(|i| i.param_info) } }).collect::>(); - fields + + (fields, needs_force_padding) } /// Generate a struct type's body for a file from the given definition. @@ -267,13 +288,12 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { &self, typescript: bool, - type_name: &str, - struct_def: &'tcx hir::StructDef

, fields: &Vec>, methods: &MethodsInfo, is_out: bool, + needs_force_padding: bool, ) -> String { #[derive(Template)] #[template(path = "js/struct.js.jinja", escape = "none")] @@ -283,6 +303,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { typescript: bool, mutable: bool, is_out: bool, + needs_force_padding: bool, lifetimes: &'a LifetimeEnv, fields: &'a Vec>, @@ -292,11 +313,12 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { } ImplTemplate { - type_name, + type_name: &self.type_name, typescript, is_out, mutable: !is_out, + needs_force_padding, lifetimes: &struct_def.lifetimes, fields, @@ -379,7 +401,9 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { "Slices must produce slice ParamBorrowInfo, found {param_borrow_kind:?}" ), } - )) + ), + // Arguments never force padding + ForcePaddingStatus::NoForce) ); // We add the pointer and size for slices: @@ -416,6 +440,8 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { param_info.name.clone(), struct_borrow_info.as_ref(), alloc, + // Arguments never force padding + ForcePaddingStatus::NoForce, )); } diff --git a/tool/templates/js/runtime.mjs b/tool/templates/js/runtime.mjs index c77c87515..95b1c719d 100644 --- a/tool/templates/js/runtime.mjs +++ b/tool/templates/js/runtime.mjs @@ -59,6 +59,18 @@ export function enumDiscriminant(wasm, ptr) { return (new Int32Array(wasm.memory.buffer, ptr, 1))[0] } +/** + * Return an array of paddingCount zeroes to be spread into a function call + * if needsPaddingFields is true, else empty + */ +export function maybePaddingFields(needsPaddingFields, paddingCount) { + if (needsPaddingFields) { + return Array(paddingCount).fill(0);; + } else { + return []; + } +} + /** * A wrapper around a slice of WASM memory that can be freed manually or * automatically by the garbage collector. diff --git a/tool/templates/js/struct.js.jinja b/tool/templates/js/struct.js.jinja index f3cf89a48..a00e89950 100644 --- a/tool/templates/js/struct.js.jinja +++ b/tool/templates/js/struct.js.jinja @@ -61,10 +61,17 @@ export class {{type_name}} { // // This method does not handle lifetime relationships: if `'foo: 'bar`, make sure fooAppendArray contains everything barAppendArray does. {%- endif -%} - {% endif %} + {%- endif %} + {%- if needs_force_padding %} + // JS structs need to be generated with or without padding depending on whether they are being passed as aggregates or splatted out into fields. + // Most of the time this is known beforehand: large structs (>2 scalar fields) always get padding, and structs passed directly in parameters omit padding + // if they are small. However small structs within large structs also get padding, and we signal that by setting forcePadding. + {%- endif %} _intoFFI( functionCleanupArena, - appendArrayMap + appendArrayMap{% if needs_force_padding %}, + forcePadding + {%- endif %} ) { return [ {%- for field in fields -%}