Re: [boost] [exception] warning about non-virtual destructor - resolution?
This is annoying me too: boost_1_49_0/boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type 'boost::error_info<boost::tag_original_exception_type, const std::type_info*>' which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor] A) easy solution /boost_1_49_0/boost/exception/detail/error_info_impl.hpp It's probably easiest to just make ~error_info_base() virtual. It doesn't cost much since there's already a virtual table. B) harder solution The other cooler C++11 alternative is declare error_info final. I tried it on gcc-4.7.0 and it works wonderfully. Since the compiler knows you can not derive a class from that type it knows a that deconstructor is complete. struct base{ virtual void foo(); }; struct derived final : base { }; //struct illegal : derived {}; // error: cannot derive from 'final' base 'derived' in derived type 'illegal' void kill(derived* d){ delete d; // no complaint } I vote for A, because you'd anyway have to detect support for final. Chris
On 04/13/2012 03:06 PM, Hite, Christopher wrote:
This is annoying me too: boost_1_49_0/boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type 'boost::error_info<boost::tag_original_exception_type, const std::type_info*>' which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor]
A) easy solution /boost_1_49_0/boost/exception/detail/error_info_impl.hpp It's probably easiest to just make ~error_info_base() virtual. It doesn't cost much since there's already a virtual table.
This is bug #4200, and the above is the proposed resolution.
On Fri, Apr 13, 2012 at 10:15 AM, Mathias Gaunard < mathias.gaunard@ens-lyon.org> wrote:
On 04/13/2012 03:06 PM, Hite, Christopher wrote:
This is annoying me too: boost_1_49_0/boost/checked_**delete.hpp:34:5: warning: deleting object of polymorphic class type 'boost::error_info<boost::tag_**original_exception_type, const std::type_info*>' which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor]
A) easy solution /boost_1_49_0/boost/exception/**detail/error_info_impl.hpp It's probably easiest to just make ~error_info_base() virtual. It doesn't cost much since there's already a virtual table.
This is bug #4200, and the above is the proposed resolution.
I seem to remember this being discussed at length in the past...I believe this thread? http://boost.2283326.n4.nabble.com/boost-exception-detail-error-info-base-do... - Jeff
On 13/04/12 19:31, Jeffrey Lee Hellrung, Jr. wrote:
I seem to remember this being discussed at length in the past...I believe this thread?
http://boost.2283326.n4.nabble.com/boost-exception-detail-error-info-base-do...
Yes, basically, the author said he didn't care and it wasn't virtual by design, and the compiler warning was bogus. In the meantime, it's a major annoyance for all users of this library. I'm hoping a sufficient amount of people can point it out to convince the author to include the fix.
On 14 April 2012 14:00, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
Yes, basically, the author said he didn't care and it wasn't virtual by design, and the compiler warning was bogus.
In the meantime, it's a major annoyance for all users of this library. I'm hoping a sufficient amount of people can point it out to convince the author to include the fix.
A better tactic would be to help him reproduce the warning. It looks like he's using '#pragma GCC system_header' which IIRC is an imperfect warning suppression mechanism - although I forget why. So I guess you're hitting whatever the weakness is. Gcc 4.6 added pragmas for pushing and popping warning state in a similar manner to Visual C++, maybe they could be used (obviously won't work for older versions).
On 14/04/12 17:18, Daniel James wrote:
On 14 April 2012 14:00, Mathias Gaunard<mathias.gaunard@ens-lyon.org> wrote:
Yes, basically, the author said he didn't care and it wasn't virtual by design, and the compiler warning was bogus.
In the meantime, it's a major annoyance for all users of this library. I'm hoping a sufficient amount of people can point it out to convince the author to include the fix.
A better tactic would be to help him reproduce the warning. It looks like he's using '#pragma GCC system_header' which IIRC is an imperfect warning suppression mechanism - although I forget why.
All preprocessor-based warning suppression mechanisms do not work with warnings generated when templates are instantiated.
On Sun, Apr 15, 2012 at 1:43 AM, Mathias Gaunard < mathias.gaunard@ens-lyon.org> wrote:
On 14/04/12 17:18, Daniel James wrote:
On 14 April 2012 14:00, Mathias Gaunard<mathias.gaunard@ens-**lyon.org<mathias.gaunard@ens-lyon.org>> wrote:
Yes, basically, the author said he didn't care and it wasn't virtual by design, and the compiler warning was bogus.
In the meantime, it's a major annoyance for all users of this library. I'm hoping a sufficient amount of people can point it out to convince the author to include the fix.
A better tactic would be to help him reproduce the warning. It looks like he's using '#pragma GCC system_header' which IIRC is an imperfect warning suppression mechanism - although I forget why.
All preprocessor-based warning suppression mechanisms do not work with warnings generated when templates are instantiated.
I'm using the new GCC warning pragmas to disable warnings in the Boost headers (including those caused by template instantiation) just fine. Could you provide a sample where this does not work? Example of what I'm currently doing (yes, I have a stupid amount of warnings enabled): #pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Weffc++" #pragma GCC diagnostic ignored "-Wshadow" #pragma GCC diagnostic ignored "-pedantic" #pragma GCC diagnostic ignored "-Wconversion" #pragma GCC diagnostic ignored "-Wunused-value" #pragma GCC diagnostic ignored "-Wstrict-aliasing" #pragma GCC diagnostic ignored "-Wnon-virtual-dtor" #pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor" #pragma GCC diagnostic ignored "-Wmissing-field-initializers" #include <boost/locale.hpp> #include <boost/thread.hpp> #include <boost/filesystem.hpp> #include <boost/archive/text_iarchive.hpp> #include <boost/interprocess/ipc/message_queue.hpp> #pragma GCC diagnostic pop
On 14 April 2012 16:43, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
On 14/04/12 17:18, Daniel James wrote:
A better tactic would be to help him reproduce the warning. It looks like he's using '#pragma GCC system_header' which IIRC is an imperfect warning suppression mechanism - although I forget why.
All preprocessor-based warning suppression mechanisms do not work with warnings generated when templates are instantiated.
Apart from being wrong, that doesn't reproduce the warning. Why don't you try writing a code sample to demonstrate the problem? And then describe your exact setup (version, platform, compiler, command line etc.).
On Sat, Apr 14, 2012 at 6:00 AM, Mathias Gaunard < mathias.gaunard@ens-lyon.org> wrote:
On 13/04/12 19:31, Jeffrey Lee Hellrung, Jr. wrote:
I seem to remember this being discussed at length in the past...I believe
this thread?
http://boost.2283326.n4.**nabble.com/boost-exception-** detail-error-info-base-does-**not-have-virtual-destructor-** td3384903.html<http://boost.2283326.n4.nabble.com/boost-exception-detail-error-info-base-does-not-have-virtual-destructor-td3384903.html>
Yes, basically, the author said he didn't care and it wasn't virtual by design, and the compiler warning was bogus.
My personal opinion about this warning aside, I do want to suppress it. I simply can't reproduce it, and nobody has provided me with code that reproduces the problem. If you see the warning, grab the code and post it in the bug tracker. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On Sat, 14 Apr 2012, Emil Dotchevski wrote:
On Sat, Apr 14, 2012 at 6:00 AM, Mathias Gaunard < mathias.gaunard@ens-lyon.org> wrote:
On 13/04/12 19:31, Jeffrey Lee Hellrung, Jr. wrote:
I seem to remember this being discussed at length in the past...I believe
this thread?
http://boost.2283326.n4.**nabble.com/boost-exception-** detail-error-info-base-does-**not-have-virtual-destructor-** td3384903.html<http://boost.2283326.n4.nabble.com/boost-exception-detail-error-info-base-does-not-have-virtual-destructor-td3384903.html>
Yes, basically, the author said he didn't care and it wasn't virtual by design, and the compiler warning was bogus.
My personal opinion about this warning aside, I do want to suppress it. I simply can't reproduce it, and nobody has provided me with code that reproduces the problem. If you see the warning, grab the code and post it in the bug tracker.
On GCC 4.7.0, trying to compile libs/thread/future.cpp triggers a similar warning for me with -Wall: gcc.compile.c++ bin.v2/libs/thread/build/gcc-4.7.0/debug/link-static/threading-multi/future.o In file included from ./boost/smart_ptr/shared_ptr.hpp:30:0, from ./boost/shared_ptr.hpp:17, from ./boost/date_time/time_clock.hpp:17, from ./boost/thread/thread_time.hpp:9, from ./boost/thread/future.hpp:14, from libs/thread/src/future.cpp:6: ./boost/checked_delete.hpp: In instantiation of ‘void boost::checked_delete(T*) [with T = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]’: ./boost/smart_ptr/detail/shared_count.hpp:95:13: required from ‘boost::detail::shared_count::shared_count(Y*) [with Y = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]’ ./boost/smart_ptr/shared_ptr.hpp:177:50: required from ‘boost::shared_ptr<T>::shared_ptr(Y*) [with Y = boost::error_info<boost::tag_original_exception_type, const std::type_info*>; T = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]’ ./boost/exception/info.hpp:171:69: required from ‘const E& boost::exception_detail::set_info(const E&, const boost::error_info<Tag, T>&) [with E = boost::unknown_exception; Tag = boost::tag_original_exception_type; T = const std::type_info*]’ ./boost/exception/info.hpp:192:46: required from ‘typename boost::enable_if<boost::exception_detail::derives_boost_exception<E>, const E&>::type boost::operator<<(const E&, const boost::error_info<Tag, T>&) [with E = boost::unknown_exception; Tag = boost::tag_original_exception_type; T = const std::type_info*; typename boost::enable_if<boost::exception_detail::derives_boost_exception<E>, const E&>::type = const boost::unknown_exception&]’ ./boost/exception/detail/exception_ptr.hpp:182:13: required from ‘void boost::unknown_exception::add_original_type(const E&) [with E = std::exception]’ ./boost/exception/detail/exception_ptr.hpp:161:32: required from here ./boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type ‘boost::error_info<boost::tag_original_exception_type, const std::type_info*>’ which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor] -- Jeremiah Willcock
2012/4/14 Jeremiah Willcock <jewillco@osl.iu.edu>:
./boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type ‘boost::error_info<boost::tag_original_exception_type, const std::type_info*>’ which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor]
That's a new warning for g++ 4.7, so unfortunately it doesn't explain the original bug report. It would be useful to have a demonstration of that as well. Anyone have any ideas? In this case the warning fires because it's triggered by the delete command, which is not covered by the system header pragma in the exception headers. This is different to the old warning which was triggered by the destructor. This might actually be a problem for other libraries as well. 'checked_delete' is used by 'shared_ptr' and is supposed to be safe for the case when the destructor isn't virtual, so this might result in warnings for valid uses of 'shared_ptr'. So, for g++ 4.7 and up, the correct solution *might* be to use the new pragmas to disable the warning in checked_delete. But it might not, I'm not familiar enough with checked_delete to know if that would be appropriate.
On Saturday 14 April 2012 22:13:36 Daniel James wrote:
'checked_delete' is used by 'shared_ptr' and is supposed to be safe for the case when the destructor isn't virtual, so this might result in warnings for valid uses of 'shared_ptr'.
My understanding is that checked_delete is intended to protect against freeing a pointer of an incomplete type, not against a missing virtual destructor. It is a tool of itself which can be used outside shared_ptr and in these cases a warning may be most appropriate. IMHO, if shared_ptr aims to support valid behavior in the lack of a virtual destructor, it should deal with the warnings itself.
On 14 April 2012 22:30, Andrey Semashev <andrey.semashev@gmail.com> wrote:
On Saturday 14 April 2012 22:13:36 Daniel James wrote:
'checked_delete' is used by 'shared_ptr' and is supposed to be safe for the case when the destructor isn't virtual, so this might result in warnings for valid uses of 'shared_ptr'.
My understanding is that checked_delete is intended to protect against freeing a pointer of an incomplete type, not against a missing virtual destructor. It is a tool of itself which can be used outside shared_ptr and in these cases a warning may be most appropriate. IMHO, if shared_ptr aims to support valid behavior in the lack of a virtual destructor, it should deal with the warnings itself.
I don't think shared_ptr can do that. There are two places where the warning can be dealt with (the delete command and the deleted object), neither of which are part of shared_ptr. Unless I've missed something the only way it could deal with it without changing checked_delete is to use its own implementation of checked_delete. If shared_ptr can't use checked_delete then that suggests to me a real weakness in checked_delete. The solution *might* be to add another function to checked_delete that doesn't warn. Incidentally this warning does come up for 'shared_ptr_test'.
On Sunday 15 April 2012 09:46:19 Daniel James wrote:
On 14 April 2012 22:30, Andrey Semashev <andrey.semashev@gmail.com> wrote:
My understanding is that checked_delete is intended to protect against freeing a pointer of an incomplete type, not against a missing virtual destructor. It is a tool of itself which can be used outside shared_ptr and in these cases a warning may be most appropriate. IMHO, if shared_ptr aims to support valid behavior in the lack of a virtual destructor, it should deal with the warnings itself.
I don't think shared_ptr can do that. There are two places where the warning can be dealt with (the delete command and the deleted object), neither of which are part of shared_ptr. Unless I've missed something the only way it could deal with it without changing checked_delete is to use its own implementation of checked_delete. If shared_ptr can't use checked_delete then that suggests to me a real weakness in checked_delete. The solution *might* be to add another function to checked_delete that doesn't warn.
Using a separate implementation of checked_delete in shared_ptr is what I had in mind. And this implementation should not be public, I think.
On Sun, Apr 15, 2012 at 10:54 AM, Andrey Semashev <andrey.semashev@gmail.com> wrote:
Using a separate implementation of checked_delete in shared_ptr is what I had in mind. And this implementation should not be public, I think.
Isn't the type guaranteed to be complete when shared_ptr calls delete? -- Olaf
On Sunday 15 April 2012 12:34:07 Olaf van der Spek wrote:
On Sun, Apr 15, 2012 at 10:54 AM, Andrey Semashev
<andrey.semashev@gmail.com> wrote:
Using a separate implementation of checked_delete in shared_ptr is what I had in mind. And this implementation should not be public, I think.
Isn't the type guaranteed to be complete when shared_ptr calls delete?
No. The deleter is initialized at the shared_ptr construction and you can initialize shared_ptr with a pointer allocated somewhere else. At the point of shared_ptr construction the type may be incomplete.
On Sun, Apr 15, 2012 at 8:17 AM, Andrey Semashev <andrey.semashev@gmail.com>wrote:
On Sunday 15 April 2012 12:34:07 Olaf van der Spek wrote:
On Sun, Apr 15, 2012 at 10:54 AM, Andrey Semashev
<andrey.semashev@gmail.com> wrote:
Using a separate implementation of checked_delete in shared_ptr is what I had in mind. And this implementation should not be public, I think.
Isn't the type guaranteed to be complete when shared_ptr calls delete?
No. The deleter is initialized at the shared_ptr construction and you can initialize shared_ptr with a pointer allocated somewhere else. At the point of shared_ptr construction the type may be incomplete.
I'm pretty sure that this warning shouldn't be dealt with within the shared_ptr framework, since it does seem to refer specifically to the boost::error_info's destructor. Does anyone have a simple cpp file and an exact g++ command line that triggers the warning? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On 15 April 2012 20:00, Emil Dotchevski <emildotchevski@gmail.com> wrote:
I'm pretty sure that this warning shouldn't be dealt with within the shared_ptr framework, since it does seem to refer specifically to the boost::error_info's destructor.
The issue isn't just in exception, the warning also occurs in the shared_ptr unit tests.
Does anyone have a simple cpp file and an exact g++ command line that triggers the warning?
demo.cpp: #include <boost/checked_delete.hpp> #include <boost/exception_ptr.hpp> Compiled with: g++-mp-4.7 -c -Wall -I $BOOST_ROOT demo.cpp g++-mp-4.7 is the g++ 4.7 executable on my computer. checked_delete.hpp is included before exception_ptr.hpp so that it isn't covered by exception's warning suppression.
On Sun, Apr 15, 2012 at 12:55 PM, Daniel James <dnljms@gmail.com> wrote:
On 15 April 2012 20:00, Emil Dotchevski <emildotchevski@gmail.com> wrote:
I'm pretty sure that this warning shouldn't be dealt with within the shared_ptr framework, since it does seem to refer specifically to the boost::error_info's destructor.
The issue isn't just in exception, the warning also occurs in the shared_ptr unit tests.
Does anyone have a simple cpp file and an exact g++ command line that triggers the warning?
demo.cpp:
#include <boost/checked_delete.hpp> #include <boost/exception_ptr.hpp>
Compiled with:
g++-mp-4.7 -c -Wall -I $BOOST_ROOT demo.cpp
Compiling this with g++ 4.7 on Windows produces no console output with the following command line: g++ -c -Wall demo.cpp Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Le 16/04/12 21:01, Emil Dotchevski a écrit :
On Sun, Apr 15, 2012 at 12:55 PM, Daniel James<dnljms@gmail.com> wrote:
On 15 April 2012 20:00, Emil Dotchevski<emildotchevski@gmail.com> wrote:
I'm pretty sure that this warning shouldn't be dealt with within the shared_ptr framework, since it does seem to refer specifically to the boost::error_info's destructor. The issue isn't just in exception, the warning also occurs in the shared_ptr unit tests.
Does anyone have a simple cpp file and an exact g++ command line that triggers the warning? demo.cpp:
#include<boost/checked_delete.hpp> #include<boost/exception_ptr.hpp>
Compiled with:
g++-mp-4.7 -c -Wall -I $BOOST_ROOT demo.cpp
Compiling this with g++ 4.7 on Windows produces no console output with the following command line:
g++ -c -Wall demo.cpp
Hi, the warning appears while running the regression test on trunk for Boost.Thread. Maybe you could try adding -Wextra -pedantic and maybe -fpermissive. Best, Vicente
On Mon, Apr 16, 2012 at 9:42 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Maybe you could try adding -Wextra -pedantic and maybe -fpermissive.
Nope olaf@testing:~$ cat demo.cpp #include <boost/checked_delete.hpp> #include <boost/exception_ptr.hpp> olaf@testing:~$ g++-4.7 demo.cpp -Wall -Wextra -pedantic -c -fpermissive olaf@testing:~$ -- Olaf
On 16 April 2012 20:01, Emil Dotchevski <emildotchevski@gmail.com> wrote:
On Sun, Apr 15, 2012 at 12:55 PM, Daniel James <dnljms@gmail.com> wrote:
demo.cpp:
#include <boost/checked_delete.hpp> #include <boost/exception_ptr.hpp>
Compiled with:
g++-mp-4.7 -c -Wall -I $BOOST_ROOT demo.cpp
Compiling this with g++ 4.7 on Windows produces no console output with the following command line:
g++ -c -Wall demo.cpp
Where did you install g++ from? And can you try running 'g++ --version'? If you let me know I'll try it later. Clang apparently has the same warning, although I'm still using 2.9, which doesn't.
On 17 April 2012 08:22, Daniel James <dnljms@gmail.com> wrote:
Clang apparently has the same warning, although I'm still using 2.9, which doesn't.
I've installed clang 3.1, and based on simple testing, it has the same behaviour as gcc 4.7.
On Tue, Apr 17, 2012 at 1:34 PM, Daniel James <dnljms@gmail.com> wrote:
On 17 April 2012 08:22, Daniel James <dnljms@gmail.com> wrote:
Clang apparently has the same warning, although I'm still using 2.9, which doesn't.
I've installed clang 3.1, and based on simple testing, it has the same behaviour as gcc 4.7.
Can I see a command line and a complete cpp file that triggers the warning? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On 17 April 2012 23:20, Emil Dotchevski <emildotchevski@gmail.com> wrote:
On Tue, Apr 17, 2012 at 1:34 PM, Daniel James <dnljms@gmail.com> wrote:
On 17 April 2012 08:22, Daniel James <dnljms@gmail.com> wrote:
Clang apparently has the same warning, although I'm still using 2.9, which doesn't.
I've installed clang 3.1, and based on simple testing, it has the same behaviour as gcc 4.7.
Can I see a command line and a complete cpp file that triggers the warning?
Same as before but with clang-mp-3.1 instead of g++-mp-4.7.
On Tue, Apr 17, 2012 at 3:36 PM, Daniel James <dnljms@gmail.com> wrote:
On 17 April 2012 23:20, Emil Dotchevski <emildotchevski@gmail.com> wrote:
On Tue, Apr 17, 2012 at 1:34 PM, Daniel James <dnljms@gmail.com> wrote:
On 17 April 2012 08:22, Daniel James <dnljms@gmail.com> wrote:
Clang apparently has the same warning, although I'm still using 2.9, which doesn't.
I've installed clang 3.1, and based on simple testing, it has the same behaviour as gcc 4.7.
Can I see a command line and a complete cpp file that triggers the warning?
Same as before but with clang-mp-3.1 instead of g++-mp-4.7.
Look, I don't know what "before" means. Why don't you file a trac ticket with all the relevant information, and I'll make sure it's taken care of. Exact cpp file, OS, compiler version. Thanks, Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Le 18/04/12 00:44, Emil Dotchevski a écrit :
On Tue, Apr 17, 2012 at 3:36 PM, Daniel James<dnljms@gmail.com> wrote:
On 17 April 2012 23:20, Emil Dotchevski<emildotchevski@gmail.com> wrote:
On Tue, Apr 17, 2012 at 1:34 PM, Daniel James<dnljms@gmail.com> wrote:
On 17 April 2012 08:22, Daniel James<dnljms@gmail.com> wrote:
Clang apparently has the same warning, although I'm still using 2.9, which doesn't. I've installed clang 3.1, and based on simple testing, it has the same behaviour as gcc 4.7.
Can I see a command line and a complete cpp file that triggers the warning?
Same as before but with clang-mp-3.1 instead of g++-mp-4.7.
Look, I don't know what "before" means. Why don't you file a trac ticket with all the relevant information, and I'll make sure it's taken care of. Exact cpp file, OS, compiler version.
Hi, Daniel posted one two days ago demo.cpp: #include<boost/checked_delete.hpp> #include<boost/exception_ptr.hpp> Compiled with: g++-mp-4.7 -c -Wall -I $BOOST_ROOT demo.cpp Best, Vicente
On 18 April 2012 00:16, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
demo.cpp:
#include<boost/checked_delete.hpp> #include<boost/exception_ptr.hpp>
Compiled with:
g++-mp-4.7 -c -Wall -I $BOOST_ROOT demo.cpp
That's the one. When testing, try explicitly using the '-Wdelete-non-virtual-dtor' flag. That way there will be an error if for versions of g++ that don't support it. This is a new feature and some distributions of gcc on windows are often a bit behind linux and mac. The following output if from trunk revision 78061: $ g++-mp-4.7 --version g++-mp-4.7 (GCC) 4.7.0 Copyright (C) 2012 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ g++-mp-4.7 -I $BOOST_ROOT demo.cpp -c -Wdelete-non-virtual-dtor In file included from demo.cpp:1:0: /Users/daniel/boost/git/boost/checked_delete.hpp: In instantiation of 'void boost::checked_delete(T*) [with T = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]': /Users/daniel/boost/git/boost/smart_ptr/detail/shared_count.hpp:95:13: required from 'boost::detail::shared_count::shared_count(Y*) [with Y = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]' /Users/daniel/boost/git/boost/smart_ptr/shared_ptr.hpp:177:50: required from 'boost::shared_ptr<T>::shared_ptr(Y*) [with Y = boost::error_info<boost::tag_original_exception_type, const std::type_info*>; T = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]' /Users/daniel/boost/git/boost/exception/info.hpp:171:69: required from 'const E& boost::exception_detail::set_info(const E&, const boost::error_info<Tag, T>&) [with E = boost::unknown_exception; Tag = boost::tag_original_exception_type; T = const std::type_info*]' /Users/daniel/boost/git/boost/exception/info.hpp:192:46: required from 'typename boost::enable_if<boost::exception_detail::derives_boost_exception<E>, const E&>::type boost::operator<<(const E&, const boost::error_info<Tag, T>&) [with E = boost::unknown_exception; Tag = boost::tag_original_exception_type; T = const std::type_info*; typename boost::enable_if<boost::exception_detail::derives_boost_exception<E>, const E&>::type = const boost::unknown_exception&]' /Users/daniel/boost/git/boost/exception/detail/exception_ptr.hpp:182:13: required from 'void boost::unknown_exception::add_original_type(const E&) [with E = std::exception]' /Users/daniel/boost/git/boost/exception/detail/exception_ptr.hpp:161:32: required from here /Users/daniel/boost/git/boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type 'boost::error_info<boost::tag_original_exception_type, const std::type_info*>' which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor] $ clang-mp-3.1 --version clang version 3.1 (trunk 154872) Target: i386-apple-darwin9.8.0 Thread model: posix $ clang-mp-3.1 -I $BOOST_ROOT demo.cpp -c -Wdelete-non-virtual-dtor In file included from demo.cpp:1: /Users/daniel/boost/git/boost/checked_delete.hpp:34:5: warning: delete called on 'boost::error_info<boost::tag_original_exception_type, const std::type_info *>' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor] delete x; ^ /Users/daniel/boost/git/boost/smart_ptr/detail/shared_count.hpp:95:13: note: in instantiation of function template specialization 'boost::checked_delete<boost::error_info<boost::tag_original_exception_type, const std::type_info *> >' requested here boost::checked_delete( p ); ^ /Users/daniel/boost/git/boost/smart_ptr/shared_ptr.hpp:177:44: note: in instantiation of function template specialization 'boost::detail::shared_count::shared_count<boost::error_info<boost::tag_original_exception_type, const std::type_info *> >' requested here explicit shared_ptr( Y * p ): px( p ), pn( p ) // Y must be complete ^ /Users/daniel/boost/git/boost/exception/info.hpp:170:42: note: in instantiation of function template specialization 'boost::shared_ptr<boost::error_info<boost::tag_original_exception_type, const std::type_info *>
::shared_ptr<boost::error_info<boost::tag_original_exception_type, const std::type_info *> >' requested here shared_ptr<error_info_tag_t> p( new error_info_tag_t(v) ); ^ /Users/daniel/boost/git/boost/exception/info.hpp:191:16: note: in instantiation of function template specialization 'boost::exception_detail::set_info<boost::unknown_exception, boost::tag_original_exception_type, const std::type_info *>' requested here return exception_detail::set_info(x,v); ^ /Users/daniel/boost/git/boost/exception/detail/exception_ptr.hpp:181:21: note: in instantiation of function template specialization 'boost::operator<<<boost::unknown_exception, boost::tag_original_exception_type, const std::type_info *>' requested here (*this) << original_exception_type(&typeid(e)); ^ /Users/daniel/boost/git/boost/exception/detail/exception_ptr.hpp:160:13: note: in instantiation of function template specialization 'boost::unknown_exception::add_original_type<std::exception>' requested here add_original_type(e); ^ 1 warning generated.
Look, I don't know what "before" means. Why don't you file a trac ticket with all the relevant information, and I'll make sure it's taken care of. Exact cpp file, OS, compiler version.
I posted updated instructions for reproducing the warning to Trac ticket #4200. Are you able to reproduce the warning now? Thanks, Nate
On Mon, Apr 30, 2012 at 2:03 PM, Nathan Ridge <zeratul976@hotmail.com>wrote:
Look, I don't know what "before" means. Why don't you file a trac ticket with all the relevant information, and I'll make sure it's taken care of. Exact cpp file, OS, compiler version.
I posted updated instructions for reproducing the warning to Trac ticket #4200. Are you able to reproduce the warning now?
I'll take a look, though I have given up -- my conclusion is that some versions of GCC out there have a bug in the system_header pragma, so I made the destructor virtual. Hopefully that doesn't cause other warnings. :) Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On 18/04/12 00:20, Emil Dotchevski wrote:
On Tue, Apr 17, 2012 at 1:34 PM, Daniel James<dnljms@gmail.com> wrote:
On 17 April 2012 08:22, Daniel James<dnljms@gmail.com> wrote:
Clang apparently has the same warning, although I'm still using 2.9, which doesn't.
I've installed clang 3.1, and based on simple testing, it has the same behaviour as gcc 4.7.
Can I see a command line and a complete cpp file that triggers the warning?
You've already been given the example where including checked_delete before exception_ptr is enough to generate the warning. It has also been pointed out that if you're not getting the warning, you're probably not using a real gcc 4.7.0.
On Wed, Apr 18, 2012 at 12:00 PM, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
Can I see a command line and a complete cpp file that triggers the warning?
You've already been given the example where including checked_delete before exception_ptr is enough to generate the warning.
It has also been pointed out that if you're not getting the warning, you're probably not using a real gcc 4.7.0.
Really? olaf@testing:~$ g++-4.7 -c -Wall demo.cpp $ cat demo.cpp #include <boost/checked_delete.hpp> #include <boost/exception_ptr.hpp> $ g++-4.7 -c -Wall demo.cpp -Wdelete-non-virtual-dtor $ g++-4.7 -v Using built-in specs. COLLECT_GCC=g++-4.7 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.7/lto-wrapper Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 4.7.0-3' --with-bugurl=file:///usr/share/doc/gcc-4.7/README.Bugs --enable-languages=c,c++,go,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.7 --enable-shared --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.7 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --enable-plugin --enable-objc-gc --with-arch-32=i586 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.7.0 (Debian 4.7.0-3) -- Olaf
On 18 April 2012 12:16, Olaf van der Spek <ml@vdspek.org> wrote:
On Wed, Apr 18, 2012 at 12:00 PM, Mathias Gaunard
olaf@testing:~$ g++-4.7 -c -Wall demo.cpp
Gcc suppresses warnings for headers installed to certain directories. Try running the test with a local boost checkout.
On Wed, Apr 18, 2012 at 1:28 PM, Daniel James <dnljms@gmail.com> wrote:
On 18 April 2012 12:16, Olaf van der Spek <ml@vdspek.org> wrote:
On Wed, Apr 18, 2012 at 12:00 PM, Mathias Gaunard
olaf@testing:~$ g++-4.7 -c -Wall demo.cpp
Gcc suppresses warnings for headers installed to certain directories. Try running the test with a local boost checkout.
Ah, thanks! $ g++-4.7 -c -Wall demo.cpp -Wdelete-non-virtual-dtor -I t/boost-trunk In file included from demo.cpp:1:0: t/boost-trunk/boost/checked_delete.hpp: In instantiation of ‘void boost::checked_delete(T*) [with T = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]’: t/boost-trunk/boost/smart_ptr/detail/shared_count.hpp:95:13: required from ‘boost::detail::shared_count::shared_count(Y*) [with Y = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]’ t/boost-trunk/boost/smart_ptr/shared_ptr.hpp:177:50: required from ‘boost::shared_ptr<T>::shared_ptr(Y*) [with Y = boost::error_info<boost::tag_original_exception_type, const std::type_info*>; T = boost::error_info<boost::tag_original_exception_type, const std::type_info*>]’ t/boost-trunk/boost/exception/info.hpp:171:69: required from ‘const E& boost::exception_detail::set_info(const E&, const boost::error_info<Tag, T>&) [with E = boost::unknown_exception; Tag = boost::tag_original_exception_type; T = const std::type_info*]’ t/boost-trunk/boost/exception/info.hpp:192:46: required from ‘typename boost::enable_if<boost::exception_detail::derives_boost_exception<E>, const E&>::type boost::operator<<(const E&, const boost::error_info<Tag, T>&) [with E = boost::unknown_exception; Tag = boost::tag_original_exception_type; T = const std::type_info*; typename boost::enable_if<boost::exception_detail::derives_boost_exception<E>, const E&>::type = const boost::unknown_exception&]’ t/boost-trunk/boost/exception/detail/exception_ptr.hpp:182:13: required from ‘void boost::unknown_exception::add_original_type(const E&) [with E = std::exception]’ t/boost-trunk/boost/exception/detail/exception_ptr.hpp:161:32: required from here t/boost-trunk/boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type ‘boost::error_info<boost::tag_original_exception_type, const std::type_info*>’ which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor] -- Olaf
From: Of Olaf van der Spek
On Sun, Apr 15, 2012 at 10:54 AM, Andrey Semashev <andrey.semashev@gmail.com> wrote:
Using a separate implementation of checked_delete in shared_ptr is what I had in mind. And this implementation should not be public, I think.
Isn't the type guaranteed to be complete when shared_ptr calls delete?
No. Let's take Z derived from Y derived from X. All three have non-virtual deconstructor: shared_ptr<Y>(new Y); // safe since shared_ptr<X>(new Y); // safe since type Y is passed eventually to checked_delete shared_ptr<X>(static_cast<Y*>( new Z )); // no way it's safe - calls only ~Y() Seriously the guy could just put in a "virtual ". It costs almost nothing and would be way less painful then a bunch of compiler specific warning supression. Heaping things is way more expensive than a virtual call. My vote is for A) adding "virtual". If for some reason he's got a problem with it he can do B) define error_info final so gcc shuts up. There's a reason they added it to the standard. I would care if this were in some cpp, but it's in a header included indirectly by serveral of my compilation scopes. Actually I don't care as much about this as my issues with mpl/fusion, but it just seems trivial to fix. Chris
On 14/04/12 23:13, Daniel James wrote:
In this case the warning fires because it's triggered by the delete command, which is not covered by the system header pragma in the exception headers. This is different to the old warning which was triggered by the destructor. This might actually be a problem for other libraries as well. 'checked_delete' is used by 'shared_ptr' and is supposed to be safe for the case when the destructor isn't virtual, so this might result in warnings for valid uses of 'shared_ptr'. So, for g++ 4.7 and up, the correct solution *might* be to use the new pragmas to disable the warning in checked_delete. But it might not, I'm not familiar enough with checked_delete to know if that would be appropriate.
Calling delete on a pointer to a polymorphic type with a non-virtual destructor might be a bug. It would be a bad idea for shared_ptr to disable this warning for any other type than error_info. The simplest solution is to make the destructor of error_info virtual, since it doesn't add any significant overhead, especially compared to how bloated Boost.Exception is as a whole... It would also have the advantage of not relying on fragile compiler-specific warning suppression mechanisms. As a rule, rewriting the code so that the warning doesn't happen is much more valuable than disabling the warning.
On 15 April 2012 13:53, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
Calling delete on a pointer to a polymorphic type with a non-virtual destructor might be a bug. It would be a bad idea for shared_ptr to disable this warning for any other type than error_info.
But shared_ptr is explicitly written to deal with this case so the warning is almost always incorrect (remember that if you delete the object by some other means, the warning will appear for the other deletion). You can see this tested in shared_ptr_test. Look at the contents of n_spt_abstract (in a couple of places). Note also that gcc's std::shared_ptr does not warn for this use case.
The simplest solution is to make the destructor of error_info virtual, since it doesn't add any significant overhead, especially compared to how bloated Boost.Exception is as a whole...
This isn't just about Boost.Exception. I'm not arguing against it making its destructor virtual.
It would also have the advantage of not relying on fragile compiler-specific warning suppression mechanisms.
Since this warning was added in 4.7, the warning mechanism introduced in 4.6 can be used. It isn't fragile.
As a rule, rewriting the code so that the warning doesn't happen is much more valuable than disabling the warning.
Sometimes that's that case, but sometimes the opposite is true. Rewriting code can cause unnecessary complexity and introduce bugs. Especially when you consider the multitude of warnings we have to deal with.
On 15/04/12 21:56, Daniel James wrote:
On 15 April 2012 13:53, Mathias Gaunard<mathias.gaunard@ens-lyon.org> wrote:
Calling delete on a pointer to a polymorphic type with a non-virtual destructor might be a bug. It would be a bad idea for shared_ptr to disable this warning for any other type than error_info.
But shared_ptr is explicitly written to deal with this case so the warning is almost always incorrect (remember that if you delete the object by some other means, the warning will appear for the other deletion). You can see this tested in shared_ptr_test. Look at the contents of n_spt_abstract (in a couple of places). Note also that gcc's std::shared_ptr does not warn for this use case.
I don't see what this has to do with abstract classes. struct A { virtual void foo() {} }; struct B : A {}; boost::shared_ptr<A> p(new B); should issue that warning.
The simplest solution is to make the destructor of error_info virtual, since it doesn't add any significant overhead, especially compared to how bloated Boost.Exception is as a whole...
This isn't just about Boost.Exception. I'm not arguing against it making its destructor virtual.
All the warnings being reported are about error_info not having a virtual destructor; I don't see anything specific to shared_ptr here.
It would also have the advantage of not relying on fragile compiler-specific warning suppression mechanisms.
Since this warning was added in 4.7, the warning mechanism introduced in 4.6 can be used. It isn't fragile.
First, it's specific to that compiler. What if tomorrow a new compiler decides to add that kind of warning too? What if it occurs in slightly different scenarios? Also it's not clear how warnings are suppressed with those mechanisms, especially when templates are involved. See, we have something that seems to depend on include order here. That's the worst possible behaviour. It looks like if I include boost/exception_ptr.hpp first, then all possible warning messages (which *are* useful) would be suppressed from the boost/shared_ptr.hpp header.
Sometimes that's that case, but sometimes the opposite is true. Rewriting code can cause unnecessary complexity and introduce bugs. Especially when you consider the multitude of warnings we have to deal with.
I've made codebases of several hundreds of thousands of lines of code warning-free on a variety of different compilers, yet I cannot say I share your experience. On the contrary, it tends to remove bugs because the compiler does more strict checking than a human. The only added complexity is when you need to use compiler-specific workarounds, which is only required for MSVC deprecated/unsafe functions.
On Mon, Apr 16, 2012 at 11:20 AM, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
I don't see what this has to do with abstract classes.
struct A { virtual void foo() {} }; struct B : A {};
boost::shared_ptr<A> p(new B);
should issue that warning.
Right, but if make_shared<B> is used, it should not. Olaf
On 16 April 2012 10:20, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
I don't see what this has to do with abstract classes.
struct A { virtual void foo() {} }; struct B : A {};
boost::shared_ptr<A> p(new B);
should issue that warning.
If you bother to try that, you'll find that shared_ptr will correctly call B's destructor, so it shouldn't issue a warning. It also demonstrates why you can't rely on the warning, since it will disappear with the slightest of changes.
This isn't just about Boost.Exception. I'm not arguing against it making its destructor virtual.
All the warnings being reported are about error_info not having a virtual destructor; I don't see anything specific to shared_ptr here.
The warning turns up in shared_ptr_test. I think I mentioned that before.
First, it's specific to that compiler. What if tomorrow a new compiler decides to add that kind of warning too?
You really can't program for future compiler warnings.
What if it occurs in slightly different scenarios?
Explain please.
Also it's not clear how warnings are suppressed with those mechanisms, especially when templates are involved.
They're not clear to you because you never use them.
See, we have something that seems to depend on include order here. That's the worst possible behaviour.
That's the system header pragma, which, as I previously said, is problematic. I claimed that the warning mechanism introduced in 4.6 is not fragile. Please stop confusing the two.
It looks like if I include boost/exception_ptr.hpp first, then all possible warning messages (which *are* useful) would be suppressed from the boost/shared_ptr.hpp header.
That's a problem in exception, not with the change we're discussing. It probably should move its warning pragmas to after it includes other headers.
I've made codebases of several hundreds of thousands of lines of code warning-free on a variety of different compilers, yet I cannot say I share your experience.
Well done, that's great, but different development techniques are available. Much of boost is pretty low level, so we often include code that would be problematic in higher level development. shared_ptr deals with deleting objects so that we don't have to. We do need to put thought into disabling warnings, and not just do it everywhere. But a dogmatic insistence on the correctness of all warnings is almost as bad as dismissing them out of hand.
On 16/04/12 15:29, Daniel James wrote:
On 16 April 2012 10:20, Mathias Gaunard<mathias.gaunard@ens-lyon.org> wrote:
I don't see what this has to do with abstract classes.
struct A { virtual void foo() {} }; struct B : A {};
boost::shared_ptr<A> p(new B);
should issue that warning.
If you bother to try that, you'll find that shared_ptr will correctly call B's destructor, so it shouldn't issue a warning.
I didn't know that shared_ptr called the destructor of the type it was given in the constructor. That seems like a bit of a dangerous design. So the above will work, but not A* p0 = new B; boost::shared_ptr<A> p(p0); I think silencing the warning might be the best solution in this case.
You really can't program for future compiler warnings.
Compiler warnings output messages that have been engineered by people to find potential problems in source code. If you code in a way that a human wouldn't see a potential problem in your code, then a compiler also will not.
What if it occurs in slightly different scenarios?
Explain please.
In certain cases, the potential problem of a non-virtual destructor generates a warning when the declaration of the destructor is encountered. In other cases, the warning gets generated when a call to the destructor is made. In yet other cases, only if the call to the destructor is made through the delete operator. Fixing the source of the problem is easier than adding kludges in every place the problem could end up being diagnosed.
Also it's not clear how warnings are suppressed with those mechanisms, especially when templates are involved.
They're not clear to you because you never use them.
I've used the MSVC pragma push/pop warning disable thing pretty extensively, and I've had quite a few problems with it when templates are concerned. I suppose GCC, while essentially using the same syntax, does it better?
Well done, that's great, but different development techniques are available. Much of boost is pretty low level, so we often include code that would be problematic in higher level development. shared_ptr deals with deleting objects so that we don't have to.
Boost isn't particularly low-level. On the contrary, it is very high level compared to average C or C++ development.
Mathias Gaunard wrote:
On 16/04/12 15:29, Daniel James wrote: In certain cases, the potential problem of a non-virtual destructor generates a warning when the declaration of the destructor is encountered. In other cases, the warning gets generated when a call to the destructor is made. In yet other cases, only if the call to the destructor is made through the delete operator. I think they've gotten better at this.
If you put something like this on the stack you know the correct deconstructor is called and the Virtual table isn't even used so there's no reason to complain. I could put a class on the stack and call someone else with an interface to it. If they don't try to delete it good. No reason to complain. Deleting a pointer to a final class is also fine. You're really hard pressed to come up with situations where you would care about an extra virtual table entry and an extra indirection while deleting. With C++11 you're even harder pressed to want to delete a non-final class which has virtual methods but no virtual deconstructor.
Fixing the source of the problem is easier than adding kludges in every place the problem could end up being diagnosed.
I would add the deconstructor to someone's code unless he had a comment telling me exactly why not. Chris
On Mon, Apr 16, 2012 at 9:14 AM, Hite, Christopher < Christopher.Hite@partner.commerzbank.com> wrote:
Mathias Gaunard wrote:
On 16/04/12 15:29, Daniel James wrote: In certain cases, the potential problem of a non-virtual destructor generates a warning when the declaration of the destructor is encountered. In other cases, the warning gets generated when a call to the destructor is made. In yet other cases, only if the call to the destructor is made through the delete operator. I think they've gotten better at this.
You're really hard pressed to come up with situations where you would care about an extra virtual table entry and an extra indirection while deleting.
The reason to care isn't performance. I tend to take the virtual keyword seriously when I read code: it is an invitation to call the function through a base type pointer. In this case it would be a bug if the destructor is ever called virtually. You'll know your compiler is wrong when it tries to "protect" you from conscious and correct design decisions. That said, the warning is annoying and I want it gone. I have not been able to reproduce it. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On Monday 16 April 2012 12:12:21 Emil Dotchevski wrote:
You're really hard pressed to come up with situations where you would care about an extra virtual table entry and an extra indirection while deleting.
The reason to care isn't performance. I tend to take the virtual keyword seriously when I read code: it is an invitation to call the function through a base type pointer. In this case it would be a bug if the destructor is ever called virtually. You'll know your compiler is wrong when it tries to "protect" you from conscious and correct design decisions.
IMHO, you are taking it too seriously. If you want to disable the call to the destructor, you typically declare it private or protected. Whether it is virtual or not is another story and irrelevant to the access restriction (which is actually what you're trying to impose). And declaring it virtual doesn't make the code uglier. In fact, compared to all the machinery required to disable the warning for every compiler, it is a cleaner solution. PS: Don't get me wrong, I too am annoyed by spurious warnings sometimes and I do not support the "no-warnings" policies in general. However, I tend to follow the "minimal code ugliness" criteria in making the decision whether to disable the warning or adjust the code.
On Mon, Apr 16, 2012 at 1:51 PM, Andrey Semashev <andrey.semashev@gmail.com>wrote:
On Monday 16 April 2012 12:12:21 Emil Dotchevski wrote:
You're really hard pressed to come up with situations where you would
care
about an extra virtual table entry and an extra indirection while deleting.
The reason to care isn't performance. I tend to take the virtual keyword seriously when I read code: it is an invitation to call the function through a base type pointer. In this case it would be a bug if the destructor is ever called virtually. You'll know your compiler is wrong when it tries to "protect" you from conscious and correct design decisions.
IMHO, you are taking it too seriously. If you want to disable the call to the destructor, you typically declare it private or protected. Whether it is virtual or not is another story and irrelevant to the access restriction (which is actually what you're trying to impose).
The meaning of virtual is "this function is designed to be called through a base type pointer". This is simply not true for this code, it would be misleading and incorrect to use virtual in this case.
And declaring it virtual doesn't make the code uglier. In fact, compared to all the machinery required to disable the warning for every compiler, it is a cleaner solution.
The "machinery" is needed anyway to disable all other warnings, and it is much cleaner than dealing with each individual warning emitted by the many compilers Boost is used with. Besides, if a warning (any warning) escapes the "machinery", this is a bug that I want to fix. In this particular case, people say they get the warning but nobody has given me exact code that produces it. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
The meaning of virtual is "this function is designed to be called through a base type pointer". This is simply not true for this code, it would be misleading and incorrect to use virtual in this case.
Yes but a protected destructor can't be called, and making it virtual gets rid of the warning. Some battles are just not worth fighting.
On Monday 16 April 2012 15:32:06 Emil Dotchevski wrote:
On Mon, Apr 16, 2012 at 1:51 PM, Andrey Semashev <andrey.semashev@gmail.com>wrote:
IMHO, you are taking it too seriously. If you want to disable the call to the destructor, you typically declare it private or protected. Whether it is virtual or not is another story and irrelevant to the access restriction (which is actually what you're trying to impose).
The meaning of virtual is "this function is designed to be called through a base type pointer". This is simply not true for this code, it would be misleading and incorrect to use virtual in this case.
It doesn't matter if you can't call it anyway since it's protected.
And declaring it virtual doesn't make the code uglier. In fact, compared to all the machinery required to disable the warning for every compiler, it is a cleaner solution.
The "machinery" is needed anyway to disable all other warnings, and it is much cleaner than dealing with each individual warning emitted by the many compilers Boost is used with.
Besides, if a warning (any warning) escapes the "machinery", this is a bug that I want to fix. In this particular case, people say they get the warning but nobody has given me exact code that produces it.
The current machinery is applied header-wise and doesn't really clutter the code. I supposed this particular warning would be disabled for a particular class or expression (because disabling it globally is excessive).
Emil Dotchevski wrote:
On Mon, Apr 16, 2012 at 9:14 AM, Hite, Christopher < Christopher.Hite@partner.commerzbank.com> wrote:
You're really hard pressed to come up with situations where you would care about an extra virtual table entry and an extra indirection while deleting.
The reason to care isn't performance. I tend to take the virtual keyword seriously when I read code: it is an invitation to call the function through a base type pointer. In this case it would be a bug if the destructor is ever called virtually. You'll know your compiler is wrong when it tries to "protect" you from conscious and correct design decisions.
Well you did make the dtor protected. That's a pretty strong signal to anyone reading the code that the interface is doesn't include ownership. Adding a virtual dtor to the child class should aslo make the warning go away too.
That said, the warning is annoying and I want it gone. I have not been able to reproduce it.
I was able to do the same test as others gcc-4.7.0 Linux. g++ -c -Wall -I$BOOST_ROOT boost_exceptionwarning.cpp /fs/tools/L4/boost_1_49_0/boost/checked_delete.hpp:34:5: warning: deleting object of polymorphic class type 'boost::error_info<boost::tag_original_exception_type, const std::type_info*>' which has non-virtual destructor might cause undefined behaviour [-Wdelete-non-virtual-dtor] Chris
On Tue, Apr 17, 2012 at 4:41 AM, Hite, Christopher < Christopher.Hite@partner.commerzbank.com> wrote:
I was able to do the same test as others gcc-4.7.0 Linux. g++ -c -Wall -I$BOOST_ROOT boost_exceptionwarning.cpp
Can I see the exact content of boost_exceptionwarning.cpp? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
participants (12)
-
Andrey Semashev -
Daniel James -
Emil Dotchevski -
Hite, Christopher -
Jeffrey Lee Hellrung, Jr. -
Jeremiah Willcock -
Joshua Boyce -
Mathias Gaunard -
Nathan Ridge -
Olaf van der Spek -
Peter Dimov -
Vicente J. Botet Escriba