Re: [dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

Nikolai Pretzell wrote:

Hi Stephan and all,

The consensus back then was to keep -Wnon-virtual-dtor switched on 
globally, and only switch it off (together with all other warnings) 
within cppumaker-generated headers, on the grounds that 
-Wnon-virtual-dtor was considered a useful tool to find errors.


However, trying to compile 
cppuhelper/qa/propertysetmixin/comp_propertysetmixin.cxx, I fear we 
have to rethink the case:  cppu::PropertySetMixin is part of the 
stable API of OOo/URE, is a class template, and contains a non-virtual 
dtor and virtual functions.  Since it is part of the stable API, we 
cannot change its dtor to be virtual (and even if we could, one could 
argue that that would not be what one would want to do, but consider 
it a hack to work around the warning).  And since it is a template, we 
cannot disable the warning locally---the warning is emitted at the end 
of each compilation unit that includes 
cppuhelper/propertysetmixin.hxx, rather than from within 
cppuhelper/propertysetmixin.hxx itself.


The only solution I see is to bite the bullet and globally disable 
-Wnon-virtual-dtor.



Mhmm. Why not make the d'tor protected?
As for compatibility, there should be no binary difference.


The dtor is already protected.  The problem is that changing a dtor from 
non-virtual to virtual affects the vtable (with some compilers, at 
least), so that calls through the vtable could fail after such an 
incompatible change.


-Stephan

Only the use would be constrained, but if cppu::PropertySetMixin is 
indeed used as a mixin, then in most cases (whenever it is not the first 
base class or any other base class is virtual, may be even whenever it 
is not the only base class) calling its d'tor directly would be a bug 
anyway.

So maybe this does not break practical compatibility?

You mentioned somewhere else, that making non-virtual d'tors protected 
does not solve the problem of the warning. But as it solves the defect, 
may be later versions of the compiler will not complain anymore?


So we would at least keep the option to switch the warning on in future.

Nikolai


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Nikolai Pretzell]

Hi Stephan and all,

The consensus back then was to keep -Wnon-virtual-dtor switched on 
globally, and only switch it off (together with all other warnings) 
within cppumaker-generated headers, on the grounds that 
-Wnon-virtual-dtor was considered a useful tool to find errors.


However, trying to compile 
cppuhelper/qa/propertysetmixin/comp_propertysetmixin.cxx, I fear we have 
to rethink the case:  cppu::PropertySetMixin is part of the stable API 
of OOo/URE, is a class template, and contains a non-virtual dtor and 
virtual functions.  Since it is part of the stable API, we cannot change 
its dtor to be virtual (and even if we could, one could argue that that 
would not be what one would want to do, but consider it a hack to work 
around the warning).  And since it is a template, we cannot disable the 
warning locally---the warning is emitted at the end of each compilation 
unit that includes cppuhelper/propertysetmixin.hxx, rather than from 
within cppuhelper/propertysetmixin.hxx itself.


The only solution I see is to bite the bullet and globally disable 
-Wnon-virtual-dtor.


Mhmm. Why not make the d'tor protected?
As for compatibility, there should be no binary difference.
Only the use would be constrained, but if cppu::PropertySetMixin is 
indeed used as a mixin, then in most cases (whenever it is not the first 
base class or any other base class is virtual, may be even whenever it 
is not the only base class) calling its d'tor directly would be a bug 
anyway.

So maybe this does not break practical compatibility?

You mentioned somewhere else, that making non-virtual d'tors protected 
does not solve the problem of the warning. But as it solves the defect, 
may be later versions of the compiler will not complain anymore?


So we would at least keep the option to switch the warning on in future.

Nikolai

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Eike Rathke]

Hi Stephan,

On Wed, Feb 01, 2006 at 17:52:30 +0100, Stephan Bergmann wrote:

> >>When we are done with warnings01, we should have most warnings in the
> >>default warning level. People don't have to use wall=tr anymore, so this
> >>one could be a candidate for some extra warning in wall=tr
> >
> >+1
> >
> >  Eike
> 
> +-0,
> 
> as the past has shown that any such mechanism that is not actively 
> maintained over time becomes unusable through too much noise (see 
> assertions; the current, pre-warnings01 wall=1; maybe also memory leaks 
> at program termination time).

I jumped in on that so quickly because I almost always use wall=t when
compiling a source I just edited from within the editor. Which of course
is quite a pain sometimes, arising from lower modules not in my scope at
that time.. At least having the non-virtual-dtor warning, or any other
warning we have to disable because of certain files can't be changed for
compatibility reasons but are not included frequently, in wall=t is
better than disabling it completely. One should always be able to
override it though, in case it gets too annoying in some scenario.

I wonder whether awareness of the need to use mechanisms like wall=t and
assertions _and fix_ occurences can be raisen.

  Eike

-- 
 OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 GnuPG key 0x293C05FD:  997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

Eike Rathke wrote:

Hi Malte,

On Wed, Feb 01, 2006 at 13:00:24 +0100, Malte Timmermann wrote:



When we are done with warnings01, we should have most warnings in the
default warning level. People don't have to use wall=tr anymore, so this
one could be a candidate for some extra warning in wall=tr



+1

  Eike


+-0,

as the past has shown that any such mechanism that is not actively 
maintained over time becomes unusable through too much noise (see 
assertions; the current, pre-warnings01 wall=1; maybe also memory leaks 
at program termination time).


-Stephan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Eike Rathke]

Hi Malte,

On Wed, Feb 01, 2006 at 13:00:24 +0100, Malte Timmermann wrote:

> When we are done with warnings01, we should have most warnings in the
> default warning level. People don't have to use wall=tr anymore, so this
> one could be a candidate for some extra warning in wall=tr

+1

  Eike

-- 
 OOo/SO Calc core developer. Number formatter stricken i18n transpositionizer.
 GnuPG key 0x293C05FD:  997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

Stephan Bergmann wrote:

On Nov 2, 2005, Stephan Bergmann wrote:


Hi all,

 shows the 
following problem:  On compilers later than the GCC 3.4.1 we use at 
Hamburg (e.g., GCC 4.0.2, but from looking at the documentation also 
GCC  3.4.4), -Wall implies a new -Wnon-virtual-dtor which warns about 
classes with virtual functions that do not have a virtual destructor.


On such compilers, this warning will occur for each idlc-generated UNO 
header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of 
compatibility, it is not an option to change those idlc-generated 
headers and add a virtual destructor to them.


Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
off *all* warnings from within idlc-generated headers (#pragma GCC 
system_header).  The second option is probably preferable, as 
-Wnon-virtual-dtor might point to real issues in other places of the 
code base, and it is unlikely that there would be other warnings from 
any idlc-generated header that we would thus suppress---the 
idlc-generated headers are pretty boiler-plate.


Opinions?

-Stephan



The consensus back then was to keep -Wnon-virtual-dtor switched on 
globally, and only switch it off (together with all other warnings) 
within cppumaker-generated headers, on the grounds that 
-Wnon-virtual-dtor was considered a useful tool to find errors.


However, trying to compile 
cppuhelper/qa/propertysetmixin/comp_propertysetmixin.cxx, I fear we have 
to rethink the case:  cppu::PropertySetMixin is part of the stable API 
of OOo/URE, is a class template, and contains a non-virtual dtor and 
virtual functions.  Since it is part of the stable API, we cannot change 
its dtor to be virtual (and even if we could, one could argue that that 
would not be what one would want to do, but consider it a hack to work 
around the warning).  And since it is a template, we cannot disable the 
warning locally---the warning is emitted at the end of each compilation 
unit that includes cppuhelper/propertysetmixin.hxx, rather than from 
within cppuhelper/propertysetmixin.hxx itself.


The only solution I see is to bite the bullet and globally disable 
-Wnon-virtual-dtor.


Opinions?

-Stephan


Besides, all is not lost, as we can keep the corresponding warning on 
wntmsci10 enabled.


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Malte Timmermann]

When we are done with warnings01, we should have most warnings in the
default warning level. People don't have to use wall=tr anymore, so this
one could be a candidate for some extra warning in wall=tr

Malte.

Thorsten Behrens wrote:
> Stephan Bergmann <[EMAIL PROTECTED]> writes:
> 
>> The only solution I see is to bite the bullet and globally disable
>> -Wnon-virtual-dtor.
>>
>> Opinions?
>>
> Hi Stephan,
> 
> yes, I think so. But I'd still like to have this (and possibly other)
> warnings in an optional pedantic mode - to be run after warnings01 is
> finished, and inspected for cases where using a non-virtual dtor was
> not intentional.
> 
> Cheers,
> 

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Thorsten Behrens]

Stephan Bergmann <[EMAIL PROTECTED]> writes:

> The only solution I see is to bite the bullet and globally disable
> -Wnon-virtual-dtor.
>
> Opinions?
>
Hi Stephan,

yes, I think so. But I'd still like to have this (and possibly other)
warnings in an optional pedantic mode - to be run after warnings01 is
finished, and inspected for cases where using a non-virtual dtor was
not intentional.

Cheers,

-- 

Thorsten

If you're not failing some of the time, you're not trying hard enough.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

On Nov 2, 2005, Stephan Bergmann wrote:

Hi all,

 shows the 
following problem:  On compilers later than the GCC 3.4.1 we use at 
Hamburg (e.g., GCC 4.0.2, but from looking at the documentation also GCC 
 3.4.4), -Wall implies a new -Wnon-virtual-dtor which warns about 
classes with virtual functions that do not have a virtual destructor.


On such compilers, this warning will occur for each idlc-generated UNO 
header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of 
compatibility, it is not an option to change those idlc-generated 
headers and add a virtual destructor to them.


Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
off *all* warnings from within idlc-generated headers (#pragma GCC 
system_header).  The second option is probably preferable, as 
-Wnon-virtual-dtor might point to real issues in other places of the 
code base, and it is unlikely that there would be other warnings from 
any idlc-generated header that we would thus suppress---the 
idlc-generated headers are pretty boiler-plate.


Opinions?

-Stephan


The consensus back then was to keep -Wnon-virtual-dtor switched on 
globally, and only switch it off (together with all other warnings) 
within cppumaker-generated headers, on the grounds that 
-Wnon-virtual-dtor was considered a useful tool to find errors.


However, trying to compile 
cppuhelper/qa/propertysetmixin/comp_propertysetmixin.cxx, I fear we have 
to rethink the case:  cppu::PropertySetMixin is part of the stable API 
of OOo/URE, is a class template, and contains a non-virtual dtor and 
virtual functions.  Since it is part of the stable API, we cannot change 
its dtor to be virtual (and even if we could, one could argue that that 
would not be what one would want to do, but consider it a hack to work 
around the warning).  And since it is a template, we cannot disable the 
warning locally---the warning is emitted at the end of each compilation 
unit that includes cppuhelper/propertysetmixin.hxx, rather than from 
within cppuhelper/propertysetmixin.hxx itself.


The only solution I see is to bite the bullet and globally disable 
-Wnon-virtual-dtor.


Opinions?

-Stephan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

Stephan Bergmann wrote:

Thorsten Behrens wrote:


Stephan Bergmann <[EMAIL PROTECTED]> writes:



On such compilers, this warning will occur for each idlc-generated UNO
header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of
compatibility, it is not an option to change those idlc-generated
headers and add a virtual destructor to them.



Is there really no feasible way to close that hole? I'm perfectly fine
with the pragma solution, but still think it would be a good idea to
forbid calling base class destructors (what about declaring a
protected non-virtual destructor in XInterface?).



Good idea, thanks.  Adding protected non-virtual dtor to all the 
"interface" classes generated by codemaker.


Bad idea   :(

It appears that the GCC 3.4.1 we use in Hamburg supports 
-Wnon-virtual-dtor, but in a broken way that only issues warnings in 
some situations and not in others.  Adding protected non-virtual dtors 
to "interface" classes generated by codemaker "live" on CWS warnings01 
caused warnings to appear for example for cppu::ImplHelper1 which did 
not appear before.  So, I am right now backing out the addition of 
protected non-virtual dtors on CWS warnings01 again, and will look for a 
better solution...


-Stephan


-Stephan


Cheers,


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

Thorsten Behrens wrote:

Stephan Bergmann <[EMAIL PROTECTED]> writes:



On such compilers, this warning will occur for each idlc-generated UNO
header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of
compatibility, it is not an option to change those idlc-generated
headers and add a virtual destructor to them.



Is there really no feasible way to close that hole? I'm perfectly fine
with the pragma solution, but still think it would be a good idea to
forbid calling base class destructors (what about declaring a
protected non-virtual destructor in XInterface?).


Good idea, thanks.  Adding protected non-virtual dtor to all the 
"interface" classes generated by codemaker.


-Stephan


Cheers,


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Thorsten Behrens]

Stephan Bergmann <[EMAIL PROTECTED]> writes:

> On such compilers, this warning will occur for each idlc-generated UNO
> header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of
> compatibility, it is not an option to change those idlc-generated
> headers and add a virtual destructor to them.
>
Is there really no feasible way to close that hole? I'm perfectly fine
with the pragma solution, but still think it would be a good idea to
forbid calling base class destructors (what about declaring a
protected non-virtual destructor in XInterface?).

Cheers,

-- 

Thorsten

If you're not failing some of the time, you're not trying hard enough.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Jürgen Schmidt]

Stephan Bergmann wrote:

Hi all,

 shows the 
following problem:  On compilers later than the GCC 3.4.1 we use at 
Hamburg (e.g., GCC 4.0.2, but from looking at the documentation also GCC 
 3.4.4), -Wall implies a new -Wnon-virtual-dtor which warns about 
classes with virtual functions that do not have a virtual destructor.


On such compilers, this warning will occur for each idlc-generated UNO 
header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of 
compatibility, it is not an option to change those idlc-generated 
headers and add a virtual destructor to them.


Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
off *all* warnings from within idlc-generated headers (#pragma GCC 
system_header).  The second option is probably preferable, as 
-Wnon-virtual-dtor might point to real issues in other places of the 
code base, and it is unlikely that there would be other warnings from 
any idlc-generated header that we would thus suppress---the 
idlc-generated headers are pretty boiler-plate.


+1 for switching off this warnings in the generated header

- Juergen



Opinions?

-Stephan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Malte Timmermann]

Also +1 for switching this warning off from within idlc-generated
headers only

Malte.

Tino Rachui - Sun Germany - Development - Software Engineer wrote:
> Stephan Bergmann wrote:
> 
> 
>>Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
>>off *all* warnings from within idlc-generated headers (#pragma GCC 
>>system_header).  The second option is probably preferable, as 
>>-Wnon-virtual-dtor might point to real issues in other places of the 
>>code base, and it is unlikely that there would be other warnings from 
>>any idlc-generated header that we would thus suppress---the 
>>idlc-generated headers are pretty boiler-plate.
> 
> 
> +1 for switching this warning off from within idlc-generated headers only
> 
> -Tino
> 
> -
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
> 

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Tino Rachui - Sun Germany - Development - Software Engineer]

Stephan Bergmann wrote:

Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
off *all* warnings from within idlc-generated headers (#pragma GCC 
system_header).  The second option is probably preferable, as 
-Wnon-virtual-dtor might point to real issues in other places of the 
code base, and it is unlikely that there would be other warnings from 
any idlc-generated header that we would thus suppress---the 
idlc-generated headers are pretty boiler-plate.


+1 for switching this warning off from within idlc-generated headers only

-Tino

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Kohei Yoshida]

On 11/2/05, Stephan Bergmann <[EMAIL PROTECTED]> wrote:

> Thus, we can either switch off -Wnon-virtual-dtor globally, or switch
> off *all* warnings from within idlc-generated headers (#pragma GCC
> system_header).  The second option is probably preferable, as
> -Wnon-virtual-dtor might point to real issues in other places of the
> code base, and it is unlikely that there would be other warnings from
> any idlc-generated header that we would thus suppress---the
> idlc-generated headers are pretty boiler-plate.

I like the second option.  So, +1 for using the pragma macro to
locally suppress the warning within hdl headers.  Because of the
severity of overlooking this warning, turning it off globally would be
IMO a very bad move.

Kohei

--
Kohei Yoshida
OpenOffice.org Calc contributor
http://kohei.us/ooo/

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [dev] warnings01: -Wnon-virtual-dtor

[Daniel Boelzle]

> On such compilers, this warning will occur for each idlc-generated UNO 
> header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of 
> compatibility, it is not an option to change those idlc-generated 
> headers and add a virtual destructor to them.

Should be sufficient using this pragma only in interface header files
generated by cppumaker.

> 
> Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
> off *all* warnings from within idlc-generated headers (#pragma GCC 
> system_header).  The second option is probably preferable, as 
> -Wnon-virtual-dtor might point to real issues in other places of the 
> code base, and it is unlikely that there would be other warnings from 
> any idlc-generated header that we would thus suppress---the 
> idlc-generated headers are pretty boiler-plate.

+1, switching off globally is really no good option.

-Daniel

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

[dev] warnings01: -Wnon-virtual-dtor

[Stephan Bergmann]

Hi all,

 shows the 
following problem:  On compilers later than the GCC 3.4.1 we use at 
Hamburg (e.g., GCC 4.0.2, but from looking at the documentation also GCC 
 3.4.4), -Wall implies a new -Wnon-virtual-dtor which warns about 
classes with virtual functions that do not have a virtual destructor.


On such compilers, this warning will occur for each idlc-generated UNO 
header (com/sun/star/uno/XInterface.hpp etc.).  For reasons of 
compatibility, it is not an option to change those idlc-generated 
headers and add a virtual destructor to them.


Thus, we can either switch off -Wnon-virtual-dtor globally, or switch 
off *all* warnings from within idlc-generated headers (#pragma GCC 
system_header).  The second option is probably preferable, as 
-Wnon-virtual-dtor might point to real issues in other places of the 
code base, and it is unlikely that there would be other warnings from 
any idlc-generated header that we would thus suppress---the 
idlc-generated headers are pretty boiler-plate.


Opinions?

-Stephan

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]