AMDG I ran the attached program on the Subversion trunk. output attached. There are quite a few headers that suppress warnings but fail to restore them. Some of these are harmless and a a few are paired disable/enable headers. In Christ, Steven Watanabe ./boost/archive/detail/abi_prefix.hpp ./boost/archive/detail/abi_suffix.hpp ./boost/archive/impl/basic_xml_grammar.hpp ./boost/asio/detail/pop_options.hpp ./boost/asio/detail/push_options.hpp ./boost/config/compiler/visualc.hpp ./boost/fusion/sequence/container/map/detail/map_lookup.hpp ./boost/fusion/sequence/container/set/detail/set_lookup.hpp ./boost/fusion/support/detail/compiler_config.hpp ./boost/interprocess/allocators/detail/adaptive_node_pool.hpp ./boost/interprocess/allocators/detail/node_pool.hpp ./boost/interprocess/allocators/detail/node_tools.hpp ./boost/interprocess/detail/config_begin.hpp ./boost/interprocess/detail/config_end.hpp ./boost/interprocess/smart_ptr/segment_deleter.hpp ./boost/intrusive/detail/config_begin.hpp ./boost/intrusive/detail/config_end.hpp ./boost/iostreams/detail/config/disable_warnings.hpp ./boost/iostreams/detail/config/enable_warnings.hpp ./boost/operators.hpp ./boost/ptr_container/detail/static_move_ptr.hpp ./boost/python/detail/config.hpp ./boost/python/detail/wrap_python.hpp ./boost/regex/pattern_except.hpp ./boost/regex/v4/regex_workaround.hpp ./boost/serialization/collections_load_imp.hpp ./boost/serialization/detail/stack_constructor.hpp ./boost/serialization/ephemeral.hpp ./boost/serialization/hash_collections_load_imp.hpp ./boost/serialization/nvp.hpp ./boost/serialization/serialization.hpp ./boost/serialization/variant.hpp ./boost/serialization/void_cast.hpp ./boost/spirit/core/primitives/impl/primitives.ipp ./boost/spirit/core/primitives/primitives.hpp ./boost/statechart/detail/rtti_policy.hpp ./boost/statechart/detail/state_base.hpp ./boost/statechart/state_machine.hpp ./boost/static_warning.hpp ./boost/test/detail/enable_warnings.hpp ./boost/test/detail/suppress_warnings.hpp ./boost/test/utils/iterator/ifstream_line_iterator.hpp ./boost/test/utils/nullstream.hpp ./boost/test/utils/runtime/config.hpp ./boost/tuple/detail/tuple_basic_no_partial_spec.hpp ./libs/serialization/src/basic_xml_grammar.ipp ./libs/statechart/example/Camera/Camera.hpp ./libs/statechart/example/Camera/Configuring.hpp ./libs/statechart/example/Camera/Shooting.hpp ./libs/type_traits/test/test.hpp ./libs/wave/samples/cpp_tokens/cpp_tokens_config.hpp ./libs/wave/samples/list_includes/list_includes_config.hpp ./libs/wave/samples/token_statistics/token_statistics.hpp ./libs/wave/samples/waveidl/idl_config.hpp ./tools/quickbook/detail/actions.hpp ./tools/wave/cpp_config.hpp // check_pragmas.cpp // // Copyright (c) 2007 // Steven Watanabe // // Distributed under the Boost Software License, Version 1.0. (See // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) #include <string> #include <iterator> #include <iostream> #include <boost/filesystem.hpp> #include <boost/filesystem/fstream.hpp> #include <boost/regex.hpp> namespace fs = boost::filesystem; boost::regex pragmas("#\\s*pragma\\s+warning\\s*\\(\\s*((push)|(pop)|(disable))"); int main() { fs::recursive_directory_iterator iter("."); fs::recursive_directory_iterator end; for(; iter != end; ++iter) { if(!fs::is_directory(iter->path())) { const std::string extension(fs::extension(iter->path())); if(extension == ".hpp" || extension == ".ipp") { fs::ifstream file(iter->path()); std::string file_contents(std::istreambuf_iterator<char>(file.rdbuf()), std::istreambuf_iterator<char>()); int push_count = 0; boost::regex_iterator<std::string::iterator> regex_iter(file_contents.begin(), file_contents.end(), pragmas); boost::regex_iterator<std::string::iterator> regex_end; for(; regex_iter != regex_end; ++regex_iter) { if((*regex_iter)[2].matched) { ++push_count; } else if((*regex_iter)[3].matched) { --push_count; if(push_count < 0) { std::cout << *iter << std::endl; goto done_with_file; } } else if((*regex_iter)[4].matched) { if(push_count <= 0) { std::cout << *iter << std::endl; goto done_with_file; } } } if(push_count != 0) { std::cout << *iter << std::endl; } done_with_file:; } } } }
Steven Watanabe wrote:
AMDG
I ran the attached program on the Subversion trunk. output attached.
There are quite a few headers that suppress warnings but fail to restore them. Some of these are harmless and a a few are paired disable/enable headers.
./boost/fusion/sequence/container/map/detail/map_lookup.hpp ./boost/fusion/sequence/container/set/detail/set_lookup.hpp ./boost/fusion/support/detail/compiler_config.hpp ./boost/spirit/core/primitives/impl/primitives.ipp ./boost/spirit/core/primitives/primitives.hpp ./tools/quickbook/detail/actions.hpp Fixed. Thanks! -- Joel de Guzman http://www.boost-consulting.com http://spirit.sf.net
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: 10 August 2007 02:17 To: boost@lists.boost.org Subject: [boost] warning suppression
AMDG
I ran the attached program on the Subversion trunk. output attached.
There are quite a few headers that suppress warnings but fail to restore them. Some of these are harmless and a a few are paired disable/enable headers.
Thanks for this neat test. A nice example of using Boost's file system and regex :-) Glad to see that I am not the only one to have forgotten a pop or two ;-) It would be nice to have this test run before release at least, though it's not a showstopper, it is bad manners to change the warning level and leave it thus. Should I/someone nag the authors? How? Paul --- Paul A Bristow Prizet Farmhouse, Kendal, Cumbria UK LA8 8AB +44 1539561830 & SMS, Mobile +44 7714 330204 & SMS pbristow@hetp.u-net.com
2007/8/10, Paul A Bristow <pbristow@hetp.u-net.com>:
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: 10 August 2007 02:17 To: boost@lists.boost.org Subject: [boost] warning suppression
AMDG
I ran the attached program on the Subversion trunk. output attached.
There are quite a few headers that suppress warnings but fail to restore them. Some of these are harmless and a a few are paired disable/enable headers.
It would be nice to have this test run before release at least, though it's not a showstopper, it is bad manners to change the warning level and leave it thus.
Why not in the commit hook? /$
From: Henrik Sundberg
2007/8/10, Paul A Bristow <pbristow@hetp.u-net.com>:
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: 10 August 2007 02:17 To: boost@lists.boost.org Subject: [boost] warning suppression
AMDG
I ran the attached program on the Subversion trunk. output attached.
There are quite a few headers that suppress warnings but fail to restore them. Some of these are harmless and a a few are paired disable/enable headers.
It would be nice to have this test run before release at least, though it's not a showstopper, it is bad manners to change the warning level and leave it thus.
Why not in the commit hook?
Because "a few are paired disable/enable headers". That seems a reasonable implementation approach to me, and a commit hook would prevent it. -- Martin Bonner Project Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer
Martin Bonner wrote:
Why not in the commit hook?
Because "a few are paired disable/enable headers". That seems a reasonable implementation approach to me, and a commit hook would prevent it.
True, but we can always have a back door for the cases that need it: // [no_check_push_pop] could disable the check? John.
John Maddock wrote:
Martin Bonner wrote:
Why not in the commit hook?
Because "a few are paired disable/enable headers". That seems a reasonable implementation approach to me, and a commit hook would prevent it.
True, but we can always have a back door for the cases that need it:
// [no_check_push_pop]
could disable the check?
Unless there was a reference to the "other file" that contained the other operation. Then you could check that it matched across files, even in the commit-hook. Cheers, /Marcus
----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Paul A Bristow Sent: 10 August 2007 09:50 To: boost@lists.boost.org Subject: Re: [boost] warning suppression
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: 10 August 2007 02:17 To: boost@lists.boost.org Subject: [boost] warning suppression
AMDG
I ran the attached program on the Subversion trunk. output attached.
There are quite a few headers that suppress warnings but fail to restore them. Some of these are harmless and a a few are paired disable/enable headers.
Thanks for this neat test. A nice example of using Boost's file system and regex :-)
Glad to see that I am not the only one to have forgotten a pop or two ;-)
It would be nice to have this test run before release at least, though it's not a showstopper, it is bad manners to change the warning level and leave it thus.
Should I/someone nag the authors? How?
Raise Trac tickets for each component with a problem? -- Martin Bonner Project Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer
AMDG Paul A Bristow <pbristow <at> hetp.u-net.com> writes:
It would be nice to have this test run before release at least, though it's not a showstopper, it is bad manners to change the warning level and leave it thus.
Should I/someone nag the authors? How?
What about adding it to inspect? In Christ, Steven Watanabe
Steven Watanabe <watanabesj <at> gmail.com> writes:
./boost/statechart/detail/rtti_policy.hpp ./boost/statechart/detail/state_base.hpp ./boost/statechart/state_machine.hpp
All disabled warnings carry the following comment: // We permanently turn off the following level 4 warnings because users will // have to do so themselves anyway if we turn them back on The warnings in question are: # pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing. Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Andreas Huber Sent: 10 August 2007 13:29 To: boost@lists.boost.org Subject: Re: [boost] warning suppression
Steven Watanabe <watanabesj <at> gmail.com> writes:
./boost/statechart/detail/rtti_policy.hpp ./boost/statechart/detail/state_base.hpp ./boost/statechart/state_machine.hpp
All disabled warnings carry the following comment:
// We permanently turn off the following level 4 warnings because users will // have to do so themselves anyway if we turn them back on
The warnings in question are:
# pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated
IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing.
OK, OK, OK - you won't get a Trac from me ;-) Clearly, if we are to enforce push'n'pop, we need to have an opt-out. But I don't want to make a mountain out of a modest molehill. Won't a check slow down commit? On the other hand, the time when it is least trouble to correct or declare it deliberate is at commit time. Paul PS There are similar features in gcc... but let's not go there just yet?
Andreas Huber wrote:
Steven Watanabe <watanabesj <at> gmail.com> writes:
./boost/statechart/detail/rtti_policy.hpp ./boost/statechart/detail/state_base.hpp ./boost/statechart/state_machine.hpp
All disabled warnings carry the following comment:
// We permanently turn off the following level 4 warnings because users will // have to do so themselves anyway if we turn them back on
The warnings in question are:
# pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated
IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing.
I agree that they're useless, but I believe they'll still get suppressed for any instances of your templates that are instantiated if there is a matching #pragma warning(pop). John.
Hi John John Maddock <john <at> johnmaddock.co.uk> writes:
# pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated
IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing.
I agree that they're useless, but I believe they'll still get suppressed for any instances of your templates that are instantiated if there is a matching #pragma warning(pop).
Correct. However, typical Statechart client code looks as follows: #include <boost/statechart/simple_state.hpp> struct MyMachine ....; struct MyState : sc::simple_state<MyState, MyMachine> { ... }; simple_state<> ultimately derives from noncopyable, which is why the warnings are generated in the first place. Popping at the end of the header would suppress the warnings for simple_state<> but would produce warnings for MyState. Since I believe that *all* users who care about warnings will disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header. Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Andreas Huber wrote: :: Hi John :: :: John Maddock <john <at> johnmaddock.co.uk> writes: :::: # pragma warning( disable: 4511 ) // copy constructor could not :::: be generated # pragma warning( disable: 4512 ) // assignment op :::: could not be generated :::: :::: IMO, these warnings are not very useful. Users will get heaps of :::: them when they employ the library, so disabling them permanently :::: is a good thing. ::: ::: I agree that they're useless, but I believe they'll still get ::: suppressed for any instances of your templates that are ::: instantiated if there is a matching #pragma warning(pop). :: :: Correct. However, typical Statechart client code looks as follows: :: :: #include <boost/statechart/simple_state.hpp> :: :: struct MyMachine ....; :: struct MyState : sc::simple_state<MyState, MyMachine> { ... }; :: :: simple_state<> ultimately derives from noncopyable, which is why :: the warnings are generated in the first place. Popping at the end :: of the header would suppress the warnings for simple_state<> but :: would produce warnings for MyState. Since I believe that *all* :: users who care about warnings will disable these in their code :: anyway, I think the most user-friendly approach is to permanently :: disable them in the library header. :: Even those who don't care, but use one of the MS frameworks, like MFC or ATL, will get them disabled anyway. These warnings are disabled in 8-10 headers supplied with VC8. No pushing, no popping! Bo Persson
Andreas Huber wrote:
Hi John
John Maddock <john <at> johnmaddock.co.uk> writes:
# pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated
IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing. I agree that they're useless, but I believe they'll still get suppressed for any instances of your templates that are instantiated if there is a matching #pragma warning(pop).
Correct. However, typical Statechart client code looks as follows:
#include <boost/statechart/simple_state.hpp>
struct MyMachine ....; struct MyState : sc::simple_state<MyState, MyMachine> { ... };
simple_state<> ultimately derives from noncopyable, which is why the warnings are generated in the first place. Popping at the end of the header would suppress the warnings for simple_state<> but would produce warnings for MyState. Since I believe that *all* users who care about warnings will disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header.
I'm not sure these warnings are caused by derivation from noncopyable. I tried to compile class A : private boost::noncopyable { }; in warning level 4 and got no warnings at all. I only get those 4511/4512 warnings when I write a class with a data member (not base class) that is not CopyConstrucible/Assignable. I solve these warnings either by suppressing them, or by declaring the copy-c'tor/assignment-op as private, and with no body (private declaration only).
Hi Yuval "Yuval Ronen" <ronen_yuval@yahoo.com> wrote in message news:f9nak5$i45$1@sea.gmane.org...
simple_state<> ultimately derives from noncopyable, which is why the warnings are generated in the first place. Popping at the end of the header would suppress the warnings for simple_state<> but would produce warnings for MyState. Since I believe that *all* users who care about warnings will disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header.
I'm not sure these warnings are caused by derivation from noncopyable. I tried to compile
class A : private boost::noncopyable { };
in warning level 4 and got no warnings at all. I only get those 4511/4512 warnings when I write a class with a data member (not base class) that is not CopyConstrucible/Assignable.
Ok, that could indeed be the case for my code as well. I'll look into it. Thanks for the hint! Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Andreas Huber wrote:
Since I believe that *all* users who care about warnings will disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header.
BTW, the fact that all users who care about warnings will disable these in their code anyway, is an argument *against* disabling them permanently in the library header, not for it... Why insist on being "rude" as Michael claimed (and I happen to agree with him) by forcing such decision on the user, if all users who care will make that decision anyway? If the answer here is "because I save the user from disabling these warning himself" then it's not true. The user will have to disable them himself because they appear all over the place, not only from the Boost.Statechart code. And if he didn't disable them, then I guess he enjoys seeing pages of warnings, and that's his choice.
Yuval Ronen <ronen_yuval <at> yahoo.com> writes:
Andreas Huber wrote:
Since I believe that *all* users who care about warnings will disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header.
BTW, the fact that all users who care about warnings will disable these in their code anyway, is an argument *against* disabling them permanently in the library header, not for it...
My wording is unclear, let me rephrase: Since I believe that *all* users who care about warnings will *want* to disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header. (Note the added "want")
Why insist on being "rude" as Michael claimed (and I happen to agree with him) by forcing such decision on the user, if all users who care will make that decision anyway? If the answer here is "because I save the user from disabling these warning himself" then it's not true. The user will have to disable them himself because they appear all over the place, not only from the Boost.Statechart code.
I'm not sure whether that applies to the majority of the users. Anyway, as you have pointed out: The permanent warning suppression might well be avoidable, which would render this discussion obsolete. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Andreas Huber wrote:
Since I believe that *all* users who care about warnings will *want* to disable these in their code anyway, I think the most user-friendly approach is to permanently disable them in the library header.
Not necessarily correct. I care about warnings (most of the time) but don't disable 4511/4512 and don't get them, except from third-party code.
Andreas Huber wrote:
Steven Watanabe <watanabesj <at> gmail.com> writes:
./boost/statechart/detail/rtti_policy.hpp ./boost/statechart/detail/state_base.hpp ./boost/statechart/state_machine.hpp
All disabled warnings carry the following comment:
// We permanently turn off the following level 4 warnings because users will // have to do so themselves anyway if we turn them back on
The warnings in question are:
# pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated
IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing.
Thats what I thought until recently I noticed that I had 4 similar functor classes and I was getting this warning in only 1 of them. Turns out I forgotten to make some member pointers const (I only made the pointed to data const). Fixing this actually ended up generating better code on another compiler (the one for the shipping platform) in a performance critical area. I don't think libraries should force such decisions on end users. If you have evaluated your code and made sure it is doing the right thing than disable the push the warning disable but restore it so that users might catch problems in their code. Anything else is rude. Thanks, Michael Marcin
"Michael Marcin" <mmarcin@method-solutions.com> wrote in message news:f9im3f$qqi$1@sea.gmane.org...
# pragma warning( disable: 4511 ) // copy constructor could not be generated # pragma warning( disable: 4512 ) // assignment op could not be generated
IMO, these warnings are not very useful. Users will get heaps of them when they employ the library, so disabling them permanently is a good thing.
Thats what I thought until recently I noticed that I had 4 similar functor classes and I was getting this warning in only 1 of them. Turns out I forgotten to make some member pointers const (I only made the pointed to data const). Fixing this actually ended up generating better code on another compiler (the one for the shipping platform) in a performance critical area.
I'd say you just got lucky as the flaws you've found have little to do with the intent of the warnings. For the vast majority of code these warnings can safely be ignored, because your code will simply refuse to compile, should you ever try to use one of the functions the compiler failed to generate. Other compiler vendors seem to agree (by not adding such warnings to their compiler). So, for most users these warnings are just a nuisance, turning them back on at the end of the header would cause more grief than not doing so.
I don't think libraries should force such decisions on end users. If you have evaluated your code and made sure it is doing the right thing than disable the push the warning disable but restore it so that users might catch problems in their code.
See above, your finding of flaws with these warnings is not exactly common.
Anything else is rude.
The code is open to your inspection and the permanent suppressions can be found very easily as Steven has shown. If you really do care so much about these warnings then turn them back on after including the headers. This approach is more work for you, but makes live easier for the vast majority of users. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
AMDG Andreas Huber <ahd6974-spamboostorgtrap <at> yahoo.com> writes:
See above, your finding of flaws with these warnings is not exactly common.
It's happened to me too. I was accidentally passing a reference type to alignment_of. I knew there was a problem in my code but the warning helped my find it immediately. If you really need to suppress this warning for users you can write template<class T> class C { public: C(const C& c) { BOOST_STATIC_ASSERT((!boost::is_same<T, T>::value)); } C& operator=(const C& c) { BOOST_STATIC_ASSERT((!boost::is_same<T, T>::value)); } }; In Christ, Steven Watanabe
"Steven Watanabe" <steven@providere-consulting.com> wrote in message news:loom.20070811T184620-255@post.gmane.org...
See above, your finding of flaws with these warnings is not exactly common.
It's happened to me too. I was accidentally passing a reference type to alignment_of. I knew there was a problem in my code but the warning helped my find it immediately. If you really need to suppress this warning for users you can write
template<class T> class C { public: C(const C& c) { BOOST_STATIC_ASSERT((!boost::is_same<T, T>::value)); } C& operator=(const C& c) { BOOST_STATIC_ASSERT((!boost::is_same<T, T>::value)); } };
I don't see how this could work for my situation, see my answer to John. Statechart users derive their normal (non-template) classes from the class template statechart::simple_state. Unless I'm missing something, adding static asserts to simple_state (as you do in the class template C) would not work as the compiler would then try to generate copy functions for the user-defined classes only to issue an error when it tries to instantiate simple_state copy functions. I ask again: If you really do care so much about these warnings then why don't you simply reenable them after including the headers that disable them? As Bo has mentioned, chances are that other headers that you include also disable these warnings. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
AMDG Andreas Huber <ahd6974-spamboostorgtrap <at> yahoo.com> writes:
Unless I'm missing something, adding static asserts to simple_state (as you do in the class template C) would not work as the compiler would then try to generate copy functions for the user-defined classes only to issue an error when it tries to instantiate simple_state copy functions.
No. The compiler will only generate the /declarations/ if they are not used. See section 12.8 of the standard.
I ask again: If you really do care so much about these warnings then why don't you simply reenable them after including the headers that disable them? As Bo has mentioned, chances are that other headers that you include also disable these warnings.
Most users including myself don't want to search headers just to find what warnings they need to turn back on. The fact that some other headers turn off the warnings is no excuse. They should be fixed too, IMO. In Christ, Steven Watanabe
"Steven Watanabe" <steven@providere-consulting.com> wrote in message news:loom.20070813T214009-336@post.gmane.org...
No. The compiler will only generate the /declarations/ if they are not used. See section 12.8 of the standard.
Ah yes, I was missing something.
I ask again: If you really do care so much about these warnings then why don't you simply reenable them after including the headers that disable them? As Bo has mentioned, chances are that other headers that you include also disable these warnings.
Most users including myself don't want to search headers just to find what warnings they need to turn back on.
The warnings were originally permanently disabled under the assumption that they cannot be avoided otherwise (this assumption might be wrong according to Yuval and Peter but lets put that aside for the moment). I still believe that only a small minority of the users cares about these warnings, I therefore maintain my assertion that setting the defaults such that this minority needs to do additional work as opposed to the vast majority was the right thing to do. Now, I do agree that avoiding warnings is usually better than disabling them (temporary or otherwise). So, if it turns out that they can be avoided I will change the headers accordingly, as long as this does not require additional work by library users.
The fact that some other headers turn off the warnings is no excuse. They should be fixed too, IMO.
Excuse or not, the permanent suppression of these warnings by other headers and the lack of equivalent warnings in other compilers just shows that the value of these warnings is quite limited. -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.
Andreas Huber wrote:
"Michael Marcin" <mmarcin@method-solutions.com> wrote in message
Anything else is rude.
The code is open to your inspection and the permanent suppressions can be found very easily as Steven has shown. If you really do care so much about these warnings then turn them back on after including the headers. This approach is more work for you, but makes live easier for the vast majority of users.
I happen to agree with Michael here. Suggesting to re-enable them is beside the point here. The point is that a library shouldn't change compiler settings for the user. Period. It doesn't matter whether that warning has any value at all or it's completely useless. What's important is that the user has the right to assume that he gets to decide the compiler settings, in whatever method he chooses (out of the methods the compiler vendor provides). The user can re-enable the warning, yes, but that's forcing him to a specific way, which will make users angry, and rightfully. Don't try to fix Visual Studio, and force your fix on the users. Let them decide what they consider important and what not, what they want to fix in VS and what not, even if you're certain you know better. Anything else is rude.
participants (13)
-
Andreas Huber -
Bo Persson -
Henrik Sundberg -
Joel de Guzman -
John Maddock -
Marcus Lindblom -
Martin Bonner -
Michael Marcin -
Paul A Bristow -
Peter Dimov -
Steven Watanabe -
Steven Watanabe -
Yuval Ronen