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