[Scummvm-devel] g++ 4.3 compiler flags

[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.

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.

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.

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?

// Johannes