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
