From cda7655997012017a71a6da094d2814cf829e536 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 15:01:09 -0800 Subject: [PATCH 01/11] Adding new tests --- feature_tests/js/test/struct-ts.mjs | 6 +++++- feature_tests/js/test/struct-ts.mts | 7 ++++++- feature_tests/js/test/struct.mjs | 7 ++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/feature_tests/js/test/struct-ts.mjs b/feature_tests/js/test/struct-ts.mjs index 5679066c1..0d951f1c9 100644 --- a/feature_tests/js/test/struct-ts.mjs +++ b/feature_tests/js/test/struct-ts.mjs @@ -1,5 +1,5 @@ import test from 'ava'; -import { MyEnum, MyStruct } from "diplomat-wasm-js-feature-tests"; +import { MyEnum, MyStruct, CyclicStructB } from "diplomat-wasm-js-feature-tests"; test("Verify invariants of struct", t => { const s = MyStruct.new_(); t.is(s.a, 17); @@ -23,3 +23,7 @@ test("Test struct creation", t => { }); t.is(s.intoA(), 17); }); +test("Function Returning Nested Struct of One Field", t => { + const a = CyclicStructB.getA(); + t.is(a.cyclicOut(), "0"); +}); diff --git a/feature_tests/js/test/struct-ts.mts b/feature_tests/js/test/struct-ts.mts index e6e45c93b..c0042ec42 100644 --- a/feature_tests/js/test/struct-ts.mts +++ b/feature_tests/js/test/struct-ts.mts @@ -1,5 +1,5 @@ import test from 'ava'; -import { MyEnum, MyStruct } from "diplomat-wasm-js-feature-tests"; +import { MyEnum, MyStruct, CyclicStructB } from "diplomat-wasm-js-feature-tests"; test("Verify invariants of struct", t => { const s = MyStruct.new_(); @@ -24,4 +24,9 @@ test("Test struct creation", t => { g: MyEnum.B }); t.is(s.intoA(), 17); +}); + +test("Function Returning Nested Struct of One Field", t => { + const a = CyclicStructB.getA(); + t.is(a.cyclicOut(), "0"); }); \ No newline at end of file diff --git a/feature_tests/js/test/struct.mjs b/feature_tests/js/test/struct.mjs index c8601bc47..6593d2263 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, ScalarPairWithPadding, BigStructWithStuff } from "diplomat-wasm-js-feature-tests"; +import { MyEnum, MyStruct, ScalarPairWithPadding, BigStructWithStuff, CyclicStructB } from "diplomat-wasm-js-feature-tests"; test("Verify invariants of struct", t => { const s = MyStruct.new_("hello"); @@ -46,3 +46,8 @@ test("Test struct layout: complex struct with multiple padding types and contain s.assertValue(853); t.is(true, true); // Ava doesn't like tests without assertions }); + +test("Function Returning Nested Struct of One Field", t => { + const a = CyclicStructB.getA(); + t.is(a.cyclicOut(), "0"); +}); \ No newline at end of file From 36abedbb9477231d80a157bd8e0d54d9661d92c4 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 15:50:25 -0800 Subject: [PATCH 02/11] Adding detection and logic for structs wrapping one primitive --- tool/src/js/converter.rs | 32 +++++++++++++++++++++----- tool/src/js/gen.rs | 37 +++++++++++++++++++++++++++++-- tool/templates/js/struct.js.jinja | 7 +++++- 3 files changed, 67 insertions(+), 9 deletions(-) diff --git a/tool/src/js/converter.rs b/tool/src/js/converter.rs index 7a3b74b2d..95be108e8 100644 --- a/tool/src/js/converter.rs +++ b/tool/src/js/converter.rs @@ -284,25 +284,44 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { offset: usize, ) -> Cow<'tcx, str> { let pointer = if offset == 0 { - variable_name + variable_name.clone() } else { format!("{variable_name} + {offset}").into() }; - match *ty { + match ty { Type::Enum(..) => format!("diplomatRuntime.enumDiscriminant(wasm, {pointer})").into(), Type::Opaque(..) => format!("diplomatRuntime.ptrRead(wasm, {pointer})").into(), - // Structs always assume they're being passed a pointer, so they handle this in their constructors: - // See NestedBorrowedFields - Type::Struct(..) | Type::Slice(..) | Type::DiplomatOption(..) => pointer, Type::Primitive(p) => format!( "(new {ctor}(wasm.memory.buffer, {pointer}, 1))[0]{cmp}", - ctor = self.formatter.fmt_primitive_slice(p), + ctor = self.formatter.fmt_primitive_slice(*p), cmp = match p { PrimitiveType::Bool => " === 1", _ => "", } ) .into(), + Type::Struct(st) if match st.id() { + hir::TypeId::OutStruct(s) => self.only_primitive(self.tcx.resolve_out_struct(s)), + hir::TypeId::Struct(s) => self.only_primitive(self.tcx.resolve_struct(s)), + _ => false + } => { + match st.id() { + hir::TypeId::OutStruct(s) => { + let first = self.tcx.resolve_out_struct(s).fields.first().unwrap(); + + self.gen_c_to_js_deref_for_type(&first.ty, variable_name, offset) + }, + hir::TypeId::Struct(s) => { + let first = self.tcx.resolve_struct(s).fields.first().unwrap(); + + self.gen_c_to_js_deref_for_type(&first.ty, variable_name, offset) + }, + _ => unreachable!("Expected struct, got {:?}", st.id()) + } + }, + // Structs (nearly) always assume they're being passed a pointer, so they handle this in their constructors: + // See NestedBorrowedFields + Type::Struct(..) | Type::Slice(..) | Type::DiplomatOption(..) => pointer, _ => unreachable!("Unknown AST/HIR variant {:?}", ty), } } @@ -381,6 +400,7 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { ReturnType::Infallible(SuccessType::OutType(ref o)) => { let mut result = "result"; match o { + Type::Struct(s) if self.wraps_a_primitive(s) => {}, Type::Struct(_) | Type::Slice(_) => { let layout = crate::js::layout::type_size_alignment(o, self.tcx); let size = layout.size(); diff --git a/tool/src/js/gen.rs b/tool/src/js/gen.rs index ae3bb1001..b85a15437 100644 --- a/tool/src/js/gen.rs +++ b/tool/src/js/gen.rs @@ -10,8 +10,7 @@ use diplomat_core::hir::borrowing_param::{ BorrowedLifetimeInfo, LifetimeEdge, LifetimeEdgeKind, ParamBorrowInfo, StructBorrowInfo, }; use diplomat_core::hir::{ - self, EnumDef, LifetimeEnv, Method, OpaqueDef, SpecialMethod, SpecialMethodPresence, Type, - TypeContext, TypeId, + self, EnumDef, LifetimeEnv, Method, OpaqueDef, SpecialMethod, SpecialMethodPresence, StructPathLike, Type, TypeContext, TypeId }; use askama::{self, Template}; @@ -293,6 +292,36 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { (fields, needs_force_padding) } + pub(super) fn only_primitive(&self, st : &hir::StructDef

) -> bool { + if st.fields.len() != 1 { + return false; + } + + let first = st.fields.first().unwrap(); + + match &first.ty { + hir::Type::Primitive(..) => true, + hir::Type::Struct(s) => { + match s.id() { + hir::TypeId::Struct(s) => self.only_primitive(self.tcx.resolve_struct(s)), + hir::TypeId::OutStruct(s) => self.only_primitive(self.tcx.resolve_out_struct(s)), + _ => false + } + }, + _ => false + } + } + + /// WASM only returns a primitive (instead of a pointer) if our struct just wraps a primitive (or nests a struct that only has one primitive as a field). + /// This is a quick way to verify that we are grabbing a value instead of a pointer. + pub(super) fn wraps_a_primitive(&self, st: &hir::ReturnableStructPath) -> bool { + match st.resolve(&self.tcx) { + hir::ReturnableStructDef::OutStruct(s) => self.only_primitive(s), + hir::ReturnableStructDef::Struct(s) => self.only_primitive(s), + _ => false + } + } + /// Generate a struct type's body for a file from the given definition. /// /// Used for both [`hir::TypeDef::Struct`] and [`hir::TypeDef::OutStruct`], which is why `is_out` exists. @@ -321,6 +350,8 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { fields: &'a Vec>, methods: &'a MethodsInfo<'a>, + wraps_primitive: bool, + docs: String, } @@ -336,6 +367,8 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { fields, methods, + wraps_primitive: self.only_primitive(struct_def), + docs: self.formatter.fmt_docs(&struct_def.docs), } .render() diff --git a/tool/templates/js/struct.js.jinja b/tool/templates/js/struct.js.jinja index 636225de5..ae6fa4451 100644 --- a/tool/templates/js/struct.js.jinja +++ b/tool/templates/js/struct.js.jinja @@ -112,15 +112,20 @@ export class {{type_name}} { // This method does not attempt to handle any dependencies between lifetimes, the caller // should handle this when constructing edge arrays. {% endif -%} - static _fromFFI(internalConstructor, ptr{%- for lifetime in lifetimes.all_lifetimes() -%}, {{lifetimes.fmt_lifetime(lifetime)}}Edges{%- endfor -%}) { + static _fromFFI(internalConstructor, {%- if !wraps_primitive %} ptr {%- else %} primitiveValue {%- endif -%} {%- for lifetime in lifetimes.all_lifetimes() -%}, {{lifetimes.fmt_lifetime(lifetime)}}Edges{%- endfor -%}) { if (internalConstructor !== diplomatRuntime.internalConstructor) { throw new Error("{{type_name}}._fromFFI is not meant to be called externally. Please use the default constructor."); } var structObj = {}; + + {%- if wraps_primitive %} + structObj.{{fields.first().unwrap().field_name}} = primitiveValue; + {% else %} {%- for field in fields %} const {{field.field_name}}Deref = {{field.c_to_js_deref}}; structObj.{{field.field_name}} = {{field.c_to_js}}; {%- endfor %} + {%- endif %} return new {{type_name}}(structObj, internalConstructor); } From e1d9239350898b648448dabcc1404da8ea87a368 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 15:50:31 -0800 Subject: [PATCH 03/11] Generation --- feature_tests/js/api/CyclicStructA.mjs | 16 ++++++---------- feature_tests/js/api/CyclicStructB.mjs | 16 ++++++---------- feature_tests/js/api/CyclicStructC.mjs | 6 +++--- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/feature_tests/js/api/CyclicStructA.mjs b/feature_tests/js/api/CyclicStructA.mjs index 86c30671c..1f0ebf76e 100644 --- a/feature_tests/js/api/CyclicStructA.mjs +++ b/feature_tests/js/api/CyclicStructA.mjs @@ -49,29 +49,25 @@ export class CyclicStructA { // 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. - static _fromFFI(internalConstructor, ptr) { + static _fromFFI(internalConstructor, primitiveValue) { if (internalConstructor !== diplomatRuntime.internalConstructor) { throw new Error("CyclicStructA._fromFFI is not meant to be called externally. Please use the default constructor."); } var structObj = {}; - const aDeref = ptr; - structObj.a = CyclicStructB._fromFFI(diplomatRuntime.internalConstructor, aDeref); + structObj.a = primitiveValue; + return new CyclicStructA(structObj, internalConstructor); } static getB() { - const diplomatReceive = new diplomatRuntime.DiplomatReceiveBuf(wasm, 1, 1, false); - - const result = wasm.CyclicStructA_get_b(diplomatReceive.buffer); + const result = wasm.CyclicStructA_get_b(); try { - return CyclicStructB._fromFFI(diplomatRuntime.internalConstructor, diplomatReceive.buffer); + return CyclicStructB._fromFFI(diplomatRuntime.internalConstructor, result); } - finally { - diplomatReceive.free(); - } + finally {} } cyclicOut() { diff --git a/feature_tests/js/api/CyclicStructB.mjs b/feature_tests/js/api/CyclicStructB.mjs index 53f4f3a50..cddce53cf 100644 --- a/feature_tests/js/api/CyclicStructB.mjs +++ b/feature_tests/js/api/CyclicStructB.mjs @@ -49,28 +49,24 @@ export class CyclicStructB { // 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. - static _fromFFI(internalConstructor, ptr) { + static _fromFFI(internalConstructor, primitiveValue) { if (internalConstructor !== diplomatRuntime.internalConstructor) { throw new Error("CyclicStructB._fromFFI is not meant to be called externally. Please use the default constructor."); } var structObj = {}; - const fieldDeref = (new Uint8Array(wasm.memory.buffer, ptr, 1))[0]; - structObj.field = fieldDeref; + structObj.field = primitiveValue; + return new CyclicStructB(structObj, internalConstructor); } static getA() { - const diplomatReceive = new diplomatRuntime.DiplomatReceiveBuf(wasm, 1, 1, false); - - const result = wasm.CyclicStructB_get_a(diplomatReceive.buffer); + const result = wasm.CyclicStructB_get_a(); try { - return CyclicStructA._fromFFI(diplomatRuntime.internalConstructor, diplomatReceive.buffer); + return CyclicStructA._fromFFI(diplomatRuntime.internalConstructor, result); } - finally { - diplomatReceive.free(); - } + finally {} } } \ No newline at end of file diff --git a/feature_tests/js/api/CyclicStructC.mjs b/feature_tests/js/api/CyclicStructC.mjs index 561d66a21..3d73f6795 100644 --- a/feature_tests/js/api/CyclicStructC.mjs +++ b/feature_tests/js/api/CyclicStructC.mjs @@ -49,13 +49,13 @@ export class CyclicStructC { // 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. - static _fromFFI(internalConstructor, ptr) { + static _fromFFI(internalConstructor, primitiveValue) { if (internalConstructor !== diplomatRuntime.internalConstructor) { throw new Error("CyclicStructC._fromFFI is not meant to be called externally. Please use the default constructor."); } var structObj = {}; - const aDeref = ptr; - structObj.a = CyclicStructA._fromFFI(diplomatRuntime.internalConstructor, aDeref); + structObj.a = primitiveValue; + return new CyclicStructC(structObj, internalConstructor); } From 298027b22f51f92fefabc22ab620024b398c1192 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 15:53:09 -0800 Subject: [PATCH 04/11] Adding test for de-referencing structs that wrap --- feature_tests/js/api/CyclicStructB.d.ts | 2 ++ feature_tests/js/api/CyclicStructB.mjs | 17 +++++++++++++++++ feature_tests/js/test/struct.mjs | 7 ++++++- feature_tests/src/structs.rs | 4 ++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/feature_tests/js/api/CyclicStructB.d.ts b/feature_tests/js/api/CyclicStructB.d.ts index 658eb7636..955c36ea1 100644 --- a/feature_tests/js/api/CyclicStructB.d.ts +++ b/feature_tests/js/api/CyclicStructB.d.ts @@ -13,4 +13,6 @@ export class CyclicStructB { constructor(structObj : CyclicStructB_Obj); static getA(): CyclicStructA; + + static getAOption(): CyclicStructA | null; } \ No newline at end of file diff --git a/feature_tests/js/api/CyclicStructB.mjs b/feature_tests/js/api/CyclicStructB.mjs index cddce53cf..2129fa5fd 100644 --- a/feature_tests/js/api/CyclicStructB.mjs +++ b/feature_tests/js/api/CyclicStructB.mjs @@ -69,4 +69,21 @@ export class CyclicStructB { finally {} } + + static getAOption() { + const diplomatReceive = new diplomatRuntime.DiplomatReceiveBuf(wasm, 2, 1, true); + + const result = wasm.CyclicStructB_get_a_option(diplomatReceive.buffer); + + try { + if (!diplomatReceive.resultFlag) { + return null; + } + return CyclicStructA._fromFFI(diplomatRuntime.internalConstructor, (new Uint8Array(wasm.memory.buffer, diplomatReceive.buffer, 1))[0]); + } + + finally { + diplomatReceive.free(); + } + } } \ No newline at end of file diff --git a/feature_tests/js/test/struct.mjs b/feature_tests/js/test/struct.mjs index 6593d2263..31041f43f 100644 --- a/feature_tests/js/test/struct.mjs +++ b/feature_tests/js/test/struct.mjs @@ -47,7 +47,12 @@ test("Test struct layout: complex struct with multiple padding types and contain t.is(true, true); // Ava doesn't like tests without assertions }); -test("Function Returning Nested Struct of One Field", t => { +test("Function Returning Nested Struct of One Primitive", t => { const a = CyclicStructB.getA(); t.is(a.cyclicOut(), "0"); +}); + +test("Function De-Referencing Nested Struct of One Primitive", t => { + const a = CyclicStructB.getAOption(); + t.is(a.cyclicOut(), "0"); }); \ No newline at end of file diff --git a/feature_tests/src/structs.rs b/feature_tests/src/structs.rs index 8cb1515a8..66039ca39 100644 --- a/feature_tests/src/structs.rs +++ b/feature_tests/src/structs.rs @@ -228,6 +228,10 @@ pub mod ffi { pub fn get_a() -> CyclicStructA { Default::default() } + + pub fn get_a_option() -> Option { + Some(Default::default()) + } } impl CyclicStructC { From baa5e9adbf042858bc2edeaef46fb5e309f49215 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 15:54:15 -0800 Subject: [PATCH 05/11] Add test to .mts --- feature_tests/js/test/struct-ts.mts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/feature_tests/js/test/struct-ts.mts b/feature_tests/js/test/struct-ts.mts index c0042ec42..2a0efc56c 100644 --- a/feature_tests/js/test/struct-ts.mts +++ b/feature_tests/js/test/struct-ts.mts @@ -29,4 +29,9 @@ test("Test struct creation", t => { test("Function Returning Nested Struct of One Field", t => { const a = CyclicStructB.getA(); t.is(a.cyclicOut(), "0"); +}); + +test("Function De-Referencing Nested Struct of One Primitive", t => { + const a = CyclicStructB.getAOption(); + t.is(a.cyclicOut(), "0"); }); \ No newline at end of file From 5eae5811d5a0702144d3b57cce10c92424c64445 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 16:01:32 -0800 Subject: [PATCH 06/11] Add quick check for owning of primitive --- feature_tests/js/api/CyclicStructA.mjs | 4 ++-- feature_tests/js/api/CyclicStructC.mjs | 4 ++-- tool/src/js/gen.rs | 2 ++ tool/templates/js/struct.js.jinja | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/feature_tests/js/api/CyclicStructA.mjs b/feature_tests/js/api/CyclicStructA.mjs index 1f0ebf76e..8c60d800f 100644 --- a/feature_tests/js/api/CyclicStructA.mjs +++ b/feature_tests/js/api/CyclicStructA.mjs @@ -54,8 +54,8 @@ export class CyclicStructA { throw new Error("CyclicStructA._fromFFI is not meant to be called externally. Please use the default constructor."); } var structObj = {}; - structObj.a = primitiveValue; - + const aDeref = primitiveValue; + structObj.a = CyclicStructB._fromFFI(diplomatRuntime.internalConstructor, aDeref); return new CyclicStructA(structObj, internalConstructor); } diff --git a/feature_tests/js/api/CyclicStructC.mjs b/feature_tests/js/api/CyclicStructC.mjs index 3d73f6795..5107418d7 100644 --- a/feature_tests/js/api/CyclicStructC.mjs +++ b/feature_tests/js/api/CyclicStructC.mjs @@ -54,8 +54,8 @@ export class CyclicStructC { throw new Error("CyclicStructC._fromFFI is not meant to be called externally. Please use the default constructor."); } var structObj = {}; - structObj.a = primitiveValue; - + const aDeref = primitiveValue; + structObj.a = CyclicStructA._fromFFI(diplomatRuntime.internalConstructor, aDeref); return new CyclicStructC(structObj, internalConstructor); } diff --git a/tool/src/js/gen.rs b/tool/src/js/gen.rs index b85a15437..8185a47a3 100644 --- a/tool/src/js/gen.rs +++ b/tool/src/js/gen.rs @@ -351,6 +351,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { methods: &'a MethodsInfo<'a>, wraps_primitive: bool, + owns_wrapped_primitive: bool, docs: String, } @@ -368,6 +369,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { methods, wraps_primitive: self.only_primitive(struct_def), + owns_wrapped_primitive: struct_def.fields.first().is_some() && matches!(struct_def.fields.first().unwrap().ty, hir::Type::Primitive(..)), docs: self.formatter.fmt_docs(&struct_def.docs), } diff --git a/tool/templates/js/struct.js.jinja b/tool/templates/js/struct.js.jinja index ae6fa4451..b79a77af7 100644 --- a/tool/templates/js/struct.js.jinja +++ b/tool/templates/js/struct.js.jinja @@ -118,11 +118,11 @@ export class {{type_name}} { } var structObj = {}; - {%- if wraps_primitive %} + {%- if wraps_primitive && owns_wrapped_primitive %} structObj.{{fields.first().unwrap().field_name}} = primitiveValue; {% else %} {%- for field in fields %} - const {{field.field_name}}Deref = {{field.c_to_js_deref}}; + const {{field.field_name}}Deref = {%- if wraps_primitive && !owns_wrapped_primitive %} primitiveValue {%- else %} {{field.c_to_js_deref}} {%- endif %}; structObj.{{field.field_name}} = {{field.c_to_js}}; {%- endfor %} {%- endif %} From b7b56f809e90daeb84d2233f06f28ee83f685bc6 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 16:01:51 -0800 Subject: [PATCH 07/11] Update struct-ts.mjs --- feature_tests/js/test/struct-ts.mjs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/feature_tests/js/test/struct-ts.mjs b/feature_tests/js/test/struct-ts.mjs index 0d951f1c9..ff9a237bc 100644 --- a/feature_tests/js/test/struct-ts.mjs +++ b/feature_tests/js/test/struct-ts.mjs @@ -27,3 +27,7 @@ test("Function Returning Nested Struct of One Field", t => { const a = CyclicStructB.getA(); t.is(a.cyclicOut(), "0"); }); +test("Function De-Referencing Nested Struct of One Primitive", t => { + const a = CyclicStructB.getAOption(); + t.is(a.cyclicOut(), "0"); +}); From 088df5af706cbc06591301aa2e1e755f4e67265d Mon Sep 17 00:00:00 2001 From: Tyler K Date: Fri, 6 Dec 2024 16:02:01 -0800 Subject: [PATCH 08/11] Formatting --- tool/src/js/converter.rs | 28 ++++++++++++++++------------ tool/src/js/gen.rs | 25 ++++++++++++++----------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/tool/src/js/converter.rs b/tool/src/js/converter.rs index 95be108e8..0291416bf 100644 --- a/tool/src/js/converter.rs +++ b/tool/src/js/converter.rs @@ -300,25 +300,29 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { } ) .into(), - Type::Struct(st) if match st.id() { - hir::TypeId::OutStruct(s) => self.only_primitive(self.tcx.resolve_out_struct(s)), - hir::TypeId::Struct(s) => self.only_primitive(self.tcx.resolve_struct(s)), - _ => false - } => { + Type::Struct(st) + if match st.id() { + hir::TypeId::OutStruct(s) => { + self.only_primitive(self.tcx.resolve_out_struct(s)) + } + hir::TypeId::Struct(s) => self.only_primitive(self.tcx.resolve_struct(s)), + _ => false, + } => + { match st.id() { hir::TypeId::OutStruct(s) => { let first = self.tcx.resolve_out_struct(s).fields.first().unwrap(); - + self.gen_c_to_js_deref_for_type(&first.ty, variable_name, offset) - }, + } hir::TypeId::Struct(s) => { let first = self.tcx.resolve_struct(s).fields.first().unwrap(); - + self.gen_c_to_js_deref_for_type(&first.ty, variable_name, offset) - }, - _ => unreachable!("Expected struct, got {:?}", st.id()) + } + _ => unreachable!("Expected struct, got {:?}", st.id()), } - }, + } // Structs (nearly) always assume they're being passed a pointer, so they handle this in their constructors: // See NestedBorrowedFields Type::Struct(..) | Type::Slice(..) | Type::DiplomatOption(..) => pointer, @@ -400,7 +404,7 @@ impl<'jsctx, 'tcx> TyGenContext<'jsctx, 'tcx> { ReturnType::Infallible(SuccessType::OutType(ref o)) => { let mut result = "result"; match o { - Type::Struct(s) if self.wraps_a_primitive(s) => {}, + Type::Struct(s) if self.wraps_a_primitive(s) => {} Type::Struct(_) | Type::Slice(_) => { let layout = crate::js::layout::type_size_alignment(o, self.tcx); let size = layout.size(); diff --git a/tool/src/js/gen.rs b/tool/src/js/gen.rs index 8185a47a3..7d07403d3 100644 --- a/tool/src/js/gen.rs +++ b/tool/src/js/gen.rs @@ -10,7 +10,8 @@ use diplomat_core::hir::borrowing_param::{ BorrowedLifetimeInfo, LifetimeEdge, LifetimeEdgeKind, ParamBorrowInfo, StructBorrowInfo, }; use diplomat_core::hir::{ - self, EnumDef, LifetimeEnv, Method, OpaqueDef, SpecialMethod, SpecialMethodPresence, StructPathLike, Type, TypeContext, TypeId + self, EnumDef, LifetimeEnv, Method, OpaqueDef, SpecialMethod, SpecialMethodPresence, + StructPathLike, Type, TypeContext, TypeId, }; use askama::{self, Template}; @@ -292,7 +293,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { (fields, needs_force_padding) } - pub(super) fn only_primitive(&self, st : &hir::StructDef

) -> bool { + pub(super) fn only_primitive(&self, st: &hir::StructDef

) -> bool { if st.fields.len() != 1 { return false; } @@ -301,14 +302,12 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { match &first.ty { hir::Type::Primitive(..) => true, - hir::Type::Struct(s) => { - match s.id() { - hir::TypeId::Struct(s) => self.only_primitive(self.tcx.resolve_struct(s)), - hir::TypeId::OutStruct(s) => self.only_primitive(self.tcx.resolve_out_struct(s)), - _ => false - } + hir::Type::Struct(s) => match s.id() { + hir::TypeId::Struct(s) => self.only_primitive(self.tcx.resolve_struct(s)), + hir::TypeId::OutStruct(s) => self.only_primitive(self.tcx.resolve_out_struct(s)), + _ => false, }, - _ => false + _ => false, } } @@ -318,7 +317,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { match st.resolve(&self.tcx) { hir::ReturnableStructDef::OutStruct(s) => self.only_primitive(s), hir::ReturnableStructDef::Struct(s) => self.only_primitive(s), - _ => false + _ => false, } } @@ -369,7 +368,11 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { methods, wraps_primitive: self.only_primitive(struct_def), - owns_wrapped_primitive: struct_def.fields.first().is_some() && matches!(struct_def.fields.first().unwrap().ty, hir::Type::Primitive(..)), + owns_wrapped_primitive: struct_def.fields.first().is_some() + && matches!( + struct_def.fields.first().unwrap().ty, + hir::Type::Primitive(..) + ), docs: self.formatter.fmt_docs(&struct_def.docs), } From b11f98e02a7d1cff849a9326920674878912096e Mon Sep 17 00:00:00 2001 From: Tyler K Date: Sat, 7 Dec 2024 20:02:20 -0800 Subject: [PATCH 09/11] Generation --- feature_tests/c/include/CyclicStructB.h | 3 +++ feature_tests/cpp/include/CyclicStructB.d.hpp | 2 ++ feature_tests/cpp/include/CyclicStructB.hpp | 8 ++++++ .../dart/lib/src/CyclicStructB.g.dart | 13 ++++++++++ feature_tests/dart/lib/src/lib.g.dart | 25 +++++++++++++++++++ .../dev/diplomattest/somelib/CyclicStructB.kt | 12 +++++++++ .../kotlin/dev/diplomattest/somelib/Lib.kt | 20 +++++++++++++++ 7 files changed, 83 insertions(+) diff --git a/feature_tests/c/include/CyclicStructB.h b/feature_tests/c/include/CyclicStructB.h index 88b0942b9..e7d8235d9 100644 --- a/feature_tests/c/include/CyclicStructB.h +++ b/feature_tests/c/include/CyclicStructB.h @@ -18,6 +18,9 @@ CyclicStructA CyclicStructB_get_a(void); +typedef struct CyclicStructB_get_a_option_result {union {CyclicStructA ok; }; bool is_ok;} CyclicStructB_get_a_option_result; +CyclicStructB_get_a_option_result CyclicStructB_get_a_option(void); + diff --git a/feature_tests/cpp/include/CyclicStructB.d.hpp b/feature_tests/cpp/include/CyclicStructB.d.hpp index f93dd8d94..a97ca090b 100644 --- a/feature_tests/cpp/include/CyclicStructB.d.hpp +++ b/feature_tests/cpp/include/CyclicStructB.d.hpp @@ -28,6 +28,8 @@ struct CyclicStructB { inline static CyclicStructA get_a(); + inline static std::optional get_a_option(); + inline diplomat::capi::CyclicStructB AsFFI() const; inline static CyclicStructB FromFFI(diplomat::capi::CyclicStructB c_struct); }; diff --git a/feature_tests/cpp/include/CyclicStructB.hpp b/feature_tests/cpp/include/CyclicStructB.hpp index dfc38f135..d9348b155 100644 --- a/feature_tests/cpp/include/CyclicStructB.hpp +++ b/feature_tests/cpp/include/CyclicStructB.hpp @@ -19,6 +19,9 @@ namespace capi { diplomat::capi::CyclicStructA CyclicStructB_get_a(void); + typedef struct CyclicStructB_get_a_option_result {union {diplomat::capi::CyclicStructA ok; }; bool is_ok;} CyclicStructB_get_a_option_result; + CyclicStructB_get_a_option_result CyclicStructB_get_a_option(void); + } // extern "C" } // namespace capi @@ -29,6 +32,11 @@ inline CyclicStructA CyclicStructB::get_a() { return CyclicStructA::FromFFI(result); } +inline std::optional CyclicStructB::get_a_option() { + auto result = diplomat::capi::CyclicStructB_get_a_option(); + return result.is_ok ? std::optional(CyclicStructA::FromFFI(result.ok)) : std::nullopt; +} + inline diplomat::capi::CyclicStructB CyclicStructB::AsFFI() const { return diplomat::capi::CyclicStructB { diff --git a/feature_tests/dart/lib/src/CyclicStructB.g.dart b/feature_tests/dart/lib/src/CyclicStructB.g.dart index 6b8aba647..62ada634e 100644 --- a/feature_tests/dart/lib/src/CyclicStructB.g.dart +++ b/feature_tests/dart/lib/src/CyclicStructB.g.dart @@ -33,6 +33,14 @@ final class CyclicStructB { return CyclicStructA._fromFfi(result); } + static CyclicStructA? getAOption() { + final result = _CyclicStructB_get_a_option(); + if (!result.isOk) { + return null; + } + return CyclicStructA._fromFfi(result.union.ok); + } + @override bool operator ==(Object other) => other is CyclicStructB && @@ -48,3 +56,8 @@ final class CyclicStructB { @ffi.Native<_CyclicStructAFfi Function()>(isLeaf: true, symbol: 'CyclicStructB_get_a') // ignore: non_constant_identifier_names external _CyclicStructAFfi _CyclicStructB_get_a(); + +@meta.RecordUse() +@ffi.Native<_ResultCyclicStructAFfiVoid Function()>(isLeaf: true, symbol: 'CyclicStructB_get_a_option') +// ignore: non_constant_identifier_names +external _ResultCyclicStructAFfiVoid _CyclicStructB_get_a_option(); diff --git a/feature_tests/dart/lib/src/lib.g.dart b/feature_tests/dart/lib/src/lib.g.dart index c00c757cb..fdc4f944a 100644 --- a/feature_tests/dart/lib/src/lib.g.dart +++ b/feature_tests/dart/lib/src/lib.g.dart @@ -118,6 +118,31 @@ class _FinalizedArena { } } +final class _ResultCyclicStructAFfiVoidUnion extends ffi.Union { + external _CyclicStructAFfi ok; + +} + +final class _ResultCyclicStructAFfiVoid extends ffi.Struct { + external _ResultCyclicStructAFfiVoidUnion union; + + @ffi.Bool() + external bool isOk; + + + factory _ResultCyclicStructAFfiVoid.ok(_CyclicStructAFfi val) { + final struct = ffi.Struct.create<_ResultCyclicStructAFfiVoid>(); + struct.isOk = true; + struct.union.ok = val; + return struct; + } + factory _ResultCyclicStructAFfiVoid.err() { + final struct = ffi.Struct.create<_ResultCyclicStructAFfiVoid>(); + struct.isOk = false; + return struct; + } +} + final class _ResultDoubleVoidUnion extends ffi.Union { @ffi.Double() external double ok; diff --git a/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/CyclicStructB.kt b/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/CyclicStructB.kt index 87bb2a3ff..e3a8b6af0 100644 --- a/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/CyclicStructB.kt +++ b/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/CyclicStructB.kt @@ -8,6 +8,7 @@ import com.sun.jna.Structure internal interface CyclicStructBLib: Library { fun CyclicStructB_get_a(): CyclicStructANative + fun CyclicStructB_get_a_option(): OptionCyclicStructANative } internal class CyclicStructBNative: Structure(), Structure.ByValue { @@ -36,6 +37,17 @@ class CyclicStructB internal constructor ( val returnStruct = CyclicStructA(returnVal) return returnStruct } + + fun getAOption(): CyclicStructA? { + + val returnVal = lib.CyclicStructB_get_a_option(); + + val intermediateOption = returnVal.option() ?: return null + + val returnStruct = CyclicStructA(intermediateOption) + return returnStruct + + } } } diff --git a/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/Lib.kt b/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/Lib.kt index 04522f675..8f5c948a5 100644 --- a/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/Lib.kt +++ b/feature_tests/kotlin/somelib/src/main/kotlin/dev/diplomattest/somelib/Lib.kt @@ -495,6 +495,26 @@ internal class OptionByte: Structure(), Structure.ByValue { } } } +internal class OptionCyclicStructANative: Structure(), Structure.ByValue { + @JvmField + internal var value: CyclicStructANative = CyclicStructANative() + + @JvmField + internal var isOk: Byte = 0 + + // Define the fields of the struct + override fun getFieldOrder(): List { + return listOf("value", "isOk") + } + + internal fun option(): CyclicStructANative? { + if (isOk == 1.toByte()) { + return value + } else { + return null + } + } +} internal class OptionDouble: Structure(), Structure.ByValue { @JvmField internal var value: Double = 0.0 From f422d504cabfda499fc1291c9911500eb40e7306 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Sun, 8 Dec 2024 13:14:34 -0800 Subject: [PATCH 10/11] Clippy --- core/src/hir/type_context.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/hir/type_context.rs b/core/src/hir/type_context.rs index 747261783..7c7fc3c41 100644 --- a/core/src/hir/type_context.rs +++ b/core/src/hir/type_context.rs @@ -103,7 +103,7 @@ impl TypeContext { ) } - pub fn all_traits<'tcx>(&'tcx self) -> impl Iterator { + pub fn all_traits<'tcx>(&'tcx self) -> impl Iterator { self.traits .iter() .enumerate() @@ -203,7 +203,7 @@ impl TypeContext { pub(super) fn from_ast_without_validation<'ast>( env: &'ast Env, attr_validator: impl AttributeValidator + 'static, - ) -> Result<(LoweringContext, Self), Vec> { + ) -> Result<(LoweringContext<'ast>, Self), Vec> { let mut ast_out_structs = SmallVec::<[_; 16]>::new(); let mut ast_structs = SmallVec::<[_; 16]>::new(); let mut ast_opaques = SmallVec::<[_; 16]>::new(); From 44505a756cf816ab08e68c6615a694fad65f6cd1 Mon Sep 17 00:00:00 2001 From: Tyler K Date: Sun, 8 Dec 2024 13:27:27 -0800 Subject: [PATCH 11/11] Clippy errors --- tool/src/js/gen.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tool/src/js/gen.rs b/tool/src/js/gen.rs index 7d07403d3..c12488edc 100644 --- a/tool/src/js/gen.rs +++ b/tool/src/js/gen.rs @@ -314,7 +314,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { /// WASM only returns a primitive (instead of a pointer) if our struct just wraps a primitive (or nests a struct that only has one primitive as a field). /// This is a quick way to verify that we are grabbing a value instead of a pointer. pub(super) fn wraps_a_primitive(&self, st: &hir::ReturnableStructPath) -> bool { - match st.resolve(&self.tcx) { + match st.resolve(self.tcx) { hir::ReturnableStructDef::OutStruct(s) => self.only_primitive(s), hir::ReturnableStructDef::Struct(s) => self.only_primitive(s), _ => false, @@ -368,7 +368,7 @@ impl<'ctx, 'tcx> TyGenContext<'ctx, 'tcx> { methods, wraps_primitive: self.only_primitive(struct_def), - owns_wrapped_primitive: struct_def.fields.first().is_some() + owns_wrapped_primitive: !struct_def.fields.is_empty() && matches!( struct_def.fields.first().unwrap().ty, hir::Type::Primitive(..)