[Scummvm-devel] g++ 4.3 compiler flags

[Max Horn]

Am 04.06.2008 um 19:38 schrieb Johannes Schickel:

> Hi,
>
> I made a commit today to have ScummVM (at least partially, as in the  
> engines I'm using ;-) compile under g++ 4.3 with -Wall and -Werror  
> again.  Check r32540 for the changes. I had to disable some IMHO  
> useful warnings, which we should think of enabling again in the  
> future.
>
> First of all I removed -Wconversion for g++ 4.3, since it warns  
> about *any* implicit type conversion which may alter the values now.  
> That means casting int to char etc. This *might* be useful, but  
> since we do such conversions all over the place someone would have  
> to get rid of all of them first IMHO, which would be quite a big/ 
> annoying task. G++ 4.3 introduced now -Wsign-conversion, which warns  
> about implicit signed <-> unsigned conversion. Also a thing I would  
> say which could be pretty useful (at least Kyra suffered from such a  
> problem once :-), but with it ScummVM is also broken with -Werror,  
> since it does implicit signed <-> unsigned casts all over the place,  
> e.g. the SDL backend, HashMap code, etc, but these are far less  
> warnings than with -Wconversion and it is much more useful IMHO.

Well, in 99% those warnings are only annoying ... so I don't mind if  
it's disabled.

>
>
> Another nifty warning added is -Wparentheses, which warns about non  
> obvious operator evaluation precedence and asks the developer (or  
> with -Werror he has)  to use parentheses to clarify it. This is  
> currently disabled by adding -Wno-parentheses to the compiler flags,  
> else one had to change large chunks of code too (I found warnings  
> like that in code from at least sound/ and scumm/ I didn't want to  
> fix all those either, since it's not always obvious what they want  
> to do :-), so I stopped checking. It also warns about using &&  
> inside of || blocks (see the gui/eval.cpp change I made). IMHO we  
> should think of enabling it again and ask the people with knowledge  
> about the specific code parts to fix the warnings for it, it should  
> be another pretty usefull warning, expecially when using bit wise  
> operators like |, & etc.

Sounds like a useful warning to me.

>
>
> Some of you might have noticed I also added -Wno-empty-body, this  
> one would usually warn about for (int i = 0; i < 10; ++i); and  
> suggest to put a space before the ';' or add {}. That's quite
> annoying IMHO, so I think it is safe to keep it disabled in the  
> future.

Well, OTOH such code *is* a potential source code bugs. I usually  
prefer to have a return before the ";" or "{}", i.e. have it in two  
lines. We have had bugs caused by an extra unwanted ";" ...

>
>
> So maybe some of you got some ideas on which warnings we should have  
> enabled and which we should have disabled. And of course it would be  
> nice if the engine developers could fix those warnings for their  
> code if we enable them. So what do you guys think of it?
I am all for fixing warnings, as long as it is sensible. However, i  
will not have any access to GCC 4.3 for the foreseeable future, so I  
won't be able to contribute myself (other than answering questions  
about pieces of code, maybe).

Cheers,
Max