From 735ea444f2c15771736260fc78704f6b9132ceac Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Mon, 29 Apr 2024 14:20:05 +0200 Subject: [PATCH] Embind: Fix out-param handling ...by using UnoInOutParam in all cases. Some types whose Embind mappings don't employ any smart pointers (like any and sequence) would have worked directly with pointers, but others (like string and type) would have caused Embind errors at runtime. So consistently use UnoInOutParam in all cases (and generate bindings for all the used cases in embindmaker). Change-Id: If26551bf4e99d10748aec1597d6e99f994dfd86e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166854 Tested-by: Jenkins Reviewed-by: Stephan Bergmann --- .../unoembindhelpers/PrimaryBindings.hxx | 6 ++ offapi/org/libreoffice/embindtest/XTest.idl | 5 +- static/source/embindmaker/embindmaker.cxx | 79 ++++++++----------- .../unoembindhelpers/PrimaryBindings.cxx | 18 ----- unotest/source/embindtest/embindtest.cxx | 15 +++- unotest/source/embindtest/embindtest.js | 38 ++++++++- 6 files changed, 95 insertions(+), 66 deletions(-) diff --git a/include/static/unoembindhelpers/PrimaryBindings.hxx b/include/static/unoembindhelpers/PrimaryBindings.hxx index 6acf2803132a..2af843ef6a3d 100644 --- a/include/static/unoembindhelpers/PrimaryBindings.hxx +++ b/include/static/unoembindhelpers/PrimaryBindings.hxx @@ -122,6 +122,12 @@ template void registerSequence(char const* name) }); registerUnoType>(); } + +template void registerInOutParameter(char const* name) +{ + emscripten::class_>(name).constructor().template constructor().property( + "val", &UnoInOutParam::get, &UnoInOutParam::set); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/offapi/org/libreoffice/embindtest/XTest.idl b/offapi/org/libreoffice/embindtest/XTest.idl index 38ac60a47618..b84753ebb8c6 100644 --- a/offapi/org/libreoffice/embindtest/XTest.idl +++ b/offapi/org/libreoffice/embindtest/XTest.idl @@ -117,7 +117,10 @@ interface XTest { void getOut( [out] boolean value1, [out] byte value2, [out] short value3, [out] unsigned short value4, [out] long value5, [out] unsigned long value6, [out] hyper value7, - [out] unsigned hyper value8, [out] float value9, [out] double value10, [out] char value11); + [out] unsigned hyper value8, [out] float value9, [out] double value10, [out] char value11, + [out] string value12, [out] type value13, [out] any value14, + [out] sequence value15, [out] Enum value16, [out] Struct value17, + [out] XTest value18); void throwRuntimeException(); void passJob([in] com::sun::star::task::XJob object); void passJobExecutor([in] com::sun::star::task::XJobExecutor object); diff --git a/static/source/embindmaker/embindmaker.cxx b/static/source/embindmaker/embindmaker.cxx index 6c0017ccb8c2..0b7634cf8409 100644 --- a/static/source/embindmaker/embindmaker.cxx +++ b/static/source/embindmaker/embindmaker.cxx @@ -558,49 +558,13 @@ void dumpParameters(std::ostream& out, rtl::Reference const& manage { out << ", "; } - bool wrap = false; - if (param.direction != unoidl::InterfaceTypeEntity::Method::Parameter::DIRECTION_IN) - { - switch (manager->getSort(resolveOuterTypedefs(manager, param.type))) - { - case codemaker::UnoType::Sort::Boolean: - case codemaker::UnoType::Sort::Byte: - case codemaker::UnoType::Sort::Short: - case codemaker::UnoType::Sort::UnsignedShort: - case codemaker::UnoType::Sort::Long: - case codemaker::UnoType::Sort::UnsignedLong: - case codemaker::UnoType::Sort::Hyper: - case codemaker::UnoType::Sort::UnsignedHyper: - case codemaker::UnoType::Sort::Float: - case codemaker::UnoType::Sort::Double: - case codemaker::UnoType::Sort::Char: - case codemaker::UnoType::Sort::Enum: - wrap = true; - break; - case codemaker::UnoType::Sort::String: - case codemaker::UnoType::Sort::Type: - case codemaker::UnoType::Sort::Any: - case codemaker::UnoType::Sort::Sequence: - case codemaker::UnoType::Sort::PlainStruct: - case codemaker::UnoType::Sort::InstantiatedPolymorphicStruct: - case codemaker::UnoType::Sort::Interface: - break; - default: - throw CannotDumpException("unexpected entity \"" + param.type - + "\" as parameter type"); - } - } if (declarations) { - if (wrap) + if (param.direction != unoidl::InterfaceTypeEntity::Method::Parameter::DIRECTION_IN) { out << "::unoembindhelpers::UnoInOutParam<"; } dumpType(out, manager, param.type); - if (wrap) - { - out << ">"; - } if (param.direction == unoidl::InterfaceTypeEntity::Method::Parameter::DIRECTION_IN) { if (passByReference(manager, param.type)) @@ -610,17 +574,13 @@ void dumpParameters(std::ostream& out, rtl::Reference const& manage } else { - out << " *"; + out << "> *"; } out << " "; } - else if (param.direction != unoidl::InterfaceTypeEntity::Method::Parameter::DIRECTION_IN - && !wrap) - { - out << "*"; - } out << param.name; - if (!declarations && wrap) + if (!declarations + && param.direction != unoidl::InterfaceTypeEntity::Method::Parameter::DIRECTION_IN) { out << "->value"; } @@ -872,6 +832,13 @@ void recordSequenceTypes(rtl::Reference const& manager, OUString co } } +void recordInOutParameterType(rtl::Reference const& manager, OUString const& type, + std::set& inOutParameters) +{ + auto const res = resolveAllTypedefs(manager, type); + inOutParameters.insert(res); +} + void writeJsMap(std::ostream& out, Module const& module, std::string const& prefix) { auto comma = false; @@ -1041,6 +1008,7 @@ SAL_IMPLEMENT_MAIN() recordSequenceTypes(mgr, mem.type, sequences); } } + std::set inOutParams; for (auto const& ifc : interfaces) { auto const ent = mgr->getManager()->findEntity(ifc); @@ -1113,6 +1081,11 @@ SAL_IMPLEMENT_MAIN() for (auto const& param : meth.parameters) { recordSequenceTypes(mgr, param.type, sequences); + if (param.direction + != unoidl::InterfaceTypeEntity::Method::Parameter::DIRECTION_IN) + { + recordInOutParameterType(mgr, param.type, inOutParams); + } } recordSequenceTypes(mgr, meth.returnType, sequences); } @@ -1160,6 +1133,24 @@ SAL_IMPLEMENT_MAIN() } cppOut << "_" << jsName(nuc) << "\");\n"; } + for (auto const& par : inOutParams) + { + cppOut << " ::unoembindhelpers::registerInOutParameter<"; + dumpType(cppOut, mgr, par); + cppOut << ">(\"uno_InOutParam_"; + sal_Int32 k; + auto const nuc = b2u(codemaker::UnoType::decompose(u2b(par), &k)); + if (k >= 1) + { + cppOut << "sequence"; + if (k > 1) + { + cppOut << k; + } + cppOut << "_"; + } + cppOut << jsName(nuc.replace(' ', '_')) << "\");\n"; + } cppOut << "}\n"; cppOut.close(); if (!cppOut) diff --git a/static/source/unoembindhelpers/PrimaryBindings.cxx b/static/source/unoembindhelpers/PrimaryBindings.cxx index 6ae2e68323dc..4972699178c8 100644 --- a/static/source/unoembindhelpers/PrimaryBindings.cxx +++ b/static/source/unoembindhelpers/PrimaryBindings.cxx @@ -158,12 +158,6 @@ void copyStruct(typelib_CompoundTypeDescription* desc, void const* source, void* } } -template void registerInOutParam(char const* name) -{ - class_>(name).constructor().constructor().property( - "val", &unoembindhelpers::UnoInOutParam::get, &unoembindhelpers::UnoInOutParam::set); -} - Reference getCurrentModelFromViewSh() { SfxViewShell* pSh = nullptr; @@ -386,18 +380,6 @@ EMSCRIPTEN_BINDINGS(PrimaryBindings) }; }); - registerInOutParam("uno_InOutParam_boolean"); - registerInOutParam("uno_InOutParam_byte"); - registerInOutParam("uno_InOutParam_short"); - registerInOutParam("uno_InOutParam_unsigned_short"); - registerInOutParam("uno_InOutParam_long"); - registerInOutParam("uno_InOutParam_unsigned_long"); - registerInOutParam("uno_InOutParam_hyper"); - registerInOutParam("uno_InOutParam_unsigned_hyper"); - registerInOutParam("uno_InOutParam_float"); - registerInOutParam("uno_InOutParam_double"); - registerInOutParam("uno_InOutParam_char"); - function("getCurrentModelFromViewSh", &getCurrentModelFromViewSh); function("getUnoComponentContext", &comphelper::getProcessComponentContext); function("throwUnoException", +[](css::uno::Type const& type, emscripten::val const& value) { diff --git a/unotest/source/embindtest/embindtest.cxx b/unotest/source/embindtest/embindtest.cxx index bbc3496a7238..c5fcea1dc42e 100644 --- a/unotest/source/embindtest/embindtest.cxx +++ b/unotest/source/embindtest/embindtest.cxx @@ -514,8 +514,12 @@ class Test : public cppu::WeakImplHelper void SAL_CALL getOut(sal_Bool& value1, sal_Int8& value2, sal_Int16& value3, sal_uInt16& value4, sal_Int32& value5, sal_uInt32& value6, sal_Int64& value7, - sal_uInt64& value8, float& value9, double& value10, - sal_Unicode& value11) override + sal_uInt64& value8, float& value9, double& value10, sal_Unicode& value11, + OUString& value12, css::uno::Type& value13, css::uno::Any& value14, + css::uno::Sequence& value15, + org::libreoffice::embindtest::Enum& value16, + org::libreoffice::embindtest::Struct& value17, + css::uno::Reference& value18) override { value1 = true; value2 = -12; @@ -528,6 +532,13 @@ class Test : public cppu::WeakImplHelper value9 = -10.25; value10 = 100.5; value11 = u'Ö'; + value12 = u"hä"_ustr; + value13 = cppu::UnoType::get(); + value14 = css::uno::Any(sal_Int32(-123456)); + value15 = { u"foo"_ustr, u"barr"_ustr, u"bazzz"_ustr }; + value16 = org::libreoffice::embindtest::Enum_E_2; + value17 = { -123456, 100.5, u"hä"_ustr }; + value18 = this; } void SAL_CALL throwRuntimeException() override diff --git a/unotest/source/embindtest/embindtest.js b/unotest/source/embindtest/embindtest.js index e4dccefa6537..16efe9acf169 100644 --- a/unotest/source/embindtest/embindtest.js +++ b/unotest/source/embindtest/embindtest.js @@ -540,7 +540,15 @@ Module.addOnPostRun(function() { const v9 = new Module.uno_InOutParam_float; const v10 = new Module.uno_InOutParam_double; const v11 = new Module.uno_InOutParam_char; - test.getOut(v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11); + const v12 = new Module.uno_InOutParam_string; + const v13 = new Module.uno_InOutParam_type; + const v14 = new Module.uno_InOutParam_any; + const v15 = new Module.uno_InOutParam_sequence_string; + const v16 = new Module.uno_InOutParam_org$libreoffice$embindtest$Enum; + const v17 = new Module.uno_InOutParam_org$libreoffice$embindtest$Struct; + const v18 = new Module.uno_InOutParam_org$libreoffice$embindtest$XTest; + test.getOut( + v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, v17, v18); console.log(v1.val); console.log(v2.val); console.log(v3.val); @@ -552,6 +560,13 @@ Module.addOnPostRun(function() { console.log(v9.val); console.log(v10.val); console.log(v11.val); + console.log(v12.val); + console.log(v13.val); + console.log(v14.val); + console.log(v15.val); + console.log(v16.val); + console.log(v17.val); + console.log(v18.val); console.assert(v1.val === 1); //TODO: true console.assert(v2.val === -12); console.assert(v3.val === -1234); @@ -563,6 +578,18 @@ Module.addOnPostRun(function() { console.assert(v9.val === -10.25); console.assert(v10.val === 100.5); console.assert(v11.val === 'Ö'); + console.assert(v12.val === 'hä'); + console.assert(v13.val.toString() === 'long'); + console.assert(v14.val.get() === -123456) + console.assert(v15.val.size() === 3); + console.assert(v15.val.get(0) === 'foo'); + console.assert(v15.val.get(1) === 'barr'); + console.assert(v15.val.get(2) === 'bazzz'); + console.assert(v16.val === uno.org.libreoffice.embindtest.Enum.E_2); + console.assert(v17.val.m1 === -123456); + console.assert(v17.val.m2 === 100.5); + console.assert(v17.val.m3 === 'hä'); + console.assert(Module.sameUnoObject(v18.val, test)); v1.delete(); v2.delete(); v3.delete(); @@ -574,6 +601,15 @@ Module.addOnPostRun(function() { v9.delete(); v10.delete(); v11.delete(); + v12.delete(); + v13.delete(); + v14.val.delete(); + v14.delete(); + v15.val.delete(); + v15.delete(); + v16.delete(); + v17.delete(); + v18.delete(); } console.assert(uno.org.libreoffice.embindtest.Constants.Boolean === true); console.assert(test.isBoolean(uno.org.libreoffice.embindtest.Constants.Boolean));