Master won't compile

[Zini]

Not entirely convinced. Both C4309 and C4244 look a bit strong (and the current gcc settings slightly weak in this area; a bit disappointed about that). Do we still get a lot of warnings when these are not disabled? Please note that while most of the warnings you have seen are silly, some indicate actual problems (again gcc settings seem slightly too weak), so we shouldn't just prune them.

Also, could you add a comment about what these codes are meaning to the cmake script? They are a bit obscure compared to the gcc flags.

Could I convince you to take this task? You are kinda on it anyway.

We should definitely add some warnings. At least initialiser list order not matching the declaration order.

[Ace (SWE)]

I wasn't really happy about disabling warnings, I'd rather fix them but I suppose that's a bigger task.
The main reason I disabled C4309 is that tables_gen.hpp in to_utf8 gives that warning on almost every line, and I'm pretty sure it doesn't happen on *nix.
C4244 and C4305 both deal with storing one type of data in a variable with another type without casting it, somthing that occurs on many places in the code. This is maybe something to adjust in GCC?

I can take the task (Ace on the bugtracker), and I'll poke around with the flags and comment them in another commit

[Zini]

The main reason I disabled C4309 is that tables_gen.hpp in to_utf8 gives that warning on almost every line, and I'm pretty sure it doesn't happen on *nix.

Not *nix specific. For some compilers char is signed, for some compilers char is unsigned. In the former case the warning is justified, which means the code needs to be fixed.

C4244 and C4305 both deal with storing one type of data in a variable with another type without casting it, somthing that occurs on many places in the code.

Ideally the compiler should warn, when we try to assign a larger type to a smaller or more limited one (e.g. int to short or float to int), but not necessarily when we try to assign a type with higher precision to one with lower precision (double to float). The former is far worse, because we might actually get a wrong result, while the later only looses a bit of precision (for some values of "a bit").

What definitely should not generate a warning is when we assign to a float a double literal that can be exactly represented by a float (e.g. float x = 1;), because that is just silly.

And yes, some minor adjustments to the gcc settings may be required.

[Ace (SWE)]

I pushed my changes to the MSVC warning levels, commenting on all the important warnings I disabled. Since MSVC didn't allow for enabling specific warnings from any command line I decided to enable all warnings and then disable the ones that weren't needed.
The first block of warnings are the ones that tend to be quite useless, take for example;
C4619: There is no warning number <number>

Some of these warnings should probably be re-enabled and maybe even have GCC equivalents found for them, 4146 for example.
It seems to occur on the lines 111 and 125 of components/compiler/controlparser.cpp

We should definitely add some warnings. At least initialiser list order not matching the declaration order.

This one warning sadly doesn't exist in Visual Studio/MSVC

[Zini]

Some of these warnings should probably be re-enabled and maybe even have GCC equivalents found for them, 4146 for example.

Actually, I would like to review all of them (the "OpenMW specific warnings" section). Don't have the time for it right now, but probably in a couple of days.

If it isn't too much work, could you find one or two examples for all of these?

[Zini]

This one warning sadly doesn't exist in Visual Studio/MSVC

I guess then we should disable it in gcc too. It's usefulness is somewhat limited anyway.

[Ace (SWE)]

I've been digging out a few examples of the warnings;

4099: nifogre/ogre_nif_loader.hpp lines 66 - 69, they're defined as struct in nif_types.hpp
4100: mangle stream.hpp lines 49, 63, 70, 92, and 97
4101: mwgui/console.cpp line 53
4127: files/windowspath.cpp line 78 (By design, but I think I'm going to change the code a bit anyway which would eliminate the warning)
4146: compiler/controlparser.cpp lines 111 and 125
4242: nif/nif_file.cpp line 193, nifogre/ogre_nif_loader lines 359, 394, 444, 455, 941, and 1326
4244: mangle std_stream.hpp lines 34, 47, and 54
4265: nif/extra.hpp line 87, nif/data.hpp lines 354, 431, 515, and 825
4305: esm/esm_reader line 156
4309: to_utf8/tables_gen.hpp on almost every line
4355: mwworld/world.cpp line 160
4365: This one has to be relocated to the standard library warning block, disregard it
4701: to_utf8/to_utf8.cpp line 171, mangle audiere_file.cpp line 29, and esm/loadglob.cpp line 18
4702: compiler/controlparser.cpp lines 99 and 157

[Zini]

Thanks. Will have a look at it once I have a bit of spare time available.

[psi29a]

The only error I see with gcc/g++ is this:

/home/brick/workspace/OpenMW/openmw/apps/openmw/mwrender/animation.cpp: In member function ‘void MWRender::Animation::handleShapes(std::vector<Nif::NiTriShapeCopy>*, Ogre::Entity*, Ogre::SkeletonInstance*)’:
/home/brick/workspace/OpenMW/openmw/apps/openmw/mwrender/animation.cpp:185:35: warning: variable ‘currentNormal’ set but not used [-Wunused-but-set-variable]

And that is with gcc/g++ version 4.6.1 with -Wall

[Zini]

Yeah, our current warning settings (-Wall) seem to be a bit light.