[patch] Improve build flags and code quality

Discussion area about developing or extending OGRE, adding plugins for it or building applications on it. No newbie questions please, use the Help forum for that.
Post Reply
User avatar
Svenstaro
Greenskin
Posts: 115
Joined: Fri Dec 15, 2006 1:30 pm
Location: Germany
x 3
Contact:

[patch] Improve build flags and code quality

Post by Svenstaro »

Hi,

I noticed Ogre doesn't use flags like -Wall, -Wextra, -Werror, -pedantic (or their MSVS equivalents). Is there any reason for that? Obviously those flags are annoying like that because you will get a lot of false warnings but those can easily be turned of using -Wno-unused-parameter -Wno-unused-but-set-parameter. In fact, Ogre used to (1.7) compile with the flags mentioned above but it no longer does (1.8rc1) and I see this is a result of not using these parameters in the first place. Using them would improve code quality due to being more compatible with projects that use strict build flags. Those projects would not compile if the compiler includes an Ogre file that does not compile using strict flags.

This is sadly an actual problem to me since my projects stopped compiling with 1.8rc1 and I'd like to improve that situation for the next rc/final.

With that in mind, I'd like to request for Ogre to adopt stricter build flags and I'm making this thread as this will likely require a discussion. On a side note, Ogre 1.8rc1 will currently not work with projects that use -std=c++0x due to this other error.
Last edited by Svenstaro on Tue Nov 29, 2011 11:46 pm, edited 1 time in total.
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58
Contact:

Re: Improve build flags and code quality

Post by CABAListic »

In my opinion, the ratio of false to useful warnings with the -Wall flag is totally off and therefore it's a pain in the ass more than it's useful. As such, I'm not a fan of the -Wall -Werror combo as a default.

However, we do (and did in the past) accept patches to shut off warnings in Ogre's headers so that they don't interfere with other projects.
User avatar
Svenstaro
Greenskin
Posts: 115
Joined: Fri Dec 15, 2006 1:30 pm
Location: Germany
x 3
Contact:

Re: Improve build flags and code quality

Post by Svenstaro »

Actually with -Wall -Wextra -Werror -Wno-unused-parameter -Wno-unused-but-set-parameter -pedantic a make should run through without false positives and you would know your code is super clean. I will contribute a trivial patch for the current cleanups necessary to make those flags possible.

However, a more problematic flag right now is -std=c++0x and it has something to do with Ogre's custom STL memory allocators but I can't quite deduce the error message. If you can fix that I will deliver the rest.
dermont
Bugbear
Posts: 812
Joined: Thu Dec 09, 2004 2:51 am
x 42

Re: Improve build flags and code quality

Post by dermont »

From your error it looks like it expects a construct taking a single parameter in OgreMemorySTLAllocator.h , something like:

Code: Select all

	void construct(pointer p)
	{
		// call placement new
		new(static_cast<void*>(p)) T();
	}
You would probably be better waiting for the developers to confirm. And off-topic in the 1.8 branch OgreImage.cpp needs to include "OgreMath.h" to build.
User avatar
Svenstaro
Greenskin
Posts: 115
Joined: Fri Dec 15, 2006 1:30 pm
Location: Germany
x 3
Contact:

Re: Improve build flags and code quality

Post by Svenstaro »

I was mistaken. Compiling ogre with std=c++0x always failed and the Ogre::STLAllocator was not changed since a long time. However, ogre uses long long which is only standard since c++0x, so we'll need to make this work.
User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
x 56
Contact:

Re: Improve build flags and code quality

Post by Klaim »

Now that you're talking about it, is it a problem to have ogre compiled without C++0x and your application using c++0x? I'm guessing that no as far as you don't rely on binary compatibility, but if you did try that any feedback would be welcome.
User avatar
sparkprime
Ogre Magi
Posts: 1137
Joined: Mon May 07, 2007 3:43 am
Location: Ossining, New York
x 13
Contact:

Re: Improve build flags and code quality

Post by sparkprime »

-Werror is a dumb idea tbh -- knowledge is power, but obstacles are a pain in the arse

-Wno-long-long (possibly misspelt) may help

You shouldn't actually need -std=c++0x for trivial things like long long and additional API functions like strtoll. It will only encourage you to start using language features that are not yet mainstream. Better to start with the old standard and then 'add' the things you need explicitly, so it's clear how much of c++0x is actually needed.

Of the false warnings in -Wall -Wextra, all of them can be avoided by making trivial changes in the source code to indicate that 'actually I did want to do that'. E.g. (void)ing unused params. That way you are avoiding potential bugs *and* avoiding getting a screenful of crap everytime you compile.
User avatar
altren
Gnome
Posts: 329
Joined: Tue Oct 24, 2006 9:02 am
Location: Moscow, Russa
x 24
Contact:

Re: Improve build flags and code quality

Post by altren »

While many developers dislike this flags due to lots of not so useful warnings it is still good to use them. Main problem is that you can't just turn it on for some big projects, because you'll get way too much to handle. I spent few days, when I decided to do that with MyGUI. We ended with several gcc flags (-Wall is far from all useful warnings) and some disable warnings like unused parameter. Here's list of flags we use (and having 0 warnings in current MyGUI code, also there were almost no useless changes just to make it shut up):

Code: Select all

-Wno-deprecated -Wall -Wctor-dtor-privacy -Winit-self -Woverloaded-virtual -Wcast-qual -Wwrite-strings -Wextra -Wno-unused-parameter -pedantic -Wshadow
(use gcc documentation to figure what does this warnings are used for).

Also it is important to make 3rd party libraries not produce warnings, since they are included, but you probably not going to change them:

Code: Select all

-isystem ${OGRE_INCLUDE_DIR}
-isystem ${OIS_INCLUDE_DIR}
And last thing that might help in such task is next flag - it show which flag generate warning, so if you dislike warning, but have no idea which flag to use this diagnostic message will help:

Code: Select all

-fdiagnostics-show-option
This option instructs the diagnostic machinery to add text to each diagnostic emitted, which indicates which command line option directly controls that diagnostic, when such an option is known to the diagnostic machinery.
sparkprime wrote:E.g. (void)ing unused params. That way you are avoiding potential bugs *and* avoiding getting a screenful of crap everytime you compile.
Use -Wno-unused-parameter with -Wall. If you think, that some warnings are not really useful - just disable them, but keep other useful warnings.
Image
User avatar
altren
Gnome
Posts: 329
Joined: Tue Oct 24, 2006 9:02 am
Location: Moscow, Russa
x 24
Contact:

Re: Improve build flags and code quality

Post by altren »

Ah, and one more thing - since we should not bother library users with warnings we use additional CMake option, that turn off extra warnings with gcc or msvc (W3 instead of W4).
Image
User avatar
sparkprime
Ogre Magi
Posts: 1137
Joined: Mon May 07, 2007 3:43 am
Location: Ossining, New York
x 13
Contact:

Re: Improve build flags and code quality

Post by sparkprime »

altren wrote: ...
useful stuff you posted

altren wrote:
sparkprime wrote:E.g. (void)ing unused params. That way you are avoiding potential bugs *and* avoiding getting a screenful of crap everytime you compile.
Use -Wno-unused-parameter with -Wall. If you think, that some warnings are not really useful - just disable them, but keep other useful warnings.

But then you don't avoid bugs, you are disabling the warning when it is helpful as well as when it is not helpful.

The point is that you get the warning and then you say "that's ok i meant to do that", make a minor modification in the code to signal your intent and make the warning go away, but when perhaps you have a real bug like void test (int x, int y) { return x == x; } the warning will still help you pick it up
User avatar
Svenstaro
Greenskin
Posts: 115
Joined: Fri Dec 15, 2006 1:30 pm
Location: Germany
x 3
Contact:

Re: Improve build flags and code quality

Post by Svenstaro »

Here, I made some changes to improve code quality. It's not complete but it's a start. Also, we definitely need that STLAllocator changes in or compiling against projects that use std=c++0x will fail. See my cleanups branch here: https://bitbucket.org/svenstaro/ogre

Patch entry: https://sourceforge.net/tracker/?func=d ... tid=302997
Post Reply