Re: [Libreoffice] -Woverloaded-virtual
On Friday 25 of March 2011, Pierre-André Jacquod wrote: > Hello, > > On 03/25/2011 02:13 PM, Lubos Lunak wrote: > > On Friday 25 of March 2011, Caolán McNamara wrote: > >> argh!, I meant to say "then things are *not* too bad", not "*too* bad". > >> I mean that's far less that I would have feared, quite manageable. > > > > Ok. In that case, if there are no objections, I'll commit my patches. > > just a bet: binfilter was off in your try? Most probably, not sure. Binfilter is obsolete code anyway, isn't it? -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
Hello, On 03/25/2011 02:13 PM, Lubos Lunak wrote: On Friday 25 of March 2011, Caolán McNamara wrote: argh!, I meant to say "then things are *not* too bad", not "*too* bad". I mean that's far less that I would have feared, quite manageable. Ok. In that case, if there are no objections, I'll commit my patches. just a bet: binfilter was off in your try? So I have some warning more to handle, once I am finished with cleaning. And I am always +1 for all what shows potential problems. Else it bites you harder later... regards Pierre-André ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
Lubos Lunak wrote: > BTW the warnings in canvas are pretty ugly - it's a template class that > inherits from some of its template arguments and sometimes one of those is a > UNO interface that implements disposing(const > com::sun::star::lang::EventObject&), whereas the class itself implements > disposing(). Solving it by "using Base::disposing" doesn't work, since the > template doesn't always inherit from that UNO class. > Hm, don't see an immediate fix, too - problem is, WeakComponentImplHelperBase's disposing *needs* to be overridden, as per the implementation of that cppu helper, that's the way to catch the message and forward it to the aggregated class. In this case, though, the warning is not critical (although of course the naming sucks). Cheers, -- Thorsten pgpWBY2BW6tWK.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Friday 25 of March 2011, Caolán McNamara wrote: > On Fri, 2011-03-25 at 13:55 +0100, Lubos Lunak wrote: > > It should be the current full list, with duplicates removed (i.e. once > > per every source of the problem, not once per every time it's reported). > > Why should it be that bad? > > argh!, I meant to say "then things are *not* too bad", not "*too* bad". > I mean that's far less that I would have feared, quite manageable. Ok. In that case, if there are no objections, I'll commit my patches. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Fri, 2011-03-25 at 13:55 +0100, Lubos Lunak wrote: > It should be the current full list, with duplicates removed (i.e. once per > every source of the problem, not once per every time it's reported). Why > should it be that bad? argh!, I meant to say "then things are *not* too bad", not "*too* bad". I mean that's far less that I would have feared, quite manageable. C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Thursday 24 of March 2011, Caolán McNamara wrote: > On Thu, 2011-03-24 at 17:29 +0100, Lubos Lunak wrote: > > a list of warnings (duplicates removed). > > I don't want to enable the warning right now, since although I've > > already reduced the number of warnings, I don't want to enable this > > too soon. > > Is that the full list ?, or just part of it. If its the full list then > things are *too* bad. It should be the current full list, with duplicates removed (i.e. once per every source of the problem, not once per every time it's reported). Why should it be that bad? It's about 70 places, most of it localized, and if the design cannot be fixed, the warning can be always removed with the 'using' keyword (except for the template stuff in canvas, where it'll be a bit more complicated). -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Fri, 2011-03-25 at 05:44 +, Michael Meeks wrote: > if this is the only big thing blocking us turning on a very valuable > warning, I'd (personally) say we should just bite the bullet and > un-publish & tweak this interface. There's always a way. I see this warning first in comphelper and the accessibility base, so how about this ?, rather than inherit from the generic template that implements the XComponent::addEventListener inherit from one that doesn't, and then we can declare the two addEventListeners side by side in the subclass and gcc is happy that we're not cocking something up. C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Thu, 2011-03-24 at 20:51 +, Caolán McNamara wrote: > Following the usual naming scheme the XAccessibleEventBroadcaster > add/remove on should have been named > [add|remove]AccessibleEventListener in the first place, I guess we're > stuck with that one now. Hmm - are those interfaces published ? and if so why ? you'd have to be pretty barking to interact directly with our UNO a11y APIs outside the tree, rather than using IAcc2 / atk / Java remotely. So - it'd be a chunk of work of course; but if this is the only big thing blocking us turning on a very valuable warning, I'd (personally) say we should just bite the bullet and un-publish & tweak this interface. Then again, if this is only part of the problem, perhaps it is un-fixable, though we could (perhaps) use some 'orrible gcc-specific hack: #ifdef GCC #pragma GCC diagnostic ignored "-Woverloaded-virtual" #endif or something ? ;-) ATB, 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] -Woverloaded-virtual
On Thu, 2011-03-24 at 17:29 +0100, Lubos Lunak wrote: > a list of warnings (duplicates removed). > I don't want to enable the warning right now, since although I've > already reduced the number of warnings, I don't want to enable this > too soon. Is that the full list ?, or just part of it. If its the full list then things are *too* bad. > I have no idea if the XAccessibleEventListener vs XEventListener are > intentional or mistakes). Following the usual naming scheme the XAccessibleEventBroadcaster add/remove on should have been named [add|remove]AccessibleEventListener in the first place, I guess we're stuck with that one now. . ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Thursday 24 of March 2011, Michael Meeks wrote: > You know - seeing a lot of warnings has a focusing effect on the mind, > and helps people work on cleaning them up - assuming there are not a > bazillion duplicates of each of them (are there ?) :-) That's exactly the problem. There were tons of them initially (it's header files that trigger them), but my patch for fixing SdrObject::operator=() has cleaned up most of them. It's probably good enough to go now. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
On Thursday 24 of March 2011, Lubos Lunak wrote: > Attached is a patch for introducing the warning (quite obvious) and a list > of warnings (duplicates removed). I don't want to enable the warning right > now, since although I've already reduced the number of warnings, I don't > want to enable this too soon. If somebody sees something easy in the list > of warnings, feel free to fix it, some of them look pretty non-obvious to > me (e.g. I have no idea if the XAccessibleEventListener vs XEventListener > are intentional or mistakes). Actually, if somebody could check the warnings related to *.hdl files, I think it's good enough to be enabled. The remaining warnings are mostly confined to small places (lotuspro filter, canvas) or are small in number and can be fixed by whoever will feel like it. BTW the warnings in canvas are pretty ugly - it's a template class that inherits from some of its template arguments and sometimes one of those is a UNO interface that implements disposing(const com::sun::star::lang::EventObject&), whereas the class itself implements disposing(). Solving it by "using Base::disposing" doesn't work, since the template doesn't always inherit from that UNO class. -- Lubos Lunak l.lu...@suse.cz ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] -Woverloaded-virtual
Hi Lubos, On Thu, 2011-03-24 at 17:29 +0100, Lubos Lunak wrote: > I want to introduce the usage of the gcc -Woverloaded-virtual switch. The > switch warns about the following situation: Looks sexy to me :-) nice work ! > Attached is a patch for introducing the warning (quite obvious) and a list > of > warnings (duplicates removed). I don't want to enable the warning right now, > since although I've already reduced the number of warnings, I don't want to > enable this too soon. You know - seeing a lot of warnings has a focusing effect on the mind, and helps people work on cleaning them up - assuming there are not a bazillion duplicates of each of them (are there ?) :-) > As for the SampleICC warnings in libs-extern, I've already pointed this out > in their bug tool. They are a quite nice demonstration of how easy it is to > do a mistake. Yep; it would be great to be protected against this by spewing warnings - I suspect lots of our newly introduced warnings are signes of auto-merging problems anyway so ... Good stuff, Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
