Re: [boost] Official warnings policy?

Subject: Re: [boost] Official warnings policy?
From: Gottlob Frege (gottlobfrege_at_[hidden])
Date: 2009-11-06 00:53:50

On Thu, Nov 5, 2009 at 1:45 PM, Sid Sacek <ssacek_at_[hidden]> wrote:
> -----Original Message-----
>>> [ Steven Watanabe wrote ]
>>> I am not convinced that tracing through all the noise from a dozen
>>> different compilers is worth the effort from that standpoint.
>>> As far as I am concerned, the primary reason for eliminating
>>> warnings is that they are annoying to users.
>
> Don't you believe that with the right syntax a coder can get every compiler to become happy?
>

No.  I'm sorry I don't remember the details, but I've had code where
fixing a warning in MSVC caused a warning in gcc, and then vice versa.
 There really wasn't a reasonable way to avoid it via code changes.
We had to change the code to make gcc happy, then use pragmas to
suppress MSVC.

But I will say we still did it - ie we still found it valuable to get
to zero warnings.

And while I'm at it, since I've gone through this process with
sizeable projects (ie Adobe applications):

1. Being at 0 warnings is worth it.  Important to remember.
2. Once you get there, stay there.  Much easier than getting there.
3. Fixing warnings can fix bugs.
4. Fixing warnings can *cause* bugs.
    For large 'sprints' of warning fixes, I'd say it causes more bugs
than it finds.  Way more.  Once you get to 0, warnings will find more
bugs than they cause, but not during the drive to 0.  The key here is
to make sure the person fixing the warning knows the code well.  Just
because you fixed the warning the same way the last 10 times, it
doesn't mean the next occurrence will work the same way.  And again,
once you get to 0, new warnings will be fixed by their instigator, and
fixed when the code is fresh in their minds - much more likely to be
correct.

Some specific warnings, and our conclusions:

- missing virtual dtor
   - didn't find any bugs in our code, but it is not a bad warning in
general, when you think about who might be using/maintaining your code
in the future.
   - solution:  add virtual dtor, make it protected, leave a comment
about the warning

- missing enum values in switch case
   - didn't find any bugs
   - seemed like a nice idea at first - lets you know if you missed a
case.  We tried various ways to restructure our code to take advantage
of this warning (ie write the code so that if a new enum value was
added you would get a warning about the switch).  However, more often
than not, the restructuring introduced bugs, or made the code more
convoluted or ... etc (typically with how to deal with "default:"
case).  Maybe we were being "too smart by half" trying to make use of
the warning.
   - I'd disable this warning if I could, otherwise I'd fix the code

- order of member initialization list does not match order actual initialization
  (ie members are initialized in the order listed inside the class,
not the order in the constructor mem-init list)
   - didn't find any bugs
   - you shouldn't rely on member init order, and if you ever need to,
you should really comment it well
   - reordering your mem-int lists is an annoying waste of time
   - the warning basically says "hey, know your language".  Should we
warn whenever you use the comma operator or ?: because they esoteric
parts of the language? (I once worked somewhere where these were
banned for that reason...)
   - I'd disable this warning if I could, otherwise I'd fix the code

etc.  Can't remember the rest, but these 3 took up a lot of time in
our code.  The other was MS warnings for 64bit compatibility.  And
changing int to size_t without looking leads to bugs about how
negative numbers are (not) handled...

Note that as much as I hate some of the warnings, and for us overall
the fixes caused more problems than they solved, I still think it is
worth doing.  CAREFULLY.

Tony