Official warnings policy?

[John Maddock]

I'm *not* saying we should do this for 1.41, but should we have an official
policy regarding compiler warnings and which ones we regard as "failures"?

I realize these can get pretty busy-body at times, but if the user sees
several pages of warnings when building Boost it doesn't look so good.  So
my suggestion would be that we have two test-runners (if we have any spare!)
that build with warnings-as-errors, maybe:

-Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror

For gcc and:

/W3 /WX

for MSVC?

Obviously these may prove to be far too busy-body, but is this worth a try?

Cheers, John.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Vladimir Prus-3]

John Maddock wrote:

> I'm *not* saying we should do this for 1.41, but should we have an official
> policy regarding compiler warnings and which ones we regard as "failures"?
>
> I realize these can get pretty busy-body at times, but if the user sees
> several pages of warnings when building Boost it doesn't look so good.  So
> my suggestion would be that we have two test-runners (if we have any spare!)
> that build with warnings-as-errors, maybe:
>
> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror

I would remove -pedantic, but otherwise, it's a very good idea. Unfortunately,
recent discussion left me with the impression that few folks care.

- Volodya


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

John Maddock wrote:
>
> I'm *not* saying we should do this for 1.41, but should we
> have an official
> policy regarding compiler warnings and which ones we regard
> as "failures"?

If Boost documents the warnings settings used for each compiler, then all maintainers will have a consistent target while users know exactly what to expect.  Obviously, individual maintainers may use stricter settings or cater to clients that do, but a minimum, consistent, documented policy would be highly useful for all concerned.

There should also be some policies about the sorts of warnings that are considered nuisance and will not be addressed.

_____
Rob Stewart                           robert.stewart@...
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.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

John Maddock wrote:

>
> my suggestion would be that we have two test-runners (if we
> have any spare!)
> that build with warnings-as-errors, maybe:
>
> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror
>
> For gcc and:
>
> /W3 /WX
>
> for MSVC?

That won't work well for a test-runner will it?  On the first warning, the build will fail.  It would be better to have a report of all warnings that must be addressed.

FWIW, our build system's current settings for GCC are:

 -Wall -Wno-comment -Wpointer-arith -W -Wconversion -Wno-long-long

_____
Rob Stewart                           robert.stewart@...
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.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Mateusz Loskot]

John Maddock wrote:

> I'm *not* saying we should do this for 1.41, but should we have an
> official policy regarding compiler warnings and which ones we regard
> as "failures"?
>
> I realize these can get pretty busy-body at times, but if the user
> sees several pages of warnings when building Boost it doesn't look so
> good. So my suggestion would be that we have two test-runners (if we
> have any spare!) that build with warnings-as-errors, maybe:
>
> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror

By the way, building Boost gives large number of warnings related to
strict aliasing. I reported it as ticket with one big log, but I
understand it is not really usable, so I'm going to prepare report per
library. Is it a good idea at all?

> Obviously these may prove to be far too busy-body, but is this worth
> a try?

I'm not an author of any of Boost libraries, but if I may answer this,
I think it is worth.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[John Maddock]

> That won't work well for a test-runner will it?  On the first warning, the
> build will fail.  It would be better to have a report of all warnings that
> must be addressed.

The issue is that the warnings are otherwise too easy to ignore.  I fear
that unless they are made into errors for some test runners - and those
runners are part of the "required" set - then nothing will get done :-(

John.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[John Maddock]

> I would remove -pedantic, but otherwise, it's a very good idea.
> Unfortunately,
> recent discussion left me with the impression that few folks care.

If they were made into errors then they would be forced to care ;-)

One issue is that currently we just don't get to see which tests are
producing warnings (unless we run them locally), back in the day, before the
current testing infrastructure (and when Boost was much smaller) we did get
to see warnings on run tests, and some folks at least were fixing them.

John.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

John Maddock wrote:

>
> > That won't work well for a test-runner will it?  On the
> > first warning, the
> > build will fail.  It would be better to have a report of
> > all warnings that
> > must be addressed.
>
> The issue is that the warnings are otherwise too easy to
> ignore.  I fear
> that unless they are made into errors for some test runners -
> and those
> runners are part of the "required" set - then nothing will
> get done :-(

<nod>

Unfortunately, if maintainers can only address one warning at a time, for a platform not otherwise available, fixing all will be really annoying.

How about a policy that a release will be held up if warnings-as-errors fails?  Then an impending release will significantly increase the importance of the warnings report not being clean.

_____
Rob Stewart                           robert.stewart@...
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.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[John Maddock]

> By the way, building Boost gives large number of warnings related to
> strict aliasing. I reported it as ticket with one big log, but I
> understand it is not really usable, so I'm going to prepare report per
> library. Is it a good idea at all?

Absolutely, once the issue is reported, folks will hopefully take notice.
And yes, one report per library please so the bug report goes to the right
person.

John.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Bo Persson]

