Re: [boost] Official warnings policy?

Subject: Re: [boost] Official warnings policy?
From: Stewart, Robert (Robert.Stewart_at_[hidden])
Date: 2009-11-11 16:39:59

Daniel James wrote:
> 2009/11/9 Stewart, Robert <Robert.Stewart_at_[hidden]>:
>
> > If reviewers are expected to judge code quality and it
> > produces myriad
> > warnings with established warnings settings, what might
> > reviewers conclude?
>
> That it was developed with a different compiler or with different
> settings?

Certainly.  Some may also judge the quality as lower.  When I get warnings from platform headers, I'm highly annoyed because I usually don't want to take the time to determine whether the warning is so much noise or points to a real problem.  I typically assume the former, but it leaves me feeling less comfortable.

Note that I have often added the caveat that this would apply to the officially supported compilers.  I would also expect that should a library not support one of those compilers that the review announcement and documentation would so indicate.

> Warnings are useful tool, but they aren't necessary or
> sufficient for quality.

I agree and never said otherwise.

> Would you really reject a library because of
> unreferenced parameters? Shouldn't a reviewer be looking at more
> meaningful indicators of quality (eg. documentation, test coverage).

I wouldn't and never meant to suggest otherwise.  However, large numbers of warnings on supported platforms with officially sanctioned warnings flags would give me pause.

> > There could be a required evaluation step after the usual
> > review to provide
> > final acceptance.  That would make it easier to accept
> > warnings in a library
> > under review.  If the author does not meet the (not yet)
> > established warnings
> > policy after a tentative review acceptance but before final
> > acceptance, then it would be rejected.
>
> We already have something like that. Before a new library can be
> merged to the release branch it has to be approved by the release
> managers. One of the criteria is decent regression test results. So if
> we had regression tests running with warnings as errors, this would be
> checked before release.

We don't have warnings-as-errors tests yet and there isn't a documented requirement to hold up a new library for zero warnings.  Nevertheless, what you describe is consistent with what I suggested.

> > That subjective evaluation reflects your view that
> > compiling with zero
> > warnings has no bearing on code quality.  Others have a
> > differing view.
>
> Since you want extra requirements added to the review process, it's

I didn't say I did.  I have suggested that it might be worthwhile.  I have suggested some means by which it could be added were it deemed worthwhile.

> actually up to you to demonstrate that requiring warning free code
> will be sufficiently beneficial. So just saying that my views are
> subjective is not enough. And the thing is, I've also got a fine
> counter-example, Boost itself.

Reducing or eliminating the warnings enables folks to use Boost that now cannot.  It also means that the intelligence built into today's compilers is considered.  Bugs *are* found by investigating warnings, though often not.  The majority of warnings are noise in code as well tested and used as Boost, but warnings from Boost headers prevent Boost users from finding their own mistakes.

> > If Boost makes zero warnings a goal for all libraries and a
> > requirement for
> > all new libraries, at established warnings settings, then
> > it will be because
> > there is consensus that there is value in so doing,
> > regardless of individual notions.
>
> The main reason we're working on warning free code is so that users
> who like to compile with warnings on won't have a lot of distracting
> noise in their compiler's output. Which is surely only a requirement
> at the time of release.

There is value in addressing warnings because they *may* reveal bugs in Boost code, but you're right.  The *main* reason is for user experience.  However, it is hard to get to zero warnings, much as it is hard to add const-correctness after the fact.  It must be an ongoing effort, not simply addressed at release time, if it is to be done.

_____
Rob Stewart                           robert.stewart_at_[hidden]
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.