Re: [boost] [Wiki] Changes in information about gcc warnings.

Subject: Re: [boost] [Wiki] Changes in information about gcc warnings.
From: Philipp Reh (sefi_at_[hidden])
Date: 2011-01-14 09:43:37

Hi,

I have some comments on the guidelines.
First of all: Goob job on the article.

1) Type punning through unions is also undefined behaviour
but all compilers I know of just allow it.
The correct way to convert incompatible types would be to memcpy
or std::copy (via unsigend char *) them.

2) The section about VC++ warnings is pretty huge and I agree with most
of them.

* "no definition for inline function" is actually undefined behaviour
if you call the function. That should be fixed and not supressed.

* "conditional expression is constant" sometimes happens in constructs
like if(some_metafunction<Type>::value). I think they should be turned
into proper compile time branches (via enable_if for example).

3) I would like to add some stuff to the gcc section (also clang kind of
falls in this category as it tries to implement the warnings that gcc does
but is still incomplete as of version 2.8).

-Wall -Wextra -pedantic(-errors) is a must for gcc. There are more warnings
that aren't turned on by that, which I consider to be useful and reveal 
bugs, and I'm going to talk about next.

* -Wconversion: It warns about truncating (or value changing) conversions
and got kind of useful in gcc-4.3 and later. For example it will warn about
conversions between float and int and vice versa, or narrowing conversions
like int to short. clang is even better here and warns about signed/unsigned
conversions as well. Sometimes this warning can reveal real bugs,
sometimes it only forces some static_casts (but they should be used with
great care).

* -Wfloat-equal: Another warning that can reveal real bugs because 
comparison of floats via == or != sometimes works or it doesn't. Those
should always be compared with an epsilon (but it is also hard to figure
out what a large enough epsilon could be). The real problem here is that
some template code simply cannot handle this in a reasonable way.

* -Wnon-virtual-dtor: Warns about classes that have virtual functions but
not a virtual destructor. Probably indicates a bug.

Next are some warnings that might be useful in some cases:

* -Woverloaded-virtual: Warns if a virtual function is hidden by an
overloaded function in a derived class. Can be surpressed by typing
"using base::foo" to bring the virtual function in scope again.
Might also indicate a bug (like forgotten const or something).

* -Wold-style-cast: Warns about C style casts (sadly not about
functional style casts). C style casts are only useful for
casting "private away from base classes" because none of the C++ casts
can do that. It is often best practice to avoid C casts altogether.
I have seen that boost has a lot of old style casts that should be turned
into their proper C++ counterparts instead.

* -Wshadow: Warns when a name hides another name (like a local variable
hiding a member variable). Also warns about hidden typedefs in gcc-4.6.
I think it is best practice not to do that.

* -Wmissing-declarations: Only available in recent gcc versions (can't
remember which one exactly) it warns about functions that have only
a definition but no declaration (except for inline functions, templates
and functions in anonymous namespaces). Probably indicates a bug.

Cheers,
Philipp