Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Mon, Sep 05, 2011 at 09:57:30AM +0200, Stephan Bergmann wrote: On Sep 2, 2011, at 5:13 PM, Lionel Elie Mamane wrote: I assume the vtable obviously has to be exported. No, need not be exported (if none of the ctors and dtors are inline). Ah, I had not declared the copy constructor and assignment operator well, which led to the vtable being necessary to export. That is fixed now. Revised patch attached; if it is good to go (especially with respect to questions above), I'll commit (with a better commit message). gcc3.map in the latest patch (sent this morning): - Do not export the thunks (_ZThn), not needed externally. Well, the build fails if they are not exported. So for now, I export them. If the fact that they are needed comes from an error in the class design, please let me know what to do to fix that. Here's the failing build log: = (1/1) Building module comphelper = Entering /home/master/src/libreoffice/core/comphelper/prj cd .. make -s -r -j4 make -s -r deliverlog [ build CXX ] comphelper/source/property/opropertybag [ build CXX ] comphelper/source/property/propagg [ build CXX ] comphelper/source/property/propertybag [ build CXX ] comphelper/source/property/propertycontainer [ build CXX ] comphelper/source/property/propertycontainerhelper [ build CXX ] comphelper/source/property/propertystatecontainer [ build CXX ] comphelper/source/property/propmultiplex [ build CXX ] comphelper/source/property/propstate [ build DEP ] LNK:Library/libcomphelpgcc3.so [ build LNK ] Library/libcomphelpgcc3.so /home/master/src/libreoffice/core/workdir/unxlngx6/CxxObject/comphelper/source/property/opropertybag.o:(.data.rel.ro._ZTVN10comphelper12OPropertyBagE[vtable for comphelper::OPropertyBag]+0x270): undefined reference to `non-virtual thunk to cppu::OPropertySetHelper2::enableChangeListenerNotification(unsigned char)' /home/master/src/libreoffice/core/workdir/unxlngx6/CxxObject/comphelper/source/property/propagg.o:(.data.rel.ro._ZTVN10comphelper29OPropertySetAggregationHelperE[vtable for comphelper::OPropertySetAggregationHelper]+0x200): undefined reference to `non-virtual thunk to cppu::OPropertySetHelper2::enableChangeListenerNotification(unsigned char)' /home/master/src/libreoffice/core/workdir/unxlngx6/CxxObject/comphelper/source/property/propstate.o:(.data.rel.ro._ZTVN10comphelper20OStatefulPropertySetE[vtable for comphelper::OStatefulPropertySet]+0x258): undefined reference to `non-virtual thunk to cppu::OPropertySetHelper2::enableChangeListenerNotification(unsigned char)' /home/master/src/libreoffice/core/workdir/unxlngx6/CxxObject/comphelper/source/property/propstate.o:(.data.rel.ro._ZTVN10comphelper20OPropertyStateHelperE[vtable for comphelper::OPropertyStateHelper]+0x1d0): undefined reference to `non-virtual thunk to cppu::OPropertySetHelper2::enableChangeListenerNotification(unsigned char)' collect2: ld returned 1 exit status make: *** [/home/master/src/libreoffice/core/workdir/unxlngx6/LinkTarget/Library/libcomphelpgcc3.so] Error 1 dmake: Error code 2, while making 'all' -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On 09/07/2011 12:14 PM, Lionel Elie Mamane wrote: On Mon, Sep 05, 2011 at 09:57:30AM +0200, Stephan Bergmann wrote: On Sep 2, 2011, at 5:13 PM, Lionel Elie Mamane wrote: I assume the vtable obviously has to be exported. No, need not be exported (if none of the ctors and dtors are inline). Ah, I had not declared the copy constructor and assignment operator well, which led to the vtable being necessary to export. That is fixed now. good Revised patch attached; if it is good to go (especially with respect to questions above), I'll commit (with a better commit message). gcc3.map in the latest patch (sent this morning): - Do not export the thunks (_ZThn), not needed externally. Well, the build fails if they are not exported. So for now, I export them. If the fact that they are needed comes from an error in the class design, please let me know what to do to fix that. Hm, had *thought* they were not really needed, and had run a small test case to verify, but it seems there are nevertheless cases where they *are* needed. If you have no inline functions left (neither implicit nor explicit ones, incl. default functions provided by the compiler), then poor class design cannot be the reason. -- Just export them, I would say. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Mon, Sep 05, 2011 at 09:57:30AM +0200, Stephan Bergmann wrote: - For the question of which symbols for OPropertySetHelper2 to list in the various map files, see http://udk.openoffice.org/common/man/apicppclasses.html (you probably need access to builds on the various platforms to find out the correct mangled names). gcc3.map in the latest patch (sent this morning): - Do not export the thunks (_ZThn), not needed externally. They are exported for just about every other class in that file; that is then also a historical error? -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On 09/06/2011 06:39 PM, Lionel Elie Mamane wrote: On Mon, Sep 05, 2011 at 09:57:30AM +0200, Stephan Bergmann wrote: - For the question of which symbols for OPropertySetHelper2 to list in the various map files, see http://udk.openoffice.org/common/man/apicppclasses.html (you probably need access to builds on the various platforms to find out the correct mangled names). gcc3.map in the latest patch (sent this morning): - Do not export the thunks (_ZThn), not needed externally. They are exported for just about every other class in that file; that is then also a historical error? Noticed that too. Yes, either historical error or due to something being inline that should not be. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Fri, Sep 02, 2011 at 05:13:36PM +0200, Lionel Elie Mamane wrote: Revised patch attached; if it is good to go (especially with respect to questions above), I'll commit (with a better commit message). Sorry, sent wrong version of patch (forgot to git add). Here is the right one. -- Lionel From 1b333a62eb61b9bab8baebc91981e7fc7f33534f Mon Sep 17 00:00:00 2001 From: Lionel Elie Mamane lio...@mamane.lu Date: Fri, 2 Sep 2011 14:57:30 +0200 Subject: [PATCH] TMP: in-progress-ABI-breakage-fix --- comphelper/inc/comphelper/propstate.hxx |4 +- comphelper/source/property/propstate.cxx |4 +- cppuhelper/inc/cppuhelper/propshlp.hxx | 67 ++ cppuhelper/source/gcc3.map | 22 - cppuhelper/source/msvc_win32_intel.map |5 +- cppuhelper/source/propshlp.cxx | 42 +-- 6 files changed, 105 insertions(+), 39 deletions(-) diff --git a/comphelper/inc/comphelper/propstate.hxx b/comphelper/inc/comphelper/propstate.hxx index 2c8f06f..b99f75f 100644 --- a/comphelper/inc/comphelper/propstate.hxx +++ b/comphelper/inc/comphelper/propstate.hxx @@ -54,11 +54,11 @@ namespace comphelper //= OPropertyStateHelper //== /// helper class for implementing property states -class COMPHELPER_DLLPUBLIC OPropertyStateHelper :public ::cppu::OPropertySetHelper +class COMPHELPER_DLLPUBLIC OPropertyStateHelper :public ::cppu::OPropertySetHelper2 ,public ::com::sun::star::beans::XPropertyState { public: -OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp):OPropertySetHelper(rBHlp) { } +OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp):OPropertySetHelper2(rBHlp) { } OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp, ::cppu::IEventNotificationHook *i_pFireEvents); diff --git a/comphelper/source/property/propstate.cxx b/comphelper/source/property/propstate.cxx index ec621cb..3859f6a 100644 --- a/comphelper/source/property/propstate.cxx +++ b/comphelper/source/property/propstate.cxx @@ -57,7 +57,7 @@ namespace comphelper //- ::com::sun::star::uno::Any SAL_CALL OPropertyStateHelper::queryInterface(const ::com::sun::star::uno::Type _rType) throw( ::com::sun::star::uno::RuntimeException) { -::com::sun::star::uno::Any aReturn = OPropertySetHelper::queryInterface(_rType); +::com::sun::star::uno::Any aReturn = OPropertySetHelper2::queryInterface(_rType); // our own ifaces if ( !aReturn.hasValue() ) aReturn = ::cppu::queryInterface(_rType, static_cast ::com::sun::star::beans::XPropertyState*(this)); @@ -82,7 +82,7 @@ namespace comphelper OPropertyStateHelper::OPropertyStateHelper( ::cppu::OBroadcastHelper rBHlp, ::cppu::IEventNotificationHook *i_pFireEvents) -: ::cppu::OPropertySetHelper(rBHlp, i_pFireEvents) { } +: ::cppu::OPropertySetHelper2(rBHlp, i_pFireEvents) { } OPropertyStateHelper::~OPropertyStateHelper() {} diff --git a/cppuhelper/inc/cppuhelper/propshlp.hxx b/cppuhelper/inc/cppuhelper/propshlp.hxx index 670ce03..f8237da 100644 --- a/cppuhelper/inc/cppuhelper/propshlp.hxx +++ b/cppuhelper/inc/cppuhelper/propshlp.hxx @@ -351,8 +351,7 @@ public: */ class OPropertySetHelper : public ::com::sun::star::beans::XMultiPropertySet, public ::com::sun::star::beans::XFastPropertySet, - public ::com::sun::star::beans::XPropertySet, - public ::com::sun::star::beans::XPropertySetOption + public ::com::sun::star::beans::XPropertySet { public: /** @@ -372,11 +371,12 @@ public: of this object. Stored in the variable rBHelper. @param bIgnoreRuntimeExceptionsWhileFiring -indicates whether occuring RuntimeExceptions will be -ignored when firing notifications (vetoableChange((), -propertyChange()) to listeners. +indicates whether occurring RuntimeExceptions will be +ignored when firing notifications +(vetoableChange(), propertyChange()) +to listeners. PropertyVetoExceptions may still be thrown. -This flag is useful in a inter-process scenarios when +This flag is useful in an inter-process scenario when remote bridges may break down (firing DisposedExceptions). */ @@ -395,11 +395,12 @@ public: additional event notifier @param bIgnoreRuntimeExceptionsWhileFiring -indicates whether
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Sep 2, 2011, at 5:13 PM, Lionel Elie Mamane wrote: - For the question of which symbols for OPropertySetHelper2 to list in the various map files, see http://udk.openoffice.org/common/man/apicppclasses.html (you probably need access to builds on the various platforms to find out the correct mangled names). It does not say to export the vtable; I assume the vtable obviously has to be exported. No, need not be exported (if none of the ctors and dtors are inline). It says: If the destructor of a class is declared virtual, declare the destructors of all its base classes virtual. (To have a dedicated place where RTTI for the base classes is generated, which is referenced by the RTTI for the derived class. Since in general you cannot control the ways a class is used, the best advice probably is to do this for each class that is intended to be subclassed.) Well, we cannot do that anymore, since that would break ABI on OPropertySetHelper2. Can we safely ignore this instruction here? Yes. Revised patch attached; if it is good to go (especially with respect to questions above), I'll commit (with a better commit message). gcc3.map in the latest patch (sent this morning): - Do not export vtable (see above). - Comment out the RTTI symbols (_ZTI, _ZTS), as they are already implicitly exported in GCC_3_0_0 section via some hackery. (This implies that any _ZTI and _ZTS symbols covered by the previous point are always exported with version GCC_3_0_0, which defeats the correct use of versioning for those symbols.) - Do not export the thunks (_ZThn), not needed externally. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Thu, Aug 25, 2011 at 10:10:16PM +0200, Stephan Bergmann wrote: A number of comments on the patch: - The OPropertySetHelper2 ctor(s) should not be inline. (See how eventually making use of OPropertySetHelper::m_pReserved would not have worked if OPropertySetHelper's ctors had been inline.) Yes, good catch. - For the question of which symbols for OPropertySetHelper2 to list in the various map files, see http://udk.openoffice.org/common/man/apicppclasses.html (you probably need access to builds on the various platforms to find out the correct mangled names). It does not say to export the vtable; I assume the vtable obviously has to be exported. It says: If the destructor of a class is declared virtual, declare the destructors of all its base classes virtual. (To have a dedicated place where RTTI for the base classes is generated, which is referenced by the RTTI for the derived class. Since in general you cannot control the ways a class is used, the best advice probably is to do this for each class that is intended to be subclassed.) Well, we cannot do that anymore, since that would break ABI on OPropertySetHelper2. Can we safely ignore this instruction here? Revised patch attached; if it is good to go (especially with respect to questions above), I'll commit (with a better commit message). -- Lionel From 87dcabb49bac0c39a9af111562d40f4995099ca5 Mon Sep 17 00:00:00 2001 From: Lionel Elie Mamane lio...@mamane.lu Date: Fri, 2 Sep 2011 14:57:30 +0200 Subject: [PATCH] TMP: in-progress-ABI-breakage-fix --- comphelper/inc/comphelper/propstate.hxx |4 +- comphelper/source/property/propstate.cxx |4 +- cppuhelper/inc/cppuhelper/propshlp.hxx | 111 +++--- cppuhelper/source/gcc3.map |9 ++- cppuhelper/source/msvc_win32_intel.map |7 +- cppuhelper/source/propshlp.cxx | 34 ++ 6 files changed, 136 insertions(+), 33 deletions(-) diff --git a/comphelper/inc/comphelper/propstate.hxx b/comphelper/inc/comphelper/propstate.hxx index 2c8f06f..b99f75f 100644 --- a/comphelper/inc/comphelper/propstate.hxx +++ b/comphelper/inc/comphelper/propstate.hxx @@ -54,11 +54,11 @@ namespace comphelper //= OPropertyStateHelper //== /// helper class for implementing property states -class COMPHELPER_DLLPUBLIC OPropertyStateHelper :public ::cppu::OPropertySetHelper +class COMPHELPER_DLLPUBLIC OPropertyStateHelper :public ::cppu::OPropertySetHelper2 ,public ::com::sun::star::beans::XPropertyState { public: -OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp):OPropertySetHelper(rBHlp) { } +OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp):OPropertySetHelper2(rBHlp) { } OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp, ::cppu::IEventNotificationHook *i_pFireEvents); diff --git a/comphelper/source/property/propstate.cxx b/comphelper/source/property/propstate.cxx index ec621cb..3859f6a 100644 --- a/comphelper/source/property/propstate.cxx +++ b/comphelper/source/property/propstate.cxx @@ -57,7 +57,7 @@ namespace comphelper //- ::com::sun::star::uno::Any SAL_CALL OPropertyStateHelper::queryInterface(const ::com::sun::star::uno::Type _rType) throw( ::com::sun::star::uno::RuntimeException) { -::com::sun::star::uno::Any aReturn = OPropertySetHelper::queryInterface(_rType); +::com::sun::star::uno::Any aReturn = OPropertySetHelper2::queryInterface(_rType); // our own ifaces if ( !aReturn.hasValue() ) aReturn = ::cppu::queryInterface(_rType, static_cast ::com::sun::star::beans::XPropertyState*(this)); @@ -82,7 +82,7 @@ namespace comphelper OPropertyStateHelper::OPropertyStateHelper( ::cppu::OBroadcastHelper rBHlp, ::cppu::IEventNotificationHook *i_pFireEvents) -: ::cppu::OPropertySetHelper(rBHlp, i_pFireEvents) { } +: ::cppu::OPropertySetHelper2(rBHlp, i_pFireEvents) { } OPropertyStateHelper::~OPropertyStateHelper() {} diff --git a/cppuhelper/inc/cppuhelper/propshlp.hxx b/cppuhelper/inc/cppuhelper/propshlp.hxx index 670ce03..737aec9 100644 --- a/cppuhelper/inc/cppuhelper/propshlp.hxx +++ b/cppuhelper/inc/cppuhelper/propshlp.hxx @@ -351,8 +351,7 @@ public: */ class OPropertySetHelper : public ::com::sun::star::beans::XMultiPropertySet, public ::com::sun::star::beans::XFastPropertySet, - public ::com::sun::star::beans::XPropertySet, - public ::com::sun::star::beans::XPropertySetOption + public ::com::sun::star::beans::XPropertySet { public: /** @@ -412,7 +411,7 @@ public: Only returns a reference to XMultiPropertySet,
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
2) for MSVC, I'm rather more disarmed; made some incomplete guesses. ?queryInterface@OPropertySetHelper@cppu@@W3AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; ?queryInterface@OPropertySetHelper@cppu@@W7AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; The undname command that comes with MSVC can be used to demangle MSVC++ identifiers; these two are: [thunk]:public: virtual class com::sun::star::uno::Any __cdecl cppu::OPropertySetHelper::queryInterface`adjustor{4}' (class com::sun::star::uno::Type const ) [thunk]:public: virtual class com::sun::star::uno::Any __cdecl cppu::OPropertySetHelper::queryInterface`adjustor{8}' (class com::sun::star::uno::Type const ) but what that means I have no idea;) It also seems to me that the msvc_win32_x86_64.map is out of sync: it has no UDK_3.7, nor UDK_3.8 section; this looks highly suspicious. If someone is willing to take a look at it and fix it? Whenever somebody (I, or somebody else) starts looking at the 64-bit MSVC port again... In principle solenv/bin/remangle32to64.pl can be used to produce a .map file for x64 MSVC from that for x86 MSVC. --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Aug 25, 2011, at 11:00 PM, Lionel Elie Mamane wrote: On Thu, Aug 25, 2011 at 10:10:16PM +0200, Stephan Bergmann wrote: On Aug 25, 2011, at 6:09 PM, Lionel Elie Mamane wrote: On Tue, Aug 23, 2011 at 07:37:06PM +0200, Stephan Bergmann wrote: On Aug 23, 2011, at 5:00 PM, Lionel Elie Mamane wrote: - There is no reason for OPropertySetHelper2 to have a non-virtual dtor. (And thus no reason to disable warnings.) Why does OPropertySetHelper have a non-virtual dtor, actually? A historic mistake (that could not be corrected without breaking compatibility). - The OPropertySetHelper::queryInterface symbol must not be removed from gcc3.map. It was there in duplicate, I remove it only once. It is right there, three lines up from the one removed. Ah, missed that. - For the question of which symbols for OPropertySetHelper2 to list in the various map files, see http://udk.openoffice.org/common/man/apicppclasses.html (you probably need access to builds on the various platforms to find out the correct mangled names). OK, thanks will read. As to access to builds on the various platforms, that's rather optimistic… Yes, sure. As Michael already wrote, modus operandi is that somebody with access to any given platform helps out. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Aug 26, 2011, at 8:02 AM, Tor Lillqvist wrote: 2) for MSVC, I'm rather more disarmed; made some incomplete guesses. ?queryInterface@OPropertySetHelper@cppu@@W3AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; ?queryInterface@OPropertySetHelper@cppu@@W7AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; The undname command that comes with MSVC can be used to demangle MSVC++ identifiers; these two are: [thunk]:public: virtual class com::sun::star::uno::Any __cdecl cppu::OPropertySetHelper::queryInterface`adjustor{4}' (class com::sun::star::uno::Type const ) [thunk]:public: virtual class com::sun::star::uno::Any __cdecl cppu::OPropertySetHelper::queryInterface`adjustor{8}' (class com::sun::star::uno::Type const ) but what that means I have no idea;) For a class with multiple base classes, there is generally demand to adjust the this-pointer when calling a virtual function of the derived class from within a function of one of the base classes. Thats what these thunks are for. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Tue, Aug 23, 2011 at 07:37:06PM +0200, Stephan Bergmann wrote: On Aug 23, 2011, at 5:00 PM, Lionel Elie Mamane wrote: (...) I don't know what to do with queryInterface; since it is not virtual, I cannot override it in OPropertySetHelper2. The best I could come up with is something like (in OPropertySetHelper): queryInterface *is* virtual (inherited from com::sun::star::uno::XInterface), Ah yes, I was looking only at the class definition, and I forgot it is inherited from the purely abstract interface class anyway... So, that is solved cleanly. Now, I face the .map files; there, I'd appreciate some help; I think I've reverse-engineered enough of the GCC one to get a correct one (at least, I could do a modicum of testing with GCC), but: 1) Someone that knows better than me should check. 2) for MSVC, I'm rather more disarmed; made some incomplete guesses. You'll notice I remove: ?queryInterface@OPropertySetHelper@cppu@@W3AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; ?queryInterface@OPropertySetHelper@cppu@@W7AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; That's because those *look* like they are rests from a different calling convention or something like that, since all other queryInterface member functions (of the other classes) have only the @UAA version insted of @W3AA and @W7AA. We shouldn't commit that hunk until we confirm this is true :) (And it should be in a different git commit than the rest anyway). It also seems to me that the msvc_win32_x86_64.map is out of sync: it has no UDK_3.7, nor UDK_3.8 section; this looks highly suspicious. If someone is willing to take a look at it and fix it? -- Lionel diff --git a/comphelper/inc/comphelper/propstate.hxx b/comphelper/inc/comphelper/propstate.hxx index 2c8f06f..b99f75f 100644 --- a/comphelper/inc/comphelper/propstate.hxx +++ b/comphelper/inc/comphelper/propstate.hxx @@ -54,11 +54,11 @@ namespace comphelper //= OPropertyStateHelper //== /// helper class for implementing property states -class COMPHELPER_DLLPUBLIC OPropertyStateHelper :public ::cppu::OPropertySetHelper +class COMPHELPER_DLLPUBLIC OPropertyStateHelper :public ::cppu::OPropertySetHelper2 ,public ::com::sun::star::beans::XPropertyState { public: -OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp):OPropertySetHelper(rBHlp) { } +OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp):OPropertySetHelper2(rBHlp) { } OPropertyStateHelper(::cppu::OBroadcastHelper rBHlp, ::cppu::IEventNotificationHook *i_pFireEvents); diff --git a/comphelper/source/property/propstate.cxx b/comphelper/source/property/propstate.cxx index ec621cb..3859f6a 100644 --- a/comphelper/source/property/propstate.cxx +++ b/comphelper/source/property/propstate.cxx @@ -57,7 +57,7 @@ namespace comphelper //- ::com::sun::star::uno::Any SAL_CALL OPropertyStateHelper::queryInterface(const ::com::sun::star::uno::Type _rType) throw( ::com::sun::star::uno::RuntimeException) { -::com::sun::star::uno::Any aReturn = OPropertySetHelper::queryInterface(_rType); +::com::sun::star::uno::Any aReturn = OPropertySetHelper2::queryInterface(_rType); // our own ifaces if ( !aReturn.hasValue() ) aReturn = ::cppu::queryInterface(_rType, static_cast ::com::sun::star::beans::XPropertyState*(this)); @@ -82,7 +82,7 @@ namespace comphelper OPropertyStateHelper::OPropertyStateHelper( ::cppu::OBroadcastHelper rBHlp, ::cppu::IEventNotificationHook *i_pFireEvents) -: ::cppu::OPropertySetHelper(rBHlp, i_pFireEvents) { } +: ::cppu::OPropertySetHelper2(rBHlp, i_pFireEvents) { } OPropertyStateHelper::~OPropertyStateHelper() {} diff --git a/cppuhelper/inc/cppuhelper/propshlp.hxx b/cppuhelper/inc/cppuhelper/propshlp.hxx index 670ce03..737aec9 100644 --- a/cppuhelper/inc/cppuhelper/propshlp.hxx +++ b/cppuhelper/inc/cppuhelper/propshlp.hxx @@ -351,8 +351,7 @@ public: */ class OPropertySetHelper : public ::com::sun::star::beans::XMultiPropertySet, public ::com::sun::star::beans::XFastPropertySet, - public ::com::sun::star::beans::XPropertySet, - public ::com::sun::star::beans::XPropertySetOption + public ::com::sun::star::beans::XPropertySet { public: /** @@ -412,7 +411,7 @@ public: Only returns a reference to XMultiPropertySet, XFastPropertySet, XPropertySet and XEventListener. */ -::com::sun::star::uno::Any SAL_CALL queryInterface( const ::com::sun::star::uno::Type rType ) +virtual ::com::sun::star::uno::Any SAL_CALL queryInterface( const ::com::sun::star::uno::Type rType ) throw
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
Hi Lionel, On Thu, 2011-08-25 at 18:09 +0200, Lionel Elie Mamane wrote: Now, I face the .map files; there, I'd appreciate some help; I think I've reverse-engineered enough of the GCC one to get a correct one (at least, I could do a modicum of testing with GCC), but: Um ;-) I havn't been following the rest of the change, but when it comes to these map files - the general approach is to blame the madness that created these [ we need to drop them in libreoffice 4.0 in favour of uniform visibility markup I think ]. Then you check in the version that works with gcc - and (as if by magic) some toiling MSVC++ / Windows user (and/or Mac user) finds the problem and expands the hole through the map file for their platform. 1) Someone that knows better than me should check. oh - and if you're polite you ping the list / mail a known windows / mac user ask them to fix it :-) Hopefully, the tinderboxes will help catch this stuff quickly too :-) HTH, Michael. -- michael.me...@novell.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Aug 25, 2011, at 6:09 PM, Lionel Elie Mamane wrote: On Tue, Aug 23, 2011 at 07:37:06PM +0200, Stephan Bergmann wrote: On Aug 23, 2011, at 5:00 PM, Lionel Elie Mamane wrote: (...) I don't know what to do with queryInterface; since it is not virtual, I cannot override it in OPropertySetHelper2. The best I could come up with is something like (in OPropertySetHelper): queryInterface *is* virtual (inherited from com::sun::star::uno::XInterface), Ah yes, I was looking only at the class definition, and I forgot it is inherited from the purely abstract interface class anyway... So, that is solved cleanly. A number of comments on the patch: - You might or might not want to place OPropertySetHelper2 into a .hxx (and .cxx) file of its own, or keep it in propshlp.hxx. Both solutions have pros and cons. - It is good style to mark those ctors of OPropertySetHelper2 that can be called with one argument as explicit (even if this has not been done for OPropertySetHelper). - You might want to combine the three ctors of OPropertySetHelper2 into a single one, with i_pFireEvents defaulting to null and bIgnoreRuntimeExceptionsWhileFiring defaulting to false. - For DRY-nes, I would try to avoid copying the documentation (including the typographical and grammatical errors) of the OPropertySetHelper ctor parameters for the OPropertySetHelper2 ctors. - The OPropertySetHelper2 ctor(s) should not be inline. (See how eventually making use of OPropertySetHelper::m_pReserved would not have worked if OPropertySetHelper's ctors had been inline.) - There is no reason for OPropertySetHelper2 to have a non-virtual dtor. (And thus no reason to disable warnings.) - The OPropertySetHelper::queryInterface symbol must not be removed from gcc3.map. - For the question of which symbols for OPropertySetHelper2 to list in the various map files, see http://udk.openoffice.org/common/man/apicppclasses.html (you probably need access to builds on the various platforms to find out the correct mangled names). Now, I face the .map files; there, I'd appreciate some help; I think I've reverse-engineered enough of the GCC one to get a correct one (at least, I could do a modicum of testing with GCC), but: 1) Someone that knows better than me should check. 2) for MSVC, I'm rather more disarmed; made some incomplete guesses. You'll notice I remove: ?queryInterface@OPropertySetHelper@cppu@@W3AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; ?queryInterface@OPropertySetHelper@cppu@@W7AA?AVAny@uno@star@sun@com@@ABVType@4567@@Z; That's because those *look* like they are rests from a different calling convention or something like that, since all other queryInterface member functions (of the other classes) have only the @UAA version insted of @W3AA and @W7AA. We shouldn't commit that hunk until we confirm this is true :) (And it should be in a different git commit than the rest anyway). When you have access to a Windows box with MS compiler, run those symbols through undname.exe to see what exactly they represent (and I doubt they are debris that can be removed). It also seems to me that the msvc_win32_x86_64.map is out of sync: it has no UDK_3.7, nor UDK_3.8 section; this looks highly suspicious. If someone is willing to take a look at it and fix it? Indeed looks unmaintained. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] map files: how to update? [was: cppu::OPropertySetHelper ABI backwards compatibility]
On Aug 25, 2011, at 9:42 PM, Michael Meeks wrote: On Thu, 2011-08-25 at 18:09 +0200, Lionel Elie Mamane wrote: Now, I face the .map files; there, I'd appreciate some help; I think I've reverse-engineered enough of the GCC one to get a correct one (at least, I could do a modicum of testing with GCC), but: Um ;-) I havn't been following the rest of the change, but when it comes to these map files - the general approach is to blame the madness that created these [ we need to drop them in libreoffice 4.0 in favour of uniform visibility markup I think ]. Symbol versioning does have its merits, so dropping it is not ideal (C/C++ visibility markup only covers the visibility aspect of map files, not the versioning aspect, at least for now). But, sure, manually maintained map files for C++ components are far from ideal as well… Then you check in the version that works with gcc - and (as if by magic) some toiling MSVC++ / Windows user (and/or Mac user) finds the problem and expands the hole through the map file for their platform. 1) Someone that knows better than me should check. oh - and if you're polite you ping the list / mail a known windows / mac user ask them to fix it :-) Hopefully, the tinderboxes will help catch this stuff quickly too :-) A problem is that missing symbols can go unnoticed as long as no client code needs them, only to bite an extension developer who happens to be the first to need the given symbol (and then will have to wait making available his extension until a version of LO has been released that fixes the omission). As already mentioned in this thread, see http://udk.openoffice.org/common/man/apicppclasses.html for details of what symbols to list in the map files. -Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice