warnings (was: Re: kdelibs/kdeui)

Lubos Lunak l.lunak at sh.cvut.cz
Sun Apr 21 20:05:51 BST 2002


On Sunday 21 April 2002 19:22, Dirk Mueller wrote:
> On Son, 21 Apr 2002, Lubos Lunak wrote:
> >  No, it's not clean, otherwise there wouldn't be a valid warning.
> > Ideally, there should be of course 'using', but since it doesn't work
> > with the oficially recommended gcc version, overriding instead is a
> > workaround, that maybe doesn't look 100% perfect, but does the job and
> > doesn't break anything.
>
> Yes, it doesn't break, but it doesn't make the code more cleaner.

 But it gets rid of the warning.

>
> > > Of course, but most of the time this hiding was intended (or we can't
> > > change it, i.e. faults in Qt design).
> >
> >  Intended hiding of virtual methods, hmm? And faults can be fixed, or
> > should they stay there forever? They could have been fixed for Qt3.
>
> As an example is the changed signatures between QMultiLineEdit and
> QTextEdit. Therefore any inclusion of QMultiLineEdit widget will cause
> these warnings, which are "intended".

 I don't think they are intended, to me it simply looks like TT don't know 
about the warning or don't care.

 Ok, let me put it this way: If I send a patch to TT that will get rid of the 
warnings and they'll accept it, and if I clean the cases in kdelibs in case 
there's something needed too, may I add it to the default flags? I haven't 
actually yet thoroughly examined all the warnings but I think there's no 
reason why they shouldn't accept them, in fact, one of the warnings points to 
a real mistake or a very stupid thing.

>
> Anyway, I agree the warnings might be useful, but they're safe enough to be

 I guess you meant 'not safe enough here' - which I disagree.

> enabled by default. They're enabled in khtml, because that is code that is
> mostly built around its own, and doesn't depend on Qt or kdelibs inherited
> signature mismatches too much.
>
> > > by inlining a virtual method we can not change it anymore.
> >
> >  I said it can be non-inline, without any serious performance hit or
> > whatever.
>
> Yes, but that wasn't done, and this is the part that annoyed me. thats all.

 Happy now? :)

>
> > > Yes, does that prevent people from committing broken code? no. When we
> > > change the default to --enable-warnings, those people who care about
> > > the 1% performance penality will immediately switch to
> > > --disable-warnings and continue committing broken code.
> >
> >  But such people will be different from those who now don't see the
> > warnings - those who don't see the warnings now often don't know how to
> > turn it on or even that they can turn it on, while people who will use
> > --disable-warnings and commit code with warnings are ... well ... @#%#!
>
> Yes, you get the problem :-) Its the same with the recommendation to use
> --enable-debug, and still, how many developers use --enable-debug ?

 *sigh* I seem to have some problems with my English lately. I'm simply 
suggesting to make --enable-warnings the default, so that developers won't 
have to be recommended doing that. I'm sure those not willing to see warnings 
for any reason will find out how to turn it off while I'm not that sure 
people who would like to see them now do know how to turn it on.

 See also e.g. http://lists.kde.org/?l=kde-cvs&w=2&r=1&s=--enable-warnings&q=b 
where you yourself complain about --enable-warnings not being the default. 
There are also listed some Coolo's reasons why --enable-warnings is not the 
default, but while I understand it's Coolo who gets most of those stupid 
bugreports, I don't think the reasons are worth it. Developers are 
recommended to use --enable-debug, so where's the reason of compiling taking 
longer (*cough*), and I don't think we should prefer not getting stupid 
bugreports from users (how many actually?) to developers not doing stupid 
mistakes. Also, the warnings could be made disabled for the packages.

-- 
 Lubos Lunak
 llunak at suse.cz ; l.lunak at kde.org
 http://dforce.sh.cvut.cz/~seli





More information about the kde-core-devel mailing list