Subject: Re: [boost] [Wiki] Changes in information about gcc warnings.
From: Patrick Horgan (phorgan1_at_[hidden])
Date: 2011-01-15 01:06:55
On 01/14/2011 06:43 AM, Philipp Reh wrote:
> I have some comments on the guidelines.
> First of all: Goob job on the article
It's a collaboration, but I've done the GCC stuff. I can't tell you how
excited I am to have someone pitch in with comments that 1) cares about
it, and 2) has knowledgeable opinions. As you'll see below I agree with
almost everything you say, and think some of this should be added to the
wiki page. To do that we should come up with examples of code that
looks correct but is not which will be exposed by each particular
warning. Of course better is building with these things turned on and
then dealing with things that actually show up problems in boost.
Examples from real boost code of problems that are shown up by building
with warning options turned on gets people's attention. If there's no
bang for the buck it's a hard sale.
> 1) Type punning through unions is also undefined behaviour
> but all compilers I know of just allow it.
Actually, it's explicitly allowed in C. This is documented in C99 and
later C specs, as noted in this footnote to 188.8.131.52 Structure and union
85)If the member used to access the contents of a union object is not
the same as the member last used to store a value in the object, the
appropriate part of the object representation of the value is
reinterpreted as an object representation in the new type as described
in 6.2.6 (a process sometimes called ââtype punningââ). This might be a
> The correct way to convert incompatible types would be to memcpy
> or std::copy (via unsigend char *) them.
Thanks. That's certainly true for C++ and works for either C or C++,
and applies of course to any way of moving stuff around via the use of
char* or unsigned char*, not just memcpy or std::copy since a char* can
alias all of memory. I made changes to my white paper on aliasing to
reflect that. It's http://dbp-consulting.com/StrictAliasing.pdf and
started originally as that section in the wiki. I plan to bring the
changes back, but haven't yet. Soon. If you want to review that as
well, well, great! lol!
> ... patrick did elision of stuff about VC++ which he knows nothing about;) ...
> 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.
I agree with that and that's what I use in my own code. Getting
agreement for -Wall, was like pulling teeth, there was a lot of
resistance. If you add to the bjam invocation:
which inhibits all warning messages.
I would like to add -Wextra to warnings=all
> * -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).
Who wouldn't want to know that a conversion had truncated or changed a
> * -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.
This one seems pretty specialized and boost code using floating point,
done by people that understand these issues should certainly use this
flag to avoid these easily avoided errors.
> * -Wnon-virtual-dtor: Warns about classes that have virtual functions but
> not a virtual destructor. Probably indicates a bug.
This seems obvious, yet there were boosters that argued against this in
> 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).
This is a particularly crazy-making bug when you do it accidentally with
the same signature. It would save a lot of people hair pulling if this
was standard to the boost warnings.
> * -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.
That's the reason this will not be popular. Of course this is just for
C++. I believe that in C++ you should always use one of `dynamic_cast',
`static_cast', `reinterpret_cast', and `const_cast' because 1) it says
what you mean but no more, and 2) it's less prone to side effects, and
3) if you try to do something really crazy it won't let you and you will
have a chance to rethink things before you shoot yourself in the foot;)
> * -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.
I do this sometimes on purpose and wouldn't normally want to see
warnings on it. I would value building with this turned on occasionally
and making sure that I'd done it on purpose. Some names ask to be reused.
> * -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.
That's a good one. It would catch when your implementation had a typo
in the name.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk