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 : (+33)1 69 15 66 35

Joel de Guzman wrote:

I've never been a bjam expert, but what is the bjam flag that switches on warnings? On my part, I could just switch it on every time I do local testing.

Regards,

Looks like you have a couple built-in options. Just add to your bjam command line (example: bjam warnings=all release). warnings=on gcc: -Wall msvc: /W3 warnings=off gcc: -w msvc: /W0 warnings=all gcc: -Wall -pedantic msvc: /W4 warnings-as-errors=on gcc: -Werror msvc: /Wx HTH michael -- ---------------------------------- Michael Caisse Object Modeling Designs www.objectmodelingdesigns.com

Herb Sutter and Andrei Alexandrescu (who is following this discussion on boost) were kind enough to give me permission to quote their item 1 from C++ Coding Standards (note that I screen scraped this from [1]www.artima.com who also had their permission to post) You might consider buying a copy, it's a wonderful book;) : 1. Compile cleanly at high warning levels. Summary Take warnings to heart: Use your compiler's highest warning level. Require clean (warning-free) builds. Understand all warnings. Eliminate warnings by changing your code, not by reducing the warning level. Discussion Your compiler is your friend. If it issues a warning for a certain construct, often there's a potential problem in your code. Successful builds should be silent (warning-free). If they aren't, you'll quickly get into the habit of skimming the output, and you will miss real problems. (See Item 2.) To get rid of a warning: a) understand it; and then b) rephrase your code to eliminate the warning and make it clearer to both humans and compilers that the code does what you intended. Do this even when the program seemed to run correctly in the first place. Do this even when you are positive that the warning is benign. Even benign warnings can obscure later warnings pointing to real dangers. Examples Example 1: A third-party header file. A library header file that you cannot change could contain a construct that causes (probably benign) warnings. Then wrap the file with your own version that #includes the original header and selectively turns off the noisy warnings for that scope only, and then #include your wrapper throughout the rest of your project. Example (note that the warning control syntax will vary from compiler to compiler): // File: myproj/my_lambda.h -- wraps Boost's lambda.hpp // Always include this file; don't use lambda.hpp directly. // NOTE: Our build now automatically checks "grep lambda.hpp <srcfile>". // Boost.Lambda produces noisy compiler warnings that we know are innocuous. // When they fix it we'll remove the pragmas below, but this header will still exist. // #pragma warning(push) // disable for this header only #pragma warning(disable:4512) #pragma warning(disable:4180) #include <boost/lambda/lambda.hpp> #pragma warning(pop) // restore original warning level Example 2: "Unused function parameter." Check to make sure you really didn't mean to use the function parameter (e.g., it might be a placeholder for future expansion, or a required part of a standardized signature that your code has no use for). If it's not needed, simply delete the name of a function parameter: // ... inside a user-defined allocator that has no use for the hint ... // warning: "unused parameter 'localityHint'" pointer allocate( size_type numObjects, const void *localityHint = 0 ) { return static_cast<pointer>( mallocShared( numObjects * sizeof(T) ) ); } // new version: eliminates warning pointer allocate( size_type numObjects, const void * /* localityHint */ = 0 ) { return static_cast<pointer>( mallocShared( numObjects * sizeof(T) ) ); } Example 3: "Variable defined but never used." Check to make sure you really didn't mean to reference the variable. (An RAII stack-based object often causes this warning spuriously; see Item 13.) If it's not needed, often you can silence the compiler by inserting an evaluation of the variable itself as an expression (this evaluation won't impact run-time speed): // warning: "variable 'lock' is defined but never used" void Fun() { Lock lock; // ... } // new version: probably eliminates warning void Fun() { Lock lock; lock; // ... } Example 4: "Variable may be used without being initialized." Initialize the variable (see Item 19). Example 5: "Missing return." Sometimes the compiler asks for a return statement even though your control flow can never reach the end of the function (e.g., infinite loop, throw statements, other returns). This can be a good thing, because sometimes you only think that control can't run off the end. For example, switch statements that do not have a default are not resilient to change and should have a default case that does assert( false ) (see also Items 68 and 90): // warning: missing "return" int Fun( Color c ) { switch( c ) { case Red: return 2; case Green: return 0; case Blue: case Black: return 1; } } // new version: eliminates warning int Fun( Color c ) { switch( c ) { case Red: return 2; case Green: return 0; case Blue: case Black: return 1; default: assert( !"should never get here!" ); // !"string" evaluates to false return -1; } } Example 6: "Signed/unsigned mismatch." It is usually not necessary to compare or assign integers with different signedness. Change the types of the variables being compared so that the types agree. In the worst case, insert an explicit cast. (The compiler inserts that cast for you anyway, and warns you about doing it, so you're better off putting it out in the open.) Exceptions Sometimes, a compiler may emit a tedious or even spurious warning (i.e., one that is mere noise) but offer no way to turn it off, and it might be infeasible or unproductive busywork to rephrase the code to silence the warning. In these rare cases, as a team decision, avoid tediously working around a warning that is merely tedious: Disable that specific warning only, disable it as locally as possible, and write a clear comment documenting why it was necessary. References [2][Meyers97] §48 • [3][Stroustrup94] §2.6.2 References 1. http://www.artima.com/ 2. http://www.artima.com/cppsource/codestandards3.html#biblio 3. http://www.artima.com/cppsource/codestandards3.html#biblio

On Fri, Nov 6, 2009 at 8:37 PM, David Abrahams <dave@boostpro.com> wrote:

on Wed Nov 04 2009, joel <joel.falcou-AT-lri.fr> wrote:

...

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.

As long as it doesn't force us to do stupid things, like add a virtual destructor just because something is used as an implementation-detail base class, I'm all for making warnings errors and fixing all of them. The big problem is that the definition of "stupid" is subjective.  For example, any set of warnings that forced me to write

        if (0 == i)

instead of

        if (i == 0)

would make coding a lot less fun for me.  I guess I would be willing to submit to it, for the sake of Boost, if we decided on something that. So I guess we'll just have to try it and see how much pain it causes.

That's my feeling too. From the perspective of release management, I'm willing to delay 1.41.0 long enough for a second beta to clear as many warnings as possible during the next week. But I don't want to disrupt the whole release schedule. And I totally agree that we don't want to force developers to write stupid code, and we have to acknowledge that each developer will have a somewhat different definition of "stupid code". So, yes, let's give it a try and see where it takes us. --Beman

Emil Dotchevski wrote:

On Wed, Nov 4, 2009 at 9:24 AM, Vladimir Prus <vladimir@codesourcery.com> 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.

I would be happy about having -pedantic there, if there's demand. But my guess that putting it there now will result in every single test being a failure.

...

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.

I suggest we don't talk in the abstract. Once a specific set of warning options, together with -Werror is in place, you can raise your concerns about any particular warning emitted by any particular compiler, and hopefully, some per-warning-kind agreement can be reached. If we don't do anything at all because *some* warning in *some* library *might* require changes that are not acceptable to that library maintainer, we'll find ourself in the situation where a complete build of Boost produces a pile of warning, including "there's 99.9% chance your program will crash at runtime" warnings reported earlier. - Volodya

Emil Dotchevski wrote:

However this will not address the issue at hand, which is that people who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35.

This is muchly tongue in cheeky. You can build their code with high warning levels and don't get warnings. zlib is useful and clean. Well written code. I like it. Patrick

Emil Dotchevski wrote:

On Wed, Nov 4, 2009 at 12:12 PM, Vladimir Prus <vladimir@codesourcery.com> wrote:

...

Emil Dotchevski wrote:

...

...

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.

I suggest we don't talk in the abstract. Once a specific set of warning options, together with -Werror is in place, you can raise your concerns about any particular warning emitted by any particular compiler, and hopefully, some per-warning-kind agreement can be reached.

I agree that the only way warnings can be addressed effectively is to use -Werror. On the other hand, the idea that a warning is the same as an error challenges my world view. :)

I understand why you say that we can't talk in the abstract. It's downright silly not to fix certain "good" warnings and we, as a community, definitely can agree on a reasonable definition of "good".

However this will not address the issue at hand, which is that people who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35.

Yes, that's a good attitude. However, how are we going to be sure that the code works, when the compiler says it might not? The original example of a newer release pointing out that old type punning is illegal, is really frightening. Bo Persson

On Wed, Nov 4, 2009 at 4:05 PM, Bo Persson <bop@gmb.dk> wrote:

Emil Dotchevski wrote:

...

On Wed, Nov 4, 2009 at 12:12 PM, Vladimir Prus <vladimir@codesourcery.com> wrote:

...

Emil Dotchevski wrote:

...

...

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.

I suggest we don't talk in the abstract. Once a specific set of warning options, together with -Werror is in place, you can raise your concerns about any particular warning emitted by any particular compiler, and hopefully, some per-warning-kind agreement can be reached.

I agree that the only way warnings can be addressed effectively is to use -Werror. On the other hand, the idea that a warning is the same as an error challenges my world view. :)

I understand why you say that we can't talk in the abstract. It's downright silly not to fix certain "good" warnings and we, as a community, definitely can agree on a reasonable definition of "good".

However this will not address the issue at hand, which is that people who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35.

Yes, that's a good attitude. However, how are we going to be sure that the code works, when the compiler says it might not?

It's only a good attitude if the company you work for allows it to be. Many companies have very strict code policies that actually prevent you from comitting code to the repository if it results in the compiler generating a warning. It's depressing that boost can't be used at all in such a company for something so minor as a compiler warning.

Emil Dotchevski wrote:

On Wed, Nov 4, 2009 at 2:05 PM, Bo Persson <bop@gmb.dk> wrote:

...

...

On Wed, Nov 4, 2009 at 12:12 PM, Vladimir Prus <vladimir@codesourcery.com> wrote:

...

Emil Dotchevski wrote:

...

...

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. I suggest we don't talk in the abstract. Once a specific set of warning options, together with -Werror is in place, you can raise your concerns about any particular warning emitted by any particular compiler, and hopefully, some per-warning-kind agreement can be reached. I agree that the only way warnings can be addressed effectively is to use -Werror. On the other hand, the idea that a warning is the same as an error challenges my world view. :)

I understand why you say that we can't talk in the abstract. It's downright silly not to fix certain "good" warnings and we, as a community, definitely can agree on a reasonable definition of "good".

However this will not address the issue at hand, which is that people who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35. Yes, that's a good attitude. However, how are we going to be sure that

Emil Dotchevski wrote: the code works, when the compiler says it might not?

How we are going to be sure that the code works is not a simple question, but note that "fixing" a warning simply silences the compiler and (ideally) has no effect on the correctness of the code: if the code was buggy, it still is.

It is not true. Fixing a warning is supposed to fix it but not "fix" it. A warning fix may involve steps like: - fix bug in code or refactor correct code to improve quality - if code is perfectly valid, but just compiler overreacted, silent warning with appropriate annotation in code - refine compiler flags. - do not touch code, but comment/document the status and give reason why no action is to be taken Best regards, -- Mateusz Loskot, http://mateusz.loskot.net Charter Member of OSGeo, http://osgeo.org

Emil Dotchevski wrote:

On Wed, Nov 4, 2009 at 2:05 PM, Bo Persson <bop@gmb.dk> wrote:

...

Emil Dotchevski wrote:

...

On Wed, Nov 4, 2009 at 12:12 PM, Vladimir Prus <vladimir@codesourcery.com> wrote:

...

Emil Dotchevski wrote:

...

...

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.

I suggest we don't talk in the abstract. Once a specific set of warning options, together with -Werror is in place, you can raise your concerns about any particular warning emitted by any particular compiler, and hopefully, some per-warning-kind agreement can be reached.

I agree that the only way warnings can be addressed effectively is to use -Werror. On the other hand, the idea that a warning is the same as an error challenges my world view. :)

I understand why you say that we can't talk in the abstract. It's downright silly not to fix certain "good" warnings and we, as a community, definitely can agree on a reasonable definition of "good".

However this will not address the issue at hand, which is that people who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35.

Yes, that's a good attitude. However, how are we going to be sure that the code works, when the compiler says it might not?

How we are going to be sure that the code works is not a simple question, but note that "fixing" a warning simply silences the compiler and (ideally) has no effect on the correctness of the code: if the code was buggy, it still is.

(Qualifying the above with "ideally" is necessary because sometimes fixing a lame warning on one platform could also silence a valid warning on another platform.)

I'm not arguing that the code should be obfuscated to satisfy every compiler, but that there could be an official policy that some warnings are really silly and disabled. Perhaps they could be enabled by defining _BOOST_ENABLE_SILLY_MSVC_WARNINGS? Or just document that the code compiles cleanly if I disable the silly warnings. Having Boost compiling at /W2, possibly aiming for /W3, is not impressing on those who's code already compile at /W4 (with a few really silly warnings disabled). Note that VC10 has an additional /Wall level, higher than /W4. :-) Bo Persson

On Wed, Nov 4, 2009 at 3:54 PM, Mateusz Loskot <mateusz@loskot.net> wrote:

Emil Dotchevski wrote:

...

However this will not address the issue at hand, which is that people  who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35.

This attitude is a polite excuse with no practical rationale behind. One may ask, how they can make sure their code works with my compiler if I see number of warnings that suggest some dirty hacks around aliasing are used, so potential undefined behaviour is handing in the air.

Yes, but not all warnings are like that. Many warnings inform about tricky semantics or tricky side effects, like "ohai operator| has very low precedence, consider using parens." Some warnings are plain wrong, like the "you don't have a virtual destructor!!!!!" warning in GCC. Yet others help enforce coding standards. Secondly, sometimes practicality is the name of the game, and theoretical failures are just that, theoretical. For example, I try to be careful but I wouldn't bet that all of my code will behave on a platform where CHAR_BIT isn't 8.

However, "never ignore" does not necessary mean always fix your code to silent warnings. It means that if warning is reported, it should be analysed what the complain is about and action should be taken: fix code or silent warning or ignore. Ignore after check is fine, as long as "never ignore warnings" approach is followed.

Fine, but this is not what the issue is about, I don't think. The problem is that there are companies that require -Wall -Werror or some such. Otherwise, what you're describing sounds like common sense to me. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

on Wed Nov 04 2009, Emil Dotchevski <emildotchevski-AT-gmail.com> wrote:

However this will not address the issue at hand, which is that people who use higher warning levels will see tons of warnings. A better attitude is http://www.zlib.net/zlib_faq.html#faq35.

Our project is substantially different from zlib, though. I'm sure zlib's headers don't trigger warnings, but our warnings show up for our clients in their day-to-day work. -- Dave Abrahams Meet me at BoostCon: http://www.boostcon.com BoostPro Computing http://www.boostpro.com

AMDG Patrick Horgan wrote:

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.

-pedantic issues all the warnings required by strict ISO C and ISO C++. They are in the standard for a reason, and I think it fair to expect system code that I build my code on to pass it. It's the same parts of boost that always generate warnings. Other parts are always free of warnings. Not rocket science to know that some are building better code.

Even the problems reported by -pedantic are not necessarily real problems. For example, it's a huge pain to use long long with -pedantic, even though all uses in boost should be protected by #ifdefs. In Christ, Steven Watanabe

Steven Watanabe wrote:

AMDG

Patrick Horgan wrote:

...

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.

-pedantic issues all the warnings required by strict ISO C and ISO C++. They are in the standard for a reason, and I think it fair to expect system code that I build my code on to pass it. It's the same parts of boost that always generate warnings. Other parts are always free of warnings. Not rocket science to know that some are building better code.

Even the problems reported by -pedantic are not necessarily real problems. For example, it's a huge pain to use long long with -pedantic, even though all uses in boost should be protected by #ifdefs.

Sure, but you can turn off -pedantic warnings individually. I use -pedantic -Wall -Wno-long-long -Wno-unused-value a lot. Cheers, Ian

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Simonson, Lucanus J Sent: Thursday, November 05, 2009 4:09 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

If we are going to have an official boost policy for warnings we need a list of flags for each major compiler that we can all agree to.

Previous attempts to get agreement on this have not been very successful one man's meat proved another poison ;-) I concluded that every author had to do his own thing for his own warnings - but do his best to get rid of them 'locally' to his module without inflicting his choice on everyone. Not all compiler have this fine-grained enough mechanism, but then the info in the build info should allow users to add their own choice of 'not-useful' warnings. https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines is a first go at this. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com

On Thu, Nov 5, 2009 at 12:07 PM, Paul A. Bristow <pbristow@hetp.u-net.com> wrote:

https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines

is a first go at this.

Turning off Microsoft extensions (/Za) for the Filesystem library doesn't work; the library implementation for Windows uses Microsoft headers that use Microsoft extensions. So you need to qualify the turn off compiler specific extensions advise. Perhaps change "For Microsoft Visual Studio, this means setting level to 4 (command line /W4), and disabling Microsoft language extensions Disable MS extensions = Yes, command line option /Za " to: "For Microsoft Visual Studio, this means setting level to 4 (command line /W4). For code that doesn't deliberately use Microsoft language extensions, disable them with Disable MS extensions = Yes, command line option /Za" --Beman

On Fri, Nov 6, 2009 at 8:33 AM, Beman Dawes <bdawes@acm.org> wrote:

On Thu, Nov 5, 2009 at 12:07 PM, Paul A. Bristow and disabling Microsoft language extensions Disable MS extensions = Yes, command line option /Za "

Stephan Lavavej from Microsoft told me that that this option triggers known bugs in the compiler. He strongly recommends to stay away from it. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

On 07/11/2009 10:38, Paul A. Bristow wrote:

...

Stephan Lavavej from Microsoft told me that that this option triggers known bugs in the compiler. He strongly recommends to stay away from it.

Sounds like spreading FUD (Fear Uncertainty and Doubt) to me.

Why would he do that for his own company's compiler?

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Beman Dawes Sent: Saturday, November 07, 2009 1:47 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

On Sat, Nov 7, 2009 at 7:13 AM, Daniel James <daniel_james@fmail.co.uk> wrote:

...

On 07/11/2009 10:38, Paul A. Bristow wrote:

...

...

Stephan Lavavej from Microsoft told me that that this option triggers known bugs in the compiler. He strongly recommends to stay away from it.

Sounds like spreading FUD (Fear Uncertainty and Doubt) to me.

Why would he do that for his own company's compiler?

Stephan is a well-respected member of the VC++ team. If he recommends staying away from a particular feature, we can rely on that as good advice.

Well I find it worrying advice - especially as I've followed my own advice without detecting any ill effects. (Hopefully it is fixed in the next compiler release ;-) If library writers find it works OK, we users are more confident that it is portable and 'more' C++ standard compliant. If it causes trouble (as it surely will as you pointed out with Boost.Filesystem) they can simply document that using \Za doesn't work. No problem. Meanwhile, I've tidied up a bit at https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines (go to the end) but what I think we need is several more examples, and examples of suppressing gcc warnings too, especially worst-case examples where MSVC produces one warning and gcc produces another (and so #ifdef _MSC_VER ... #endif #ifdef GCC.. #endif is needed. If also needs input from other compiler users. If you have examples, either add to the wiki directly or email to me and I will add. We also need consensus that what I've suggested is actually the best advice. Collectively we already know how to do this, we just need to make it easier for library writers and maintainers to get on and do it. But I doubt if it is worth delaying release. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com

On Sat, Nov 7, 2009 at 11:18 AM, Mateusz Loskot <mateusz@loskot.net> wrote:

Beman Dawes wrote:

...

On Sat, Nov 7, 2009 at 7:13 AM, Daniel James <daniel_james@fmail.co.uk> wrote:

...

On 07/11/2009 10:38, Paul A. Bristow wrote:

...

...

Stephan Lavavej from Microsoft told me that that this option triggers known bugs in the compiler. He strongly recommends to stay away from it. Why would he do that for his own company's compiler? Yes, however, does really Stephan recommends it? That would be the first question.

What, you don't believe me when I tell you that STL personally told me that? :) With his permission: "While /Za is supported (both by the compiler and the standard library), we recommend against using it. Although it increases conformance in several areas (e.g. preventing unqualified name lookup from reaching into dependent base classes), it isn't subjected to exhaustive testing like the default setting is. There have been and will probably continue to be examples of /Za exposing compiler bugs that otherwise aren't exposed. For example, during VC10's development, /Za's elided copy constructor check was being triggered during move construction, when no copy constructor is being called (even theoretically). In particular, if you compile with multiple compilers, /Za isn't very useful - other compilers will detect nonconformant code that /Za would have detected." I would add that it my experience /Za doesn't make MSVC fully conformant anyway (I mean, in practice, not only in theory) and given Stephan's recommendation I no longer use it. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

Mateusz Loskot wrote:

Emil Dotchevski wrote:

...

On Sat, Nov 7, 2009 at 11:18 AM, Mateusz Loskot <mateusz@loskot.net> wrote:

...

Beman Dawes wrote:

...

On Sat, Nov 7, 2009 at 7:13 AM, Daniel James <daniel_james@fmail.co.uk> wrote:

...

On 07/11/2009 10:38, Paul A. Bristow wrote:

...

> Stephan Lavavej from Microsoft told me that that this option triggers > known bugs in the compiler. He strongly recommends to stay away from it. Why would he do that for his own company's compiler? Yes, however, does really Stephan recommends it? That would be the first question. What, you don't believe me when I tell you that STL personally told me that? :)

Emil,

It's not about that I believe or not. I simply would expect something posted to the public about that and officially by Visual C++ Team. The Visual C++ is not a toy maintained by one-man shop, so I'm not that eager to take every gossip as truth about this product.

Butting in... Since it seems this conversation is not doing much. But saying that a statement from STL is gossip is less than fair. As for Microsoft not publicly advertising that a non-default option which a very small subset of its users would seek out seems rather on track with what a corporation would do for a product it sells (not just MS, any corporation). Hence I'm surprised by your contention that it's not a valid argument by Emil. As it seems like a valid concern to me, as presented by Emil. So perhaps you should consider asking the VC team to publicly comment about it instead of arguing about the gossip'ness of Emil's concern. -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail

Mateusz Loskot wrote:

Rene Rivera wrote:

...

Hence I'm surprised by your contention that it's not a valid argument by Emil. As it seems like a valid concern to me, as presented by Emil. So perhaps you should consider asking the VC team to publicly comment about it instead of arguing about the gossip'ness of Emil's concern.

Rene,

I believe that, driven by caution, there is nothing wrong with asking for source of such information, for confirmation. Otherwise, it's nothing wrong to reserve rights to have second thoughts about something that have been said. Of course, it's nothing personal.

Hm, yes, of course... But my convern was with your contention that Emil's quote from STL is gossip. Or did I not understand your response to Emil? After all we are talking about Stephan T. Lavavej, Visual C++ Libraries Developer. Now-confusedly-yours :-\ -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail

Mateusz Loskot wrote:

Rene Rivera wrote:

...

Now-confusedly-yours :-\

I'm sorry for confusions.

I hope it clarifies my point.

It does. And thanks for putting up with my confusion :-) -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail

Emil Dotchevski wrote:

On Sat, Nov 7, 2009 at 11:18 AM, Mateusz Loskot <mateusz@loskot.net> wrote:

...

Beman Dawes wrote:

...

On Sat, Nov 7, 2009 at 7:13 AM, Daniel James <daniel_james@fmail.co.uk> wrote:

...

On 07/11/2009 10:38, Paul A. Bristow wrote:

...

...

Stephan Lavavej from Microsoft told me that that this option triggers known bugs in the compiler. He strongly recommends to stay away from it. Why would he do that for his own company's compiler? Yes, however, does really Stephan recommends it? That would be the first question.

What, you don't believe me when I tell you that STL personally told me that? :)

With his permission: "While /Za is supported (both by the compiler and the standard library), we recommend against using it. Although it increases conformance in several areas (e.g. preventing unqualified name lookup from reaching into dependent base classes), it isn't subjected to exhaustive testing like the default setting is. There have been and will probably continue to be examples of /Za exposing compiler bugs that otherwise aren't exposed. For example, during VC10's development, /Za's elided copy constructor check was being triggered during move construction, when no copy constructor is being called (even theoretically). In particular, if you compile with multiple compilers, /Za isn't very useful - other compilers will detect nonconformant code that /Za would have detected."

Driven by curiosity, I've exchanged e-mails with Stephan who provided some further interesting details. Here we go: <myself>Perhaps it would be useful to post a note on that to Visual C++ Team blog?</myself> <Stephan> That's a good idea. I've made a note to myself to write a blog post about /Za. (I have some more work to do for VC10, and then I have to blog about _ITERATOR_DEBUG_LEVEL and nullptr and rvalue references v2, so this won't appear in the immediate future.) By the way, the bogus elided copy constructor check that I mentioned was worse than we thought. Unfortunately, the underlying problem in the compiler isn't trivial to fix. We're going to try one more time to fix this for VC10, but if the fix is deemed too risky, then things like v.push_back(unique_ptr<int>(new int(1729))) simply won't compile under /Za. Another thing to consider is that /Za is incompatible with <windows.h>. You have my permission to quote any part of this mail. </Stephan> Best regards, -- Mateusz Loskot, http://mateusz.loskot.net Charter Member of OSGeo, http://osgeo.org

Emil Dotchevski wrote:

On Fri, Nov 6, 2009 at 8:33 AM, Beman Dawes <bdawes@acm.org> wrote:

...

On Thu, Nov 5, 2009 at 12:07 PM, Paul A. Bristow and disabling Microsoft language extensions Disable MS extensions = Yes, command line option /Za "

Stephan Lavavej from Microsoft told me that that this option triggers known bugs in the compiler. He strongly recommends to stay away from it.

Any public reference to this? A blog post or bug report in Microsoft Connect? What you're saying stays in contradiction with what Visual C++ Team is promising at http://blogs.msdn.com/vcblog/archive/2008/01/08/q-a-on-our-tr1-implementatio... "We're ensuring that TR1 compiles warning-free at /W4, in all supported scenarios. This includes switches like /Za, /Gz, and the like." Best regards, -- Mateusz Loskot, http://mateusz.loskot.net Charter Member of OSGeo, http://osgeo.org

on Thu Nov 05 2009, Beman Dawes <bdawes-AT-acm.org> wrote:

On Wed, Nov 4, 2009 at 4:12 PM, Patrick Horgan <phorgan1@gmail.com> wrote:

...

-pedantic issues all the warnings required by strict ISO C and ISO C++.  They are in the standard for a reason...

Tread carefully here. The standards require "diagnostics", not warnings. And the warnings from gcc -pedantic and other compiler's equivalent go far beyond anything mandated by the standards.

Maybe so, but -pedantic does detect some illegal code that isn't otherwise flagged and can't be guaranteed to be portable. -- Dave Abrahams Meet me at BoostCon: http://www.boostcon.com BoostPro Computing http://www.boostpro.com

on Wed Nov 04 2009, Vladimir Prus <vladimir-AT-codesourcery.com> 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.

Why should we remove -pedantic, specifically? -- Dave Abrahams Meet me at BoostCon: http://www.boostcon.com BoostPro Computing http://www.boostpro.com

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@sig.com 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.

David Abrahams wrote:

on Wed Nov 04 2009, "John Maddock" <john-AT-johnmaddock.co.uk> 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.

FWIW - I've always made an effort to minimize or eliminate warnings in the serialization library. By the most phantastic of coincidences, a user reported a track item that including headers provokes a large number of warnings with msvc when warning level 4 is used. (I use level 3). It turns out that the problem is the compiler cannot create a default constructor and assignment operator when a class has a const member. No matter that the classes are derived from boost::noncopyable and No matter that non of the tests copy or assign the classes in question. So to really fix this boost::noncopyable will need to be replaced with something else and that will have to be documented and maybe even mini-reviewed. So there is going to be a lot more here than meets the eye. Robert Ramey

Robert Ramey wrote:

David Abrahams wrote:

...

on Wed Nov 04 2009, "John Maddock" <john-AT-johnmaddock.co.uk> 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.

FWIW - I've always made an effort to minimize or eliminate warnings in the serialization library.

By the most phantastic of coincidences, a user reported a track item that including headers provokes a large number of warnings with msvc when warning level 4 is used. (I use level 3). It turns out that the problem is the compiler cannot create a default constructor and assignment operator when a class has a const member. No matter that the classes are derived from boost::noncopyable and No matter that non of the tests copy or assign the classes in question. So to really fix this boost::noncopyable will need to be replaced with something else and that will have to be documented and maybe even mini-reviewed. So there is going to be a lot more here than meets the eye.

This is exactly one of the warnings that Boost could document as a "silly warning", that developers are assumed to disable. Potential users would then know - up front - whether their policy allows them to use Boost or not. And current users can check that the next release has a compatible policy, before upgrading. Bo Persson

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@sig.com 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.

On Wed, Nov 4, 2009 at 10:03 AM, Mateusz Loskot <mateusz@loskot.net> 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

On Wed, Nov 4, 2009 at 12:56 PM, Zachary Turner <divisortheory@gmail.com>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.

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

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Simonson, Lucanus J Sent: Wednesday, November 04, 2009 2:22 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

...

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.

#pragma warnings are usually done correctly if they're pushed then popped... Example: #pragma warning( push ) #pragma warning( disable: 4244 4793 4996 ) class foobar { }; #pragma warning( pop )

AMDG Thomas Klimpel wrote:

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, it should not be disallowed.

The "warning C4127: conditional expression is constant" for a code like "if(op_type < 2)" where "op_type" is a template parameter is just nonsense.

My personal favorite example of conditional expression is constant is while(true) In Christ, Steven Watanabe

On Wed, Nov 4, 2009 at 1:06 PM, Patrick Horgan <phorgan1@gmail.com> 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.

  It not only doesn't look good.  It isn't good.  As Herb Sutter and Andrei   Alexandrescu point out in Item 1 in C++ Coding Standards which says compile   cleanly at high warning levels, if you get used to ignoring warnings, it is   guaranteed to bite you in the butt.  I know people will talk about silly   examples, and how hard it is. Wah wah wah.  I find real errors in the code   of people like that all the time.  They got used to ignoring warnings and   real problems got by them.  Someone who isn't willing to understand the   warnings is asking for trouble, and I know there are people in boost who   don't take the trouble to understand their warnings.  I see things that are   silly, and that they should have fixed.  I don't see how they can hold their   heads up for some of this stuff.

It isn't helpful to cast the blame generically like that. I don't know how you know that Boost developers don't take the trouble to understand their warnings, but a good start would be to file a specific bug report whenever you suspect that a warning is being erroneously ignored.

  Of course, there are silly warnings.  Sometimes the problem is with the   compiler.  But still, make them go away if at all possible, otherwise the   noise makes it difficult/impossible to catch the real problems, and all that   noise makes you look like a rookie wanna be, not the provider of serious   software.

Should I understand that your code compiles on MSVC without disabling C4996? It tends to generate a lot of noise even at low warning levels. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

On Wed, Nov 4, 2009 at 8:34 PM, Steven Watanabe <watanabesj@gmail.com>wrote:

It would certainly be a good thing if Boost compiled without warnings. However, I do not think that fixing all warnings would significantly reduce the number of bugs. Realistically, the vast majority of the warnings that we have to deal with are spurious. 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.

In Christ, Steven Watanabe

Well, they make the library physically unusable for some users, which is a pretty big deal. I don't know how prevalent this practice is in the industry but I know that I've worked at more than one company where you just aren't allowed to check in code that has warnings in it period. And it's not like you can just take a poll of how many people this affects, because if it does affect someone they certainly aren't here reading this mailing list. Zach

On Thu, Nov 5, 2009 at 10:45 AM, Sid Sacek <ssacek@securewatch24.com> 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?

It isn't always about syntax, and it isn't always desirable to work around the warning. Few examples: - C4996 on MSVC should be disabled. - non-virtual-dtor on GCC contradicts a valid design choice. - the GCC warning about using C-style cast is annoying when I'm casting 0 to a null pointer as in (T *)0. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

On Thu, Nov 5, 2009 at 1:50 PM, Kim Barrett <kab.conundrums@verizon.net> wrote:

On Nov 5, 2009, at 2:04 PM, Emil Dotchevski wrote:

...

- non-virtual-dtor on GCC contradicts a valid design choice.

At least in recent versions of gcc, this warning is not generated when there is a non-public dtor.

This doesn't help much. If you make a base destructor protected, it won't complain in the base class, but it will complain in the derived class, which typically doesn't even define a destructor explicitly. It isn't practical to silence that warning by limiting access to the derived destructor because that destructor is safe to call as long as the derived type isn't used as a base. And if it is designed to also be used as a base, it should have a protected (or public virtual) destructor.

A public non-virtual dtor for a base class seems like a request for slicing bugs rather than a valid design choice.

There is a chance of slicing bugs, but the non-virtual destructor means "no, you are not welcome to call the destructor through a base pointer or reference." That is a design choice and I don't know of another way to express it. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

On Thu, Nov 5, 2009 at 3:46 PM, Kim Barrett <kab.conundrums@verizon.net> wrote:

On Nov 5, 2009, at 5:32 PM, Emil Dotchevski wrote:

...

On Thu, Nov 5, 2009 at 1:50 PM, Kim Barrett <kab.conundrums@verizon.net> wrote:

...

...

- non-virtual-dtor on GCC contradicts a valid design choice. At least in recent versions of gcc, this warning is not generated when

On Nov 5, 2009, at 2:04 PM, Emil Dotchevski wrote: there is a non-public dtor. This doesn't help much. [...] but it will complain in the derived class, which typically doesn't even define a destructor explicitly. On the other hand, once one has paid the cost of making a class polymorphic anyway, is there really much (if any?) benefit to making its destructor non-virtual?

Q: Who would want to know that boost::exception_ptr's destructor is not virtual? A: Only someone who wants to build a polymorphic type hierarchy with boost::exception_ptr as a base, calling delete through a boost::exception_ptr base pointer. So, exception_ptr's destructor should be virtual to keep *that* guy out of trouble? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

On Thu, Nov 5, 2009 at 8:03 PM, Steven Watanabe <watanabesj@gmail.com> wrote:

AMDG

Emil Dotchevski wrote:

...

Q: Who would want to know that boost::exception_ptr's destructor is not virtual?

A: Only someone who wants to build a polymorphic type hierarchy with boost::exception_ptr as a base, calling delete through a boost::exception_ptr base pointer.

So, exception_ptr's destructor should be virtual to keep *that* guy out of trouble?

Emil, can you please silence the compiler somehow?

I had silenced it already, I believe. That's what triggered that particular rant. :) I see that your patch deals with the warnings better. For now, I will leave my commit from earlier today, but probably after the release is out I'll fix it your way. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

From my understanding of this pragma, it turns off all warnings from the

Phil Richards wrote: On Thu, 2009-11-05 at 20:03 -0800, Steven Watanabe wrote: Emil, can you please silence the compiler somehow? We seem to waste time arguing about it on a regular basis. I've attached a patch that uses #pragma GCC system_header header. Maybe I'm being incredibly dumb, but isn't this a rather shotgun approach to the problem? What if, by some chance, the header is edited at some later stage and an issue is introduced that would have generated (valid) warnings? Problem easily solved. In boost test builds using gcc use -Wsystem-headers to turn the warnings back on. Patrick

On Mon, 2009-11-09 at 09:36 -0500, Stewart, Robert wrote:

...

Phil Richards wrote: [with regards to #pragma GCC system_header being potentially too encompasing]

...

Problem easily solved. In boost test builds using gcc use -Wsystem-headers to turn the warnings back on. That's too broad in effect. It would enable warnings in headers

Patrick Horgan wrote: that one doesn't want to hear from including, I suspect, those provided by the platform.

Yep, it would.

The best approach likely is a library specific mechanism that, by default, honors a Boost-wide config to disable warnings using #pragma GCC system_header.

I think that any such mechanism would need to be wrapped with a BOOST_TESTED_AT(...) to make sure that real warnings highlighted by new versions of the compiler didn't sneak through. It isn't reasonable to assume that the library author will immediately test all new GCC releases with the pragma disabled. Phil -- Phil Richards, <news@derived-software.ltd.uk>

Emil Dotchevski wrote:

On Thu, Nov 5, 2009 at 3:46 PM, Kim Barrett <kab.conundrums@verizon.net> wrote:

...

On Nov 5, 2009, at 5:32 PM, Emil Dotchevski wrote:

...

On Thu, Nov 5, 2009 at 1:50 PM, Kim Barrett <kab.conundrums@verizon.net> wrote:

...

On Nov 5, 2009, at 2:04 PM, Emil Dotchevski wrote:

...

...

...

...

- non-virtual-dtor on GCC contradicts a valid design choice.

...

...

...

At least in recent versions of gcc, this warning is not generated when there is a non-public dtor.

...

...

This doesn't help much. [...] but it will complain in the derived class, which typically doesn't even define a destructor explicitly.

...

On the other hand, once one has paid the cost of making a class polymorphic anyway, is there really much (if any?) benefit to making its destructor non-virtual?

Exactly.

Q: Who would want to know that boost::exception_ptr's destructor is not virtual?

A: Only someone who wants to build a polymorphic type hierarchy with boost::exception_ptr as a base, calling delete through a boost::exception_ptr base pointer.

So, exception_ptr's destructor should be virtual to keep *that* guy out of trouble?

Making the dtor virtual does no harm and avoids questions and concerns from any of your customers. The decision seems easy. _____ Rob Stewart robert.stewart@sig.com 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.

The point is not that fixing warnings would not make Boost better. I'm saying that if the correctness of Boost were the only consideration, the effort spent dealing with warnings could probably be better spent in other ways (Such as dealing with the >850 open trac tickets). The main problem with warnings is not that they indicate bugs, so much as that they make the code less usable because of the noise that they generate for users. Au Contraire! Fixing warnings can quite likely make code better. Whenever I've been involved in a project that generated lots of warnings and the effort was made (often at my encouragement), to clean them up,

Steven Watanabe wrote: people found real bugs--things that embarrassed them. They had been told by the compiler about them, but because of the noise, missed the information. I certainly include myself in that. Those embarrassments are why I go with a policy of cleaning up all the warnings. I don't like being surprised by bugs I ignored, and it's especially bad if someone else finds it first. You quickly learn to code differently and don't generate more warnings. I notice that much of the discussion from people who don't want to fix warnings has focused on the silly ones. They seem to have been trained by compiler writers to ignore warnings. That's frustrating. Getting rid of your warnings is a good coding practice, and if all of them are the silly ones you're a better coder than I. If you have virtual functions and no virtual destructor, and you know why, for example, it can be frustrating to get whined at. But, if you provide the virtual destructor, you make it so that someone else maintaining your code years down the road won't be able to easily screw the pooch. Patrick

On Thu, Nov 5, 2009 at 1:45 PM, Sid Sacek <ssacek@securewatch24.com> 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

...

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

I read your post carefully and it seems like fixing the warnings didn't uncover any bugs and at least for one warning the fix introduced bugs; +0 for the former, -1 for the latter leaves me wondering what value do you find in fixing the warnings.

Honestly, I can't believe the amount of effort expended on *discussing* this rather than doing something about it. I can say that in my Boost contributions fixing warnings *has* found bugs and I believe improved code quality, further there are some things that don't get warned about (particularly by gcc) which would have made bug-hunting so much easier (for example an implicit narrowing conversion between floating point types). All of which is not to say there aren't situations where I find myself cursing the compiler for issuing stupid busy-body warnings: we all have our favorite list of those. But you know what? It's quicker to fix the warning than it is to spend the time griping about it! To take one example quoted here: the gcc "class does not have a virtual destructor" warning. I've been hit by that in template metaprogramming code, where there is absolutely zero chance that it could ever represent a bug, none the less users complained, I griped, and then in the end I fixed it. What I learned from this was: * It's quicker to fix than gripe. * These often get reported to me multiple times via multiple channels (list, bug report etc), so the quicker it's fixed the less time it takes me dealing with support requests. * The volume of template back-trace can be so high, that even if a complete C++ novice can recognize that the warning is meaningless, the volume of data obscures the *real* problems. * There's no way a user can tell whether this represents a true issue or not without delving into your code and figuring it out for themselves. That's a real concern with this "class does not have a virtual destructor" warning: you may know and understand the context, a user does not: indeed a new Boost user more familiar with OO design than Boost-paradigms may well say to themselves "gees these guys can't even make a destructor virtual when it should be", and just move on to some other library, or worse, reinvent the wheel by writing less good code themselves. Do you expect every new user to Boost to investigate every warning and check it's harmless for themselves? All 13 thousand lines of them from the last build? ;-) And now I must apologize, because this is starting to turn into a rant as well :-( So... here's what I'm going to do: please vote for your favorite worst offenders and I'll start submitting patches and/or making fixes. Any other volunteers? Cheers, John.

On Fri, Nov 6, 2009 at 12:29 PM, Paul A. Bristow <pbristow@hetp.u-net.com> wrote:

No new stuff should be accepted until it passes warning free, for a start.

I don't think that'd be fair, at least not for both gcc and visual c++. Some of the onus to fix a compiler's warnings should be on the people who care about that compiler.

On Fri, Nov 6, 2009 at 1:38 PM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

By "stuff," I assume Paul meant "libraries," as in "no new libraries accepted into Boost unless warning free."  That's fair, if it is established and known before a review begins.

Currently, we don't even require that a library builds on any specific compilers, let alone warning free. What you're suggesting adds a considerable burden on a developer - which is particularly unfair if the library is eventually rejected. Implementation issues can be fixed after the review and, in this case, I would hope it would be with the help of the boost community. http://www.boost.org/development/requirements.html#Portability

On Fri, Nov 6, 2009 at 2:13 PM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

Daniel James wrote:

...

Currently, we don't even require that a library builds on any specific compilers, let alone warning free. What you're suggesting adds a considerable burden on a developer - which is particularly unfair if the library is eventually rejected. Implementation issues can be fixed after the review and, in this case, I would hope it would be with the help of the boost community.

It isn't unfair if the submitter understands a policy a priori.

How does understanding an unfair burden in advance make it fair?

 Furthermore, proving Boost quality and readiness, for a review, means meeting Boost coding policies, whatever they might be. If a potential submitter finds following a policy too onerous before a review, what might be found too burdensome after acceptance?

Supporting setups other than your own is much easier after acceptance as you have access to the testing system and the support of the community.

 Whether such a warnings policy is ever made official is completely separate; whatever policies are established, it is fair to expect submitters to follow them

Not really, if those policies have no bearing on the quality of the library then they don't have to be a review requirement. They can be met between review and release. Requiring warning free code at the review is just box ticking. Daniel

On Mon, Nov 9, 2009 at 9:27 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

When a potential library submitter evaluates Boost policies, those policies may be considered too onerous or not.  If the former, Boost might be denied a useful library, but the denial wasn't unfair.  All submitters face the same policies and choose whether to accept them.  Whether a policy is well or ill conceived can be determined more objectively than can its fairness.

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.  I assume that such a two-stage review process would be fairer in your mind?

I don't think another review is necessary. What if it was the same review process, but to be allowed to merge to a release branch you have to meet the new stricter guidelines? That seems like a good compromise. --Michael Fawcett

Emil Dotchevski wrote:

On Mon, Nov 9, 2009 at 6:27 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

...

Determining whether a policy is unfair is subjective. If one considers, in this case, that zero warnings at some Boost-established warnings setting is important to demonstrating code quality and to make the job of reviewers as easy as possible, then it is a fair policy.

Alternatively, the reviewers could compile at lower warnings level. Most likely they wouldn't even need to do that, because they will simply build using the author scripts, which will likely produce a build without warnings.

Either is certainly possible, but if there are established warning levels, it is reasonable to expect that level or stricter. A reviewer should be able to examine the library as any Boost user would. Why would following a warnings policy be different unless a specific exception were incorporated into the policy?

...

If reviewers are expected to judge code quality and it produces myriad warnings with established warnings settings, what might reviewers conclude?

The only reasonable conclusion is that the author prefers a lower warning level, or uses a different compiler. As far as I am concerned, a library should be evaluated only based on its documented interface.

A reasonable conclusion is that the submitter chose to ignore established Boost norms and doesn't care about the effect on reviewers, your assertion to the contrary notwithstanding. If trying to test against the library produces myriad warnings at established Boost warning levels, as a Boost user should expect, the reviewer shouldn't judge the usability of the library highly. _____ Rob Stewart robert.stewart@sig.com 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.

Emil Dotchevski wrote: ... elision by patrick... This assumes that a policy that requires warnings to be "fixed" is desirable or that it will lead to a better Boost (note that I'm not arguing against disabling warnings in Boost headers.) It's a safe assumption. Having been through this pain a number of times I know that it leads to consistently better code. I've seen it many times. In fact yesterday, coincidently, (or not, it was because I was talking to him about this discussion), I was working through the code of someone I highly respect as a programmer with decades of experience who's sat on a couple of different language standard committees, and while getting rid of warnings in his code, I found some real bugs. Had he been used to running at a high warning level, those bugs wouldn't have crept into his code. This didn't surprise me a bit. It always happens. You start building with higher warning levels on any large body of code and you always find real bugs. That's the point of it. A couple of them were really obvious, once the messages pointed me to them, but of course you know how our eyes see what we expect. A lot of the warnings were annoying and made me do silly things, like reverse a condition so that I could move a throw up and a return down. Some of them were technically bugs, like comparisons between signed and unsigned but in the problem domain we knew that the bug would never be hit, but still bad code, and sometimes you get data outside the problem domain--surprise! Eventually I got it down to multiple copies of a warning out of boost/archive/binary_oarchive.hpp signed vs unsigned conversion just from including it. Frustrating. In the next day or two I'll get back into it and figure that out (it's a line in basic_binary_oarchive::save_override). You might think that a waste of time, but it's not. The point is that now the code can be built normally with a higher warning level, and now that I've gotten rid of most of the noise, except for boost noise, more bugs will be caught before they get out in the wild. It's obvious that you don't like it. I know. It's a pain in the ass. But--there are real and large benefits that will help you and those that come after with this code for as long as people are building it and building with it. The potential payoff is huge! Just silencing all the warnings by disabling them means that you don't get the advantage that building with higher warning levels brings you. Certainly, if that's all you were going to do, it would pointless. If you're sure a complaint is a bug for the compiler that generates the warning, write a bug against them, but silence it for your users. Make sure if you do that that you revisit silencing it after the compiler writers fix their bug. Patrick

2009/11/9 Stewart, Robert <Robert.Stewart@sig.com>:

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? Warnings are useful tool, but they aren't necessary or sufficient for quality. 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).

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.  I assume that such a two-stage review process would be fairer in your mind?

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.

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 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.

 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. Daniel

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Daniel James Sent: Friday, November 06, 2009 2:00 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

On Fri, Nov 6, 2009 at 1:38 PM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

...

By "stuff," I assume Paul meant "libraries," as in "no new libraries

accepted into Boost unless warning free."  That's fair, if it is established and known before a review begins.

Currently, we don't even require that a library builds on any specific compilers, let alone warning free. What you're suggesting adds a considerable burden on a developer - which is particularly unfair if the library is eventually rejected. Implementation issues can be fixed after the review and, in this case, I would hope it would be with the help of the boost community.

I wasn't suggesting that all this needs to be done pre-review - but before 'stuff hits the streets' it should be done. But as John says, fixing while you are going along doesn't really take much effort. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com

On Sat, Nov 7, 2009 at 7:03 AM, Daniel James <daniel_james@fmail.co.uk> wrote:

On 06/11/2009 16:34, Paul A. Bristow wrote:

...

I wasn't suggesting that all this needs to be done pre-review - but before 'stuff hits the streets' it should be done.

Sorry about that.

...

But as John says, fixing while you are going along doesn't really take much effort.

It does for compilers that aren't available for your operating system.

Agreed. We really need test-on-demand for to make that easy. --Beman

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Stewart, Robert Sent: Friday, November 06, 2009 1:38 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

Paul A. Bristow wrote:

...

(Warning about while(true) isn't - but it's easily silenced - especially with some saved snippets to paste).

I've found that "for (;;)" silences that warning, so it ought to be listed in the Wiki.

Ugh! Paul

Stewart, Robert wrote:

Paul A. Bristow wrote:

...

Stewart, Robert wrote:

...

Paul A. Bristow wrote:

...

(Warning about while(true) isn't - but it's easily silenced - especially with some saved snippets to paste).

I've found that "for (;;)" silences that warning, so it ought to be listed in the Wiki.

Ugh!

Some actually like it. I use it because it works and avoids that warning.

So what about filing a bug report with the compiler team, instead of for the library? :-) Bo Persson

John Maddock wrote:

To take one example quoted here: the gcc "class does not have a virtual destructor" warning. I've been hit by that in template metaprogramming code, where there is absolutely zero chance that it could ever represent a bug, nonetheless users complained, I griped, and then in the end I fixed it. What I learned from this was:

* It's quicker to fix than gripe. * These often get reported to me multiple times via multiple channels (list, bug report etc), so the quicker it's fixed the less time it takes me dealing with support requests. * The volume of template back-trace can be so high, that even if a complete C++ novice can recognize that the warning is meaningless, the volume of data obscures the *real* problems. * There's no way a user can tell whether this represents a true issue or not without delving into your code and figuring it out for themselves.

That's a real concern with this "class does not have a virtual destructor" warning: you may know and understand the context, a user does not: indeed a new Boost user more familiar with OO design than Boost-paradigms may well say to themselves "gees these guys can't even make a destructor virtual when it should be", and just move on to some other library, or worse, reinvent the wheel by writing less good code themselves. Do you expect every new user to Boost to investigate every warning and check it's harmless for themselves? All 13 thousand lines of them from the last build? ;-)

That is excellent fodder for the rationale for eliminating warnings section in the policy being developed in the Wiki. _____ Rob Stewart robert.stewart@sig.com 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.

On Fri, Nov 6, 2009 at 2:50 AM, Emil Dotchevski <emildotchevski@gmail.com> wrote:

On Thu, Nov 5, 2009 at 9:53 PM, Gottlob Frege <gottlobfrege@gmail.com> wrote:

...

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

I read your post carefully and it seems like fixing the warnings didn't uncover any bugs and at least for one warning the fix introduced bugs; +0 for the former, -1 for the latter leaves me wondering what value do you find in fixing the warnings.

Hard to find, but it was there:

...

Once you get to 0, warnings will find more bugs than they cause, but not during the drive to 0

I think everyone agrees 0 warnings are worthwhile (mostly to increase visibility of new warnings, some of which will flag real bugs). It is just a question of how worthwhile, and what, if anything, should be done when you are in the way too many warnings situation. Tony

Emil Dotchevski wrote:

On Fri, Nov 6, 2009 at 1:33 PM, Gottlob Frege <gottlobfrege@gmail.com> wrote:

...

Hard to find, but it was there:

...

...

Once you get to 0, warnings will find more bugs than they cause, but not during the drive to 0

Ah.

...

I think everyone agrees 0 warnings are worthwhile (mostly to increase visibility of new warnings, some of which will flag real bugs).

We are all at close to 0 warnings, just accept that. The difference is only in what level of warnings a particular developer is most comfortable with. We are all reasonable people and I think that it is safe to assume that none of us are sloppy in that selection.

Note also #pragma GCC system_header used in the GCC standard library. Easy solution to the same problem. This also indicates that they didn't think that "fixing" all warnings in STL was a better idea, but I guess we're better than those hackers.

Well, when you implement a standard library for a particular compiler, you are allowed (and in some places even required) to use code that isn't portable. Writing a highly portable library is a totally different thing. Bo Persson

On Fri, Nov 6, 2009 at 8:52 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

Gottlob Frege wrote:

...

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.

That's an excellent rationale.  You may handle it well in the current code, but a user may be concerned, may use your code in an unexpected way, or a maintainer may violate your design idea.

...

   - solution:  add virtual dtor, make it protected, leave a comment about the warning

That's a bit vague.  The policy should be public and virtual or protected and non-virtual.  If the latter leads to a warning from a compiler, we should document the manner by which to quiet that compiler.  We could even create a pair of macros to bracket the protected, non-virtual dtor or use a macro to generate the declaration with the warning suppression.

That's the thing - in all of our cases there were no bugs - ie we were not deleting derived classes via the base class. So we didn't _need_ virtual. But gcc complains. Even if we made it protected, hoping to make our intent clear (to other developers and to the compiler) we still needed to make it virtual to get rid of the warning. I hear that may have since changed. Maybe because of us, actually, because I do recall someone was going to bring up the issue with gcc developers.

Keeping the initializer list in good order means you can't be confused about the order of initialization.  I *have* found bugs in others' code because of this.  Many developers are ignorant of the order of initialization and can be bitten by it.  If such developers are trained to honor warnings, then this warning will help them.  At the very least, they will ask me about it.

It bothers me that the compiler is pandering to developers who don't know their language well enough. But maybe that's what all warnings are about, to some extent.

The fix for this warning is so trivial that there shouldn't be an issue with it.  Fix the initializer list order and be done with it.

It would be trivial unless you had a large code-base and hundreds of occurrences that needed fixing. Then it just seems a waste of time. But it's the same thing - once you are at 0 it is easier and worthwhile. It is just getting there that's the problem. Tony

On Thu, Nov 5, 2009 at 4:36 AM, John Maddock <john@johnmaddock.co.uk> 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.

 It not only doesn't look good.  It isn't good.

No disagreement from me.

Some stats might help:

Boost-1.41 pre-beta on ubutunu-9.1 with gcc-4.4.1 produces an 11 Mb log file from the build, with 133 THOUSAND LINES of output.

And that's just from building the binaries - so a tiny subset of Boost - I dread to think what a full test build would reveal.

In fact the more I think about this, the more I feel that we should fix as much of this as we can for 1.41.

I agree. One possible approach would be to create a wiki page that describes: * A first cut at a Boost policy on warnings. * For the current release of the critical compilers, what compiler switches apply. "Critical compilers" are GCC, VC++, for now. * How a developer using bjam can run local tests with those switches. * A list of libraries known to have serious warning issues. Anything else will take too much time to be useful for 1.41.0. Would anyone like to take a cut at starting such a wiki page? Volunteer(s) needed. --Beman

Paul A. Bristow wrote:

Beman Dawes wrote:

...

One possible approach would be to create a wiki page that describes:

* A first cut at a Boost policy on warnings.

* For the current release of the critical compilers, what compiler switches apply. "Critical compilers" are GCC, VC++, for now.

* How a developer using bjam can run local tests with those switches.

Since I've been a moaned about warning, I've had a stab at this at

https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines

Thanks Paul. I don't have access -- I may never have signed up, but I don't have my passwords with me to check -- so I'll comment here. My rationale for each suggested change is in parentheses. Heading: "Managing Warnings" (This heading fits the section better and describes the purpose a bit better.) s/avoid warnings/eliminate or suppress warnings/ (We want to avoid them, but the purpose of this section is to eliminate them or, if not possible, suppress them, isn't it?) s/Some reasons are:/Reasons to eliminate or suppress warnings:/ 1. "To allow users, whose environment requires no warnings, to use Boost code." (Added commas for dependent clause and clarified "these conditions" as "no warnings.") 3. s/focussing/focusing/, s/writers/writer's/ (The former may be a British/American English difference.) 4. "To improve portability by focusing developer attention on potentially non-portable code." (Don't want library user's attention drawn to the problems.) 6. "To permit users to set high warning levels when using Boost libraries without overwhelming them with a barrage of library warnings." (Clarity.) 7. (This bullet isn't a reason to eliminate/avoid warnings, but with the addition of "or suppress" above, it fits.) s/What to do/Actions:/ 1. (Rather than prose, use bullets or a table to list the settings for each compiler: Compiler | Settings | Comments ------------------------------------------------------------ MSVC | /W4 /Za | disabling language extensions GCC | -Wall -pedantic | Using bjam, set warnings=all. Notice the spelling correction for "pedantic.") 2. "For each supported compiler, apply these steps for each warning found:" (Highlight the need to check each supported compiler and highlight the need to iterate over all warnings.) 2. a. "Rewrite the code to avoid the warning, if possible. For example, adding a static_cast will indicate that any warning about loss of accuracy has been judged not possible or significant. Remove or comment out parameters to avoid warnings about unused parameters." (Clarity. Focus on warning elimination rather than value of parameter names.) 2. b. "If the warning cannot be eliminated, use whatever compiler-specific mechanisms are available to suppress warnings constrained to the smallest scope possible so as to avoid suppressing warnings in user code." (Clarity. Note s/depricated/deprecated/ in first code block comment.) "For MSVC, this involves pushing...to restore the original warning state:" (s/involved/involves/ and explains what's restored.) "If the warning is only for a specific compiler version, use this approach:" (Reads better with the latter clause.) 2. c. (Delete based upon rephrasing of 2.) 2. d. "If a warning cannot be eliminated or suppressed, explain why in the code and the documentation. If appropriate, document in build files, as well. Consider indicating the highest warning level possible or compiler-specific settings that will provide a warning-free build." (Clarity.) Specific remedies should be added for the various warnings encountered, too. Between 1. and 2., in the latter list, there should be a new bullet suggesting to use warnings-as-errors. _____ Rob Stewart robert.stewart@sig.com 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.

On Wed, Nov 4, 2009 at 4:06 PM, Patrick Horgan <phorgan1@gmail.com> wrote:

...   An example of one of my least favorite avoidable warnings is when one part   of boost moves an include file and deprecates the old location, and release   after release, I have to look at warnings from other parts of boost.  I   can't even say what I think about that.  I don't think a release should go   out with issues like that.  Boost should at least be internally consistent,   and not have warnings about misusing other parts of boost.  You'd think that   would be the bare minimum to avoid looking like amateurs.  How hard would it   have been to fix for example, in 1.40 when I see hundreds of warnings from   the graph stuff because:   boost/property_map.hpp:15:4: warning: #warning "This header is deprecated.   Please use: boost/property_map/property_map.hpp"   I'm guessing the graph guys are used to ignoring warnings.  But the code was   released like this.  That's embarrassing.  There's probably 35 places they   would have had to fix this so that hundreds of errors wouldn't spew out   whenever someone used their code.  They aren't even willing to grab the   low-hanging fruit.   There have long been other places where boost code used deprecated headers   from the standard libraries.  That's unprofessional.   Sorry to rant.  This is a long standing issue for me and I thought I better   write this before I saw the inevitable responses from people trying to   justify their bad coding.  I really would have ranted then!

The result of this long discussion may well be a Boost policy on warnings. But it the meantime, you might want to open tickets against against libraries that are using long deprecated headers. --Beman

Emil Dotchevski wrote:

but I'll point out yet another reason why this warnings discussion is silly:

Q: How do we know that Boost Exception works when RTTI is disabled?

A: I'm personally testing on whatever GCC I have installed and on MSVC 8/9, and I don't see anyone complaining about other platforms.

We don't test optimized builds, we don't test with exceptions or RTTI disabled, but apparently that "class foo has virtual functions but non-virtual destructor" is more important.

There is clearly a problem with the warnings generated by Boost code. Addressing that problem makes Boost more usable and might even make it usable where it now is not. That's hardly silly. By all means, suggest a policy WRT RTTI, but there's no need to call this discussion silly. _____ Rob Stewart robert.stewart@sig.com 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.

Emil Dotchevski wrote:

What about companies that *do* use Boost? If we are to prioritize our efforts to provide the best service to them, should we be fixing warnings or improving our testing procedures? Should we be fixing warnings in general or working on tickets?

Hmmm - not sure there's a huge difference. I get tickets on a regular base to fix warning. Some them are even marked "bug" !!! So if one strives to have a "zero ticket" policy (or as close as possible), you're going to end up with fixnig warnings. BTW - There are warnings in building a library - these are different than those generated in headers that users see. Robert Ramey

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 wrote:

What about companies that *do* use Boost? If we are to prioritize our efforts to provide the best service to them, should we be fixing warnings or improving our testing procedures? Should we be fixing warnings in general or working on tickets?

I'll help. Fixin' the warnings will help them too. Devote all your time to any show-stopper bugs, then when you've shot them, spend a day or two to get rid of all of your warnings, then assuming you don't have new development to do, if your testing procedures don't do a good job of testing, then put 80% of your time into fixing that. It's just for awhile and you'll be happy you did. It will probably generate a lot of new bugs for you, if not it's not doing it's job. Spend the other 20% on fixing bugs starting with the high-priority ones. Do all this in crunch mode for a week or two. It's a lot of fun when you're done and looking back at it later. When that's done write more code cause you'll get bored. Of course if you weren't really asking the question and were just yanking the guys chain--well it's still good advice;) <grin;> Patrick

Emil Dotchevski wrote:

On Thu, Nov 5, 2009 at 10:26 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:

...

There is clearly a problem with the warnings generated by Boost code. Addressing that problem makes Boost more usable and might even make it usable where it now is not. That's hardly silly.

At least to me, it isn't at all clear that there is a problem with warnings generated by Boost.

My builds are complicated by this issue. I strive for warning free builds for the many reasons enumerated in this discussion. The warnings are a problem for me and those that use my code.

Sure, some companies opt not to use Boost, but this doesn't necessarily indicate a problem in Boost. I've worked at several companies where STL was banned yet they spent countless hours implementing STL features and then countless hours implementing optimizations in these features that were readily implemented in STL. However, I don't blame their poor judgment on STL.

There are other companies that have found that zero-warning builds are wise and can't tolerate the noise introduced by using Boost. That's their decision, of course, but since Boost *can* do better, shouldn't it? Shouldn't Boost set a good example?

What about companies that *do* use Boost? If we are to prioritize our efforts to provide the best service to them, should we be fixing warnings or improving our testing procedures? Should we be fixing warnings in general or working on tickets?

That's a false dichotomy. We use Boost and we would be well served by eliminating the warnings that causes. Improved testing means that we don't have to discover and report bugs because tests would have found them earlier, so that's certainly helpful. Addressing bug tickets is necessary to be responsive to your customers. All of these things are valuable. You must prioritize them, of course. Note, however, that when working on a particular part of a library to address a new test failure or fix a reported bug, you can eliminate some warnings, too. Eventually, you'll address all known warnings or will have no bugs to fix and no test failures to address and can eliminate the remaining warnings instead. I must acknowledge that dealing with warnings takes time; I don't mean to trivialize that. Nevertheless, a *goal* of zero warnings on the primary compilers, with standardized warnings flags, is worthwhile for numerous reasons. _____ Rob Stewart robert.stewart@sig.com 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.

Emil Dotchevski <emildotchevski <at> gmail.com> writes:

We don't test optimized builds

It seems that some of the MSVC tests (e.g. Huang-Vista-x86_32) are actually run in release mode, though you currently can't tell that without looking at the test output. It would be useful to mark them in some way, even if it's only an equivalent of the '_0x' added to the gcc tests that are run in c++0x mode.

Stewart, Robert wrote: Emil Dotchevski wrote: Would it be a good idea to document a macro BOOST_NO_GCC_SYSTEM_HEADER, to enable developers to use something like: #ifndef BOOST_NO_GCC_SYSTEM_HEADER #pragma GCC system_header #endif The motivation being that a developer could still see warnings by #defining BOOST_NO_GCC_SYSTEM_HEADER while (by default) hiding warnings for users of the library? That seems useful. But redundant. It's already built into gcc to do this. If you just have the pragma and then build with -Wsystem-headers it turns the warnings back on. No sense in reinventing the wheel. Cheers, Patrick

Jeffrey Bosboom wrote:

Patrick Horgan wrote:

...

But redundant. It's already built into gcc to do this. If you just have the pragma and then build with -Wsystem-headers it turns the warnings back on. No sense in reinventing the wheel. Cheers, Patrick

But wouldn't that also cause the standard library and other actual system headers to generate warnings? It would presumably be difficult to pick out the Boost warnings from the standard library ones, just as Boost users currently complain about picking out warnings from their code amidst Boost warnings. That's a great point. Really, I would not use that pragma at all. I would make the header not generate errors, or silence individual ones.

Patrick

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe

...

I don't know of any way to locally silence individual warnings on gcc. For Boost, making the suppression local is more important than only suppressing a specific warning.

In Christ, Steven Watanabe

How about the use of precompiled headers? -Sid Sacek

Hi Steven, On Tuesday 10 November 2009, you wrote:

I don't know of any way to locally silence individual warnings on gcc.

With modern gcc (read: 4.3 and higher), you can use the following: -----------snip--------- #if defined(__GNUC__) #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #endif void foo(int bar { } +#if defined(__GNUC__) + #pragma GCC diagnostic warning "-Wdeprecated-declarations" +#endif -------------------------------------------------------- do disable warnings locally. If you want find out which options controls this warning, use <toolset>gcc:<cxxflags>-fdiagnostics-show-option for current gcc.

For Boost, making the suppression local is more important than only suppressing a specific warning.

This should work. Maybe we should add "-fdiagnostics-show-option" to gcc.jam for gcc greater 4.3... Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

On Wednesday 11 November 2009, you wrote:

...

#pragma GCC diagnostic ignored "-Wdeprecated-declarations"

That doesn't quite work, how do you know that the user wants -Wdeprecated-declarations to be set to "warning" after the include?

It seems I was too fast again. I've tried this only in .cpp files for one (annyoing) 3rdparty include and it worked. I just wanted to point out that gcc has some #pragma warning control, albeit not so sophisticated as msvc.

Could be the user has disabled that one on the command line, in which case if we turn it back on, that's as annoying for the end user as warnings from Boost!

Full agreement here

There was a time when some MSVC std lib headers behaved like that, and believe me it was *seriously* annoying :-(

I think I had those issues, too. yes, no fun. I really prefer "real" fixes. It took a lot of patience to introduce at least a "low warning" policy at work, but it was worth it. Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: Tuesday, November 10, 2009 9:20 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

For Boost, making the suppression local is more important than only suppressing a specific warning.

An important objective! I've added this to https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com

C4512 fix looks good to me, but I also note some warnings C4244: 'argument' : conversion from 'const int' to 'const short', possible loss of data

These could be quieted with carefully checked static_cast s.

This checks and documents that the narrowing IS expected.

As long as it really is expected :-) I haven't looked though, so I don't know anything about this case.

In the jamfile you can suppress the C4535 calling _set_se_translator() requires /EHa with

project : requirements ...

adding

<toolset>msvc:<asynch-exceptions>on # calling _set_se_translator() requires /EHa

It would be better if Boost.Test did this?

Nod. Not sure if it can in this case, since Variant doesn't actually link to the Boost.Test binaries.

boost\boostsvn\boost\test\impl\execution_monitor.ipp(1161) : warning C4701: potentially uninitialized local variable 'old_crt_hook' used

looks dodgy in Boost.Test too? Trak tickets?

I've already filed a patch against Boost.Test for this and other issues: https://svn.boost.org/trac/boost/ticket/3598 John.

-----Original Message-----

From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Richard Webb Sent: Wednesday, November 11, 2009 4:40 PM To: boost@lists.boost.org Subject: Re: [boost] Official warnings policy?

Looks like the warnings occur depending on the types used in the unit tests. For example, the warning mentioned above comes from the test code:

typedef variant< short, const char* > t_var2; typedef variant< unsigned short, const char*, t_var2 > t_var5; typedef variant< unsigned short, const char*, t_var5 > t_var6;

t_var6 v6; v6 = 58;

Which looks like a legitimate warning about converting an integer (58) to a short.

Since this is expected, could the test use a static_cast to the right type? v6 = static_cast< t_var6 >(58); Does this quiet the warning? Or must it be v6 = static_cast< unsigned short >(58); ? Either way this would document that casting/converting takes place? And perhaps users should do this too? The static_cast documents that the author has thought about this. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com

Steven Watanabe-4 wrote:

I'd be careful about adding a static_cast, since it changes exactly what's being tested. Part of the behavior of a class is the way it handles implicit conversions. Even conversions that generate warnings sometimes need to be tested.

There are a few of these type of warnings, at least some of which are 'intentional', as you say. For example, the variant test test2.cpp does: /// variant<char, const char*> v1; const int n0 = 88; v1 = n0; /// With the comment "Implicit conversion to bounded type", so you don't want to be explicitly casting the int to a char to avoid the warning. -- View this message in context: http://old.nabble.com/Official-warnings-policy--tp26200658p26307981.html Sent from the Boost - Dev mailing list archive at Nabble.com.

...

I'd be careful about adding a static_cast, since it changes exactly what's being tested. Part of the behavior of a class is the way it handles implicit conversions. Even conversions that generate warnings sometimes need to be tested.

You are right, as ever. But in this case, is it my ignorance of C++ or is there no way of specifying an (unsigned) short,

There is no portable method of specifying an unsigned short literal, except possible via the C99 macro UINT16_C in stdint.h.

The test wanted to use v6 = 58US;

Or unsigned short int i = 58U; v6 = i; ?

Or would just v6 = 58U avoid the warning?

In the test, I suspect that v6 = static_cast< t_var6 >(58); was what was really wanted (or some way of getting at the first template type from the variant that is beyond my skills).

Delving into this a bit deeper, I believe the warning is correct, and cannot / should not be fixed, to recap we have: d:\data\boost\trunk\boost/variant/variant.hpp(1297) : warning C4244: 'argument' : conversion from 'const int' to 'const short', possible loss of data d:\data\boost\trunk\boost/variant/variant.hpp(1366) : see reference to function template instantiation 'void boost::variant<T0_,T1>::convert_construct<const T>(T &,int,boost::mpl::false_)' being compiled with [ T0_=short, T1=const char *, T=int ] d:\data\boost\trunk\boost/variant/variant.hpp(1609) : see reference to function template instantiation 'boost::variant<T0_,T1>::variant<int>(const T &)' being compiled with [ T0_=short, T1=const char *, T=int ] d:\data\boost\trunk\boost/variant/variant.hpp(1619) : see reference to function template instantiation 'void boost::variant<T0_,T1>::assign<T>(const T &)' being compiled with [ T0_=short, T1=const char *, T=int ] test1.cpp(66) : see reference to function template instantiation 'boost::variant<T0_,T1> &boost::variant<T0_,T1>::operator =<int>(const T &)' being compiled with [ T0_=short, T1=const char *, T=int ] The code calls Variant's converting-assignment operator, and Boost.Variant internally decides what type to convert the argument to and makes the assignment. In this case the user *should* see a warning to be alerted to the fact that the internal conversion results in loss of precision (likewise they would see an error in this location if there is no suitable conversion). I notice that the code in question has a comment to this effect: // NOTE TO USER : // Compile error here indicates one of the source variant's types // cannot be unambiguously converted to the destination variant's // types (or that no conversion exists). But perhaps we should add a similar note about compiler warnings? John.