include/static/unoembindhelpers/PrimaryBindings.hxx |    6 +
 offapi/org/libreoffice/embindtest/XTest.idl         |    5 +
 static/source/embindmaker/embindmaker.cxx           |   79 ++++++++------------
 static/source/unoembindhelpers/PrimaryBindings.cxx  |   18 ----
 unotest/source/embindtest/embindtest.cxx            |   15 +++
 unotest/source/embindtest/embindtest.js             |   38 +++++++++
 6 files changed, 95 insertions(+), 66 deletions(-)

New commits:
commit 735ea444f2c15771736260fc78704f6b9132ceac
Author:     Stephan Bergmann <stephan.bergm...@allotropia.de>
AuthorDate: Mon Apr 29 14:20:05 2024 +0200
Commit:     Stephan Bergmann <stephan.bergm...@allotropia.de>
CommitDate: Tue Apr 30 07:53:11 2024 +0200

    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 <stephan.bergm...@allotropia.de>

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 <typename T> void registerSequence(char const* 
name)
         });
     registerUnoType<css::uno::Sequence<T>>();
 }
+
+template <typename T> void registerInOutParameter(char const* name)
+{
+    emscripten::class_<UnoInOutParam<T>>(name).constructor().template 
constructor<T>().property(
+        "val", &UnoInOutParam<T>::get, &UnoInOutParam<T>::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<string> 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<TypeManager> 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<TypeManager> 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<TypeManager> 
const& manager, OUString co
     }
 }
 
+void recordInOutParameterType(rtl::Reference<TypeManager> const& manager, 
OUString const& type,
+                              std::set<OUString>& 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<OUString> 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) << "\");
";
         }
+        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(' ', '_')) << "\");
";
+        }
         cppOut << "}
";
         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 <typename T> void registerInOutParam(char const* name)
-{
-    
class_<unoembindhelpers::UnoInOutParam<T>>(name).constructor().constructor<T>().property(
-        "val", &unoembindhelpers::UnoInOutParam<T>::get, 
&unoembindhelpers::UnoInOutParam<T>::set);
-}
-
 Reference<css::frame::XModel> getCurrentModelFromViewSh()
 {
     SfxViewShell* pSh = nullptr;
@@ -386,18 +380,6 @@ EMSCRIPTEN_BINDINGS(PrimaryBindings)
             };
         });
 
-    registerInOutParam<sal_Bool>("uno_InOutParam_boolean");
-    registerInOutParam<sal_Int8>("uno_InOutParam_byte");
-    registerInOutParam<sal_Int16>("uno_InOutParam_short");
-    registerInOutParam<sal_uInt16>("uno_InOutParam_unsigned_short");
-    registerInOutParam<sal_Int32>("uno_InOutParam_long");
-    registerInOutParam<sal_uInt32>("uno_InOutParam_unsigned_long");
-    registerInOutParam<sal_Int64>("uno_InOutParam_hyper");
-    registerInOutParam<sal_uInt64>("uno_InOutParam_unsigned_hyper");
-    registerInOutParam<float>("uno_InOutParam_float");
-    registerInOutParam<double>("uno_InOutParam_double");
-    registerInOutParam<sal_Unicode>("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<org::libreoffice::embindtest::XTest>
 
     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<OUString>& value15,
+                         org::libreoffice::embindtest::Enum& value16,
+                         org::libreoffice::embindtest::Struct& value17,
+                         
css::uno::Reference<org::libreoffice::embindtest::XTest>& value18) override
     {
         value1 = true;
         value2 = -12;
@@ -528,6 +532,13 @@ class Test : public 
cppu::WeakImplHelper<org::libreoffice::embindtest::XTest>
         value9 = -10.25;
         value10 = 100.5;
         value11 = u'Ö';
+        value12 = u"hä"_ustr;
+        value13 = cppu::UnoType<sal_Int32>::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));

Reply via email to