Bug 155048 – [CMake] Enable -Wnon-virtual-dtor and fix violations

• Component: WebKit Misc.
• Status: NEW

[Michael Catanzaro]

We should enable -Wnon-virtual-dtor in WEBKIT_SET_EXTRA_COMPILER_FLAGS. It would have caught a crash we introduced on the GTK port recently. Currently we get -Wdelete-non-virtual-dtor via -Wall, but this warning is not as effective.

But before doing so, we should make sure to fix whatever warnings pop up.

[Michael Catanzaro]

For the record:

       -Wdelete-non-virtual-dtor (C++ and Objective-C++ only)
           Warn when "delete" is used to destroy an instance of a class that
           has virtual functions and non-virtual destructor. It is unsafe to
           delete an instance of a derived class through a pointer to a base
           class if the base class does not have a virtual destructor.  This
           warning is enabled by -Wall.

       -Wnon-virtual-dtor (C++ and Objective-C++ only)
           Warn when a class has virtual functions and an accessible non-
           virtual destructor itself or in an accessible polymorphic base
           class, in which case it is possible but unsafe to delete an
           instance of a derived class through a pointer to the class itself
           or base class.  This warning is automatically enabled if -Weffc++
           is specified.

Note that we currently use -Wall, but not spammy and nonstandard -Weffc++ (if you haven't heard of -Weffc++, it enables warnings about things Scott Meyers tells us not to do :)

[Darin Adler]

I believe we also want this warning turned on for Mac and iOS, so separately we should make sure it’s turned on in the .xcconfig files we currently use on those platforms, if it’s not already.

(In reply to comment #2)
> I believe we also want this warning turned on for Mac and iOS, so separately
> we should make sure it’s turned on in the .xcconfig files we currently use
> on those platforms, if it’s not already.

GCC_WARN_NON_VIRTUAL_DESTRUCTOR is already set to YES in every Base.xcconfig in Source except for ANGLE’s.

[Michael Catanzaro]

(In reply to comment #3)
> GCC_WARN_NON_VIRTUAL_DESTRUCTOR is already set to YES in every Base.xcconfig
> in Source except for ANGLE’s.

Good. That explains why none of these bugs affect Apple ports:

../../Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:34:11: warning: 'WebCore::TextureMapperPlatformLayer::Client' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
    class Client {
          ^

../../Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:44:11: warning: 'WebCore::TextureMapperLayer::ScrollingClient' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
    class ScrollingClient {
          ^

../../Source/WebCore/platform/graphics/texmap/TextureMapperAnimation.h:33:11: warning: 'WebCore::TextureMapperAnimation::Client' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
    class Client {
          ^

../../Source/WebCore/platform/geoclue/GeolocationProviderGeoclueClient.h:27:7: warning: 'WebCore::GeolocationProviderGeoclueClient' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
class GeolocationProviderGeoclueClient {
      ^

../../Source/WebCore/platform/network/soup/WebKitSoupRequestGenericClient.h:29:7: warning: 'WebCore::WebKitSoupRequestGenericClient' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
class WebKitSoupRequestGenericClient {
      ^

../../Source/WebKit2/NetworkProcess/CustomProtocols/soup/CustomProtocolManagerImpl.h:46:5: warning: 'WebKit::CustomProtocolManagerImpl' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
    ~CustomProtocolManagerImpl();
    ^

[Michael Catanzaro]

(In reply to comment #4)
> Good. That explains why none of these bugs affect Apple ports:

I looked at these and none seem to be actual bugs, as these classes are never deleted polymorphically, but it's still a very bad idea to have public nonvirtual destructors.

[Michael Catanzaro]

Patch is slow in coming as I'm trying to figure out a nice way to avoid passing this flag when compiling C files, as that causes GCC to print a warning.