Stewart, Robert wrote:

> John Maddock wrote:
>>
>> my suggestion would be that we have two test-runners (if we
>> have any spare!)
>> that build with warnings-as-errors, maybe:
>>
>> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror
>>
>> For gcc and:
>>
>> /W3 /WX
>>
>> for MSVC?
>
> That won't work well for a test-runner will it?  On the first
> warning, the build will fail.  It would be better to have a report
> of all warnings that must be addressed.
>

Yes, but it will still not satisfy those who compile at /W4.  :-)


Bo Persson



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

Bo Persson wrote:
>
> Yes, but it will still not satisfy those who compile at /W4.  :-)

One step at a time.  If we get /W3 then you can present your case to move to /W4.

_____
Rob Stewart                           robert.stewart@...
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.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Joel Falcou-3]

Vladimir Prus wrote:
> I would remove -pedantic, but otherwise, it's a very good idea. Unfortunately,
> recent discussion left me with the impression that few folks care.
>  
They def. should care. I am all for this proposal too.

--
___________________________________________
Joel Falcou - Assistant Professor
PARALL Team - LRI - Universite Paris Sud XI
Tel : [phone]


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Mateusz Loskot]

Bo Persson wrote:

> Stewart, Robert wrote:
>> John Maddock wrote:
>>> my suggestion would be that we have two test-runners (if we have
>>> any spare!) that build with warnings-as-errors, maybe:
>>>
>>> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror
>>>
>>> For gcc and:
>>>
>>> /W3 /WX
>>>
>>> for MSVC?
>> That won't work well for a test-runner will it?  On the first
>> warning, the build will fail.  It would be better to have a report
>> of all warnings that must be addressed.
>>
>
> Yes, but it will still not satisfy those who compile at /W4.  :-)

I've tracked quite interesting discussion about /W3 vs /W4
related to Intel Threading Building Blocks

http://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/61895/

Where Alexey Kukanov draws conclusion as follows:

"So for MSVC, /W3 seems most reasonable level of warnings, and it
is the default one as far as I remember."

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Emil Dotchevski-3]

On Wed, Nov 4, 2009 at 10:03 AM, Mateusz Loskot <mateusz@...> wrote:

> John Maddock wrote:
>>
>> I'm *not* saying we should do this for 1.41, but should we have an
>> official policy regarding compiler warnings and which ones we regard
>> as "failures"?
>>
>> I realize these can get pretty busy-body at times, but if the user
>> sees several pages of warnings when building Boost it doesn't look so
>> good. So my suggestion would be that we have two test-runners (if we
>> have any spare!) that build with warnings-as-errors, maybe:
>>
>> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror
>
> By the way, building Boost gives large number of warnings related to
> strict aliasing.

As long as these warnings report actual violation of the C++ standard
(as opposed to warning about a potential for violation) they should be
fixed IMO.

Emil Dotchevski
Reverge Studios, Inc.
http://www.revergestudios.com/reblog/index.php?n=ReCode
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Zachary Turner-2]

On Wed, Nov 4, 2009 at 12:29 PM, Emil Dotchevski
<emildotchevski@...>wrote:

> On Wed, Nov 4, 2009 at 10:03 AM, Mateusz Loskot <mateusz@...>
> wrote:
> > John Maddock wrote:
> >>
> >> I'm *not* saying we should do this for 1.41, but should we have an
> >> official policy regarding compiler warnings and which ones we regard
> >> as "failures"?
> >>
> >> I realize these can get pretty busy-body at times, but if the user
> >> sees several pages of warnings when building Boost it doesn't look so
> >> good. So my suggestion would be that we have two test-runners (if we
> >> have any spare!) that build with warnings-as-errors, maybe:
> >>
> >> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror
> >
> > By the way, building Boost gives large number of warnings related to
> > strict aliasing.
>
> As long as these warnings report actual violation of the C++ standard
> (as opposed to warning about a potential for violation) they should be
> fixed IMO.

I'm not sure I agree.  If something is a violation of the C++ standard the
compiler shouldn't be reporting it as an error in the first place.  For
example, implicit conversion between numeric types losing precision is going
to issue a warning on MSVC (not sure about GCC) but there's no violation of
the standard anywhere.  Perhaps this is what you meant and I'm just taking
what you said too literally.



That aside, should we consider how to deal with #pragma warning(disable) in
MSVC (do other compilers have similar pragmas)?  Should it be disallowed?
 Otherwise it's all too easy for a library maintainer who doesn't want to
fix a certain warning to just stick a pragma in there to disable it.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Zachary Turner-2]

