search

boostorg / exception

Boost.org exception module
http://boost.org/libs/exception
Boost Software License 1.0
15 stars 46 forks source link

exception.hpp: warning C4265 given by Visual Studio #23

Closed bersbersbers closed 4 years ago

bersbersbers commented 5 years ago

This has been reported before (https://svn.boost.org/trac10/ticket/13227), but I am not sure that tracker is still active. The problem is still relevant with boost 1.65.1 and 1.70.

Quoting:

MSVC falsely report a missing virtual destructor inside exception.hpp Would it be possible to add a "pragma warning (disable:4265)" in this file ? This will not happen with the default settings of Visual Studio : it happens when trying to raise the warning level (/we4265 : treat "misssing virtual destructor" as an error)

The above link has reproduction steps, in particular, https://github.com/ivsgroup/boost_warnings_minimal_demo

zajo commented 4 years ago

Sorry I had missed this. See https://github.com/boostorg/throw_exception/commit/43a57d518cf99fc693eebedefcbaa91074674f54

Shawn1874 commented 2 years ago

I know this is closed, but the warning / error doesn't appear to be a false report. Instead of disabling the warning, shouldn't the destructor have been made virtual?

zajo commented 2 years ago

It is incorrect for this destructor to be virtual. The destructor is never called via a pointer to the base type.

Shawn1874 commented 2 years ago

As a general rule classes with virtual methods ought to have a virtual destructor which is the reason for the warning. It certainly wouldn't hurt anything for the the destructor to be virtual. There is nothing incorrect about a virtual, protected method. It seems like it was more of a preference for someone to disable the warning.

zajo commented 2 years ago

It is a deliberate design decision. Marking a function virtual indicates dynamic polymorphism. In this design, calling the destructor polymorphically would be a logic error. In C++, this is communicated by declaring the destructor protected and non-virtual.