Fix: activate virtual destructors for all msvc versions

[jhunold]

All msvc version spew out warnings so make the destructors virtual for all.

[rogeeff]

Why do we need this?

On Sun, Mar 8, 2015 at 12:45 PM, Jürgen Hunold notifications@github.com
wrote:

All msvc version spew out warnings so make the destructors virtual for all.

You can view, comment on, or merge this pull request online at:

#41
Commit Summary

• Fix: activate virtual destructors for all msvc versions

File Changes

• M include/boost/test/detail/config.hpp
  https://github.com/boostorg/test/pull/41/files#diff-0 (2)

Patch Links:

• https://github.com/boostorg/test/pull/41.patch
• https://github.com/boostorg/test/pull/41.diff

—
Reply to this email directly or view it on GitHub
#41.

Gennadiy Rozental

[jhunold]

msvc will issue warnings about the destructors being not virtual. There is no need to make this dependent on the compiler version as all version complain.

Another solution would be to disable this via #pragma . Or just make the destructor virtual by default, as gcc and clang are already doing this.

[rogeeff]

Can we file a bug report with MSVC compiler?

On Mon, Mar 9, 2015 at 4:33 AM, Jürgen Hunold notifications@github.com
wrote:

msvc will issue warnings about the destructors being not virtual. There is
no need to make this dependent on the compiler version as all version
complain.

Another solution would be to disable this via #pragma . Or just make the
destructor virtual by default, as gcc and clang are already doing this.

—
Reply to this email directly or view it on GitHub
#41 (comment).

Gennadiy Rozental

[raffienficiaud]

FYI, I created the ticket https://svn.boost.org/trac/boost/ticket/11107. It looks to me that non-virtual is a bug, but I do not understand the first intention of this workaround. Any hint? (some more details in the ticket)

[rogeeff]

destructor of base class should be either virtual or protected. I chose
second option and its perfectly valid option - no need for any warnings

On Wed, Mar 11, 2015 at 6:19 PM, Raffi Enficiaud notifications@github.com
wrote:

FYI, I created the ticket https://svn.boost.org/trac/boost/ticket/11107.
It looks to me that non-virtual is a bug, but I do not understand the first
intention of this workaround. Any hint? (some more details in the ticket)

—
Reply to this email directly or view it on GitHub
#41 (comment).

Gennadiy Rozental

[raffienficiaud]

You're right. However, protected + virtual does not harm neither? On the other hand, I cannot see any warning with b2 --with-test. @jhunold would you please attach the logs to the ticket please? We would then better understand what we are discussing.

[jhunold]

Right, added full log to the ticket. I've just made the warning an error to just clarify things. @rogeeff All major compilers warn about this (gcc,clang and msvc) so I doubt they are all wrong. I've only found
http://www.gotw.ca/publications/mill18.htm stating:

Guideline #4: A base class destructor should be either public and virtual, or protected and nonvirtual.

True, the standard library itself does not always follow these design criteria. In part, that's a reflection of how we as a community have learned over the years.

So that is a guideline from 14 years ago. I*d really like to know if this is still valid or the exact paragraph of the standard handling this.

[raffienficiaud]

Hi,

Would you please try the develop branch again for this warning? The log you sent were not on the protected part, see commit 653aae5

[jhunold]

Uploaded a new log to the ticket. I had the patch active when doing the test run. I really should create the working branch before committing.

[raffienficiaud]

Superseded/addressed by 13ca3e0

[raffienficiaud]

Merged to master