On Wed, Nov 4, 2009 at 12:56 PM, Zachary Turner <divisortheory@...>wrote:
>
> I'm not sure I agree.  If something is a violation of the C++ standard the
> compiler shouldn't be reporting it as an error in the first place.
>
Correction.  I meant it shouldn't be reporting it as a *warning* in the
first place, it should be reporting it as an error.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Emil Dotchevski-3]

On Wed, Nov 4, 2009 at 9:24 AM, Vladimir Prus <vladimir@...> wrote:

> John Maddock wrote:
>
>> I'm *not* saying we should do this for 1.41, but should we have an official
>> policy regarding compiler warnings and which ones we regard as "failures"?
>>
>> I realize these can get pretty busy-body at times, but if the user sees
>> several pages of warnings when building Boost it doesn't look so good.  So
>> my suggestion would be that we have two test-runners (if we have any spare!)
>> that build with warnings-as-errors, maybe:
>>
>> -Wall -pedantic -Wstrict-aliasing -fstrict-aliasing -Werror
>
> I would remove -pedantic, but otherwise, it's a very good idea.

This is the problem: *you* would remove -pedantic, but others want it.

> Unfortunately,
> recent discussion left me with the impression that few folks care.

It is not about caring, once again the argument is about a personal
preference: is the ugliness and decreased readability that is often
required to silence a warning reasonable.

Emil Dotchevski
Reverge Studios, Inc.
http://www.revergestudios.com/reblog/index.php?n=ReCode
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Emil Dotchevski-3]

On Wed, Nov 4, 2009 at 10:56 AM, Zachary Turner <divisortheory@...> wrote:

> On Wed, Nov 4, 2009 at 12:29 PM, Emil Dotchevski
> <emildotchevski@...>wrote:
>> On Wed, Nov 4, 2009 at 10:03 AM, Mateusz Loskot <mateusz@...>
>> wrote:
>> > John Maddock wrote:
>> > By the way, building Boost gives large number of warnings related to
>> > strict aliasing.
>>
>> As long as these warnings report actual violation of the C++ standard
>> (as opposed to warning about a potential for violation) they should be
>> fixed IMO.
>
> I'm not sure I agree.  If something is a violation of the C++ standard the
> compiler shouldn't be reporting it as an error in the first place.  For
> example, implicit conversion between numeric types losing precision is going
> to issue a warning on MSVC (not sure about GCC) but there's no violation of
> the standard anywhere.  Perhaps this is what you meant and I'm just taking
> what you said too literally.

My comment was specifically about strict aliasing warnings. I meant
that if the compiler warns that reinterpret_cast is dangerous and
tricky, that's not news to me. :)

For example, a reinterpret_cast of a pointer may result in breaking
strict aliasing rules only if the resulting pointee type is
sufficiently different (strict aliasing rules define a few exceptions)
and only if the target pointer is actually dereferenced. At this point
there is a violation of the standard and the warning should be fixed:
another compiler could issue an error instead (at least in theory, in
practice strict aliasing rules are broken quite often in C.)

Emil Dotchevski
Reverge Studios, Inc.
http://www.revergestudios.com/reblog/index.php?n=ReCode
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Steven Watanabe-4]

AMDG

Zachary Turner wrote:
> That aside, should we consider how to deal with #pragma warning(disable) in
> MSVC (do other compilers have similar pragmas)?  Should it be disallowed?
>  

No.  The majority of warnings that I see from Boost are just noise.
Sometimes these are really annoying to get rid of without #pragmas.

> Otherwise it's all too easy for a library maintainer who doesn't want to
> fix a certain warning to just stick a pragma in there to disable it.

I don't think its a good idea to make rules assuming that library
maintainers are going to irresponsibly suppress serious warnings.
If this happens a lot we can worry about it then--but I think
we'd have more serious problems than warnings at that point.

In Christ,
Steven Watanabe

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Simonson, Lucanus J]

Zachary Turner wrote:
> That aside, should we consider how to deal with #pragma
> warning(disable) in MSVC (do other compilers have similar pragmas)?
>  Should it be disallowed? Otherwise it's all too easy for a library
> maintainer who doesn't want to
> fix a certain warning to just stick a pragma in there to disable it.

EDG frontends supports a similar system for pragmas that disable warnings, so that covers several compilers.

If only it were true that pragmas were easy to use to disable warnings in templates.  Library users don't tolerate #pragma waring disable in header files that change the warning policy for their own code.  That means you have to restore the warning setting with #pragma warning default.  However, for templates the warning policy observed is often that of the line in which the template is instantiated and not where it is defined.  That means you have to tightly wrap the line of code that produces the warning within the template definition with pragma disable and pragma default.  If the line that produces the warning is a function or class declaration then you sometimes cannot disable the warning with pragmas at the point where the declaration is made.  There are several effective C++ warnings in particular that cannot be suppressed in template code without disabling them globally.  

Fixing warnings tends to be as easier than pragma disabling them most of the time.

Regards,
Luke
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost