A very worthy effort. I would just highlight the wise words of Linus Torvalds, so this endeavour can start on the right foot:

"Please, we do NOT fix compiler warnings without understanding the code! That's a sure way to just introduce _new_ bugs, rather than fix old ones. So please, please, please, realize that the compiler is _stupid_, and fixing warnings without understanding the code is bad.

"In this case, anybody who actually spends 5 seconds looking at the code should have realized that the warning is just another way of saying that the author of the code was on some bad drugs, and the warnings WERE BETTER OFF REMAINING! Because that code _should_ have warnings. Big fat warnings about incompetence!?"

Agree completely. You'll be happy to know that we've been paying very careful attention and mistakes notwithstanding there's been some really great progress made. We now have some good floating-point comparison functions to address -Wfloat-equal warnings and we're shortly getting a lovely string interpolator that will take care of -Wformat.

It's a good time to be a Pioneer dev 🙂

Another thing to keep in mind, we compile across multiple platforms which can cause issues.

For example, I just pulled the latest master into my own fork/branch and found that VS2010 can't compile due to "int pi_lua_panic(lua_State *L)" having had it's return value removed 🙁

Obviously I can fix this locally but in the broader sense what do people feel is the right approach here?

We can macro this up with pre and postfix macros that are different per platform, using "__declspec(noreturn)" under MSVC for the prefixed version and "__attribute((noreturn))" for the GNU postfixed but then you end up with something that looks like:

Which might work but STILL requires that the WIN32 has the "return 0;" within the function.

Suggestions?

PS: I haven't tried that above code beyond seeing if it compiles.

I must apologise -- that was my fault. I've submitted a patch (it's an active pull request -- if you have time to review it I'd be grateful), which should probably be combined with your suggestion.

Re: the broader approach, my opinion would be that where possible it would be better to mark up the code for both GCC and MSVC, and try to run close to zero warnings with both compilers. So I would be happy to see your suggestion merged (*)

In this case I'll note that I don't think these noreturn functions are long-term solutions: I think pi_lua_panic should mostly go away and be replaced by a handler installed through lua_pcall (I've done some work towards this already). The other instance of this is GeoSphere::UpdateLODThread, which is a thread function and enters an infinite loop. Again, it would be nice to break out of this cleanly rather than having it simply be a noreturn function.

(*) Minor note though, so I don't later forget to mention it later, the definition you should check for to recognise MSVC is _MSC_VER, not _WIN32.

jpab's fixes are merged and I've brought the VS2008 build up to date.

No worries it wasn't meant as criticism as I've broken the build for other platforms repeatedly too, it was just the issue I hit this time 🙂

Ah that was it, I couldn't remember it so just used _WIN32 😀

Cheers.

Thats odd as I had to update quite a few header files to get it to compile, also on running it'll quit with Unhandled exception showing lauxlib.c line 635 return realloc(ptr, nsize); and ltable.c line 400 Node *mp = mainposition(t, key); on the stack.

Either my master is messed up beyond belief, or its not quite solved yet.

Edit: Forget that. Turns out it was caused by an old model cache and a dodgy master.