diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-06-08 02:20:21 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-06-08 02:20:21 +0000 |
commit | 35343fe733e22eb1a7fcb99a9d077d0b668c28ea (patch) | |
tree | 8c5b9b9e9eab87f6ebe7cd3a1aa084ad8ba4e0cb | |
parent | 141a1df417a450411cf2aa7928295eecf0aa1225 (diff) | |
download | chromium_src-35343fe733e22eb1a7fcb99a9d077d0b668c28ea.zip chromium_src-35343fe733e22eb1a7fcb99a9d077d0b668c28ea.tar.gz chromium_src-35343fe733e22eb1a7fcb99a9d077d0b668c28ea.tar.bz2 |
roll clang 131935:132017
clang recently added a warning that warns when invoking 'delete' on a polymorphic non-final class without a virtual destructor. This finds real bugs – see the bug referenced below for a few examples.
However, one common pattern where it fires is this case:
class SomeInterface {
public:
virtual void interfaceMethod() {} // or = 0;
protected:
~SomeInterface() {}
}
class WorkerClass : public SomeInterface {
public:
// many non-virtual functions, but also:
virtual void interfaceMethod() override { /* do actual work */ }
};
void f() {
scoped_ptr<WorkerClass> c(new WorkerClass); // simplified example
}
(See the 2nd half of http://www.gotw.ca/publications/mill18.htm for an explanation of this pattern.)
It is arguably correct to fire the warning here, since someone might make a subclass of WorkerClass and replace |new WorkerClass| with |new WorkerClassSubclass|. This would be broken since WorkerClass doesn't have a virtual destructor.
The solution that the clang folks recommend is to mark WorkerClass as |final| (a c++0x keyword that clang supports as an extension in normal c++ mode – like override). But chrome's base/OWNERS deemed that as too complicated and we decided to make virtual the destructors of leaf classes that implement these interfaces and that are deleted dynamically. All of the changes in this CL are to shut up the warning, not because of real problems (I fixed these in separate CLs).
(For the gtk files, this is necessary because the CHROMEGTK_CALLBACK_ macros add virtual functions.)
BUG=84424
TEST=none
Review URL: http://codereview.chromium.org/7087028
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@88270 0039d316-1c4b-4281-b951-d872f2087c98
32 files changed, 58 insertions, 23 deletions
diff --git a/base/message_loop_unittest.cc b/base/message_loop_unittest.cc index be40f39..646619d 100644 --- a/base/message_loop_unittest.cc +++ b/base/message_loop_unittest.cc @@ -428,7 +428,7 @@ class RecordDeletionProbe : public base::RefCounted<RecordDeletionProbe> { FROM_HERE, base::Bind(&RecordDeletionProbe::Run, post_on_delete_.get())); } - virtual void Run() {} + void Run() {} private: scoped_refptr<RecordDeletionProbe> post_on_delete_; bool* was_deleted_; diff --git a/build/common.gypi b/build/common.gypi index 0b78860..1f87a50 100644 --- a/build/common.gypi +++ b/build/common.gypi @@ -1356,6 +1356,17 @@ ], }]], }], + ['clang==1 and chromeos==1', { + 'target_conditions': [ + ['_toolset=="target"', { + 'cflags': [ + # TODO(thakis): Remove this once all instances of this + # are fixed in the views and chromeos code. + # http://crbug.com/84424 + '-Wno-delete-non-virtual-dtor', + ], + }]], + }], ['clang==1 and clang_use_chrome_plugins==1', { 'target_conditions': [ ['_toolset=="target"', { diff --git a/chrome/browser/download/base_file.h b/chrome/browser/download/base_file.h index f9a5c28..02563b7 100644 --- a/chrome/browser/download/base_file.h +++ b/chrome/browser/download/base_file.h @@ -30,7 +30,7 @@ class BaseFile { const GURL& referrer_url, int64 received_bytes, const linked_ptr<net::FileStream>& file_stream); - ~BaseFile(); + virtual ~BaseFile(); // If calculate_hash is true, sha256 hash will be calculated. bool Initialize(bool calculate_hash); diff --git a/chrome/browser/download/save_file.h b/chrome/browser/download/save_file.h index a5c19eb..fc610f7 100644 --- a/chrome/browser/download/save_file.h +++ b/chrome/browser/download/save_file.h @@ -22,7 +22,7 @@ class SaveFile : public BaseFile { public: explicit SaveFile(const SaveFileCreateInfo* info); - ~SaveFile(); + virtual ~SaveFile(); // Accessors. int save_id() const { return info_->save_id; } diff --git a/chrome/browser/ui/gtk/crypto_module_password_dialog.cc b/chrome/browser/ui/gtk/crypto_module_password_dialog.cc index 31f07ce..7713b2f 100644 --- a/chrome/browser/ui/gtk/crypto_module_password_dialog.cc +++ b/chrome/browser/ui/gtk/crypto_module_password_dialog.cc @@ -86,6 +86,8 @@ class CryptoModulePasswordDialog { const std::string& server, browser::CryptoModulePasswordCallback* callback); + virtual ~CryptoModulePasswordDialog() {} + void Show(); private: diff --git a/chrome/browser/ui/gtk/external_protocol_dialog_gtk.h b/chrome/browser/ui/gtk/external_protocol_dialog_gtk.h index 966d216..6dc7365 100644 --- a/chrome/browser/ui/gtk/external_protocol_dialog_gtk.h +++ b/chrome/browser/ui/gtk/external_protocol_dialog_gtk.h @@ -16,7 +16,7 @@ typedef struct _GtkWidget GtkWidget; class ExternalProtocolDialogGtk { public: explicit ExternalProtocolDialogGtk(const GURL& url); - ~ExternalProtocolDialogGtk(); + virtual ~ExternalProtocolDialogGtk(); private: CHROMEGTK_CALLBACK_1(ExternalProtocolDialogGtk, void, OnResponse, int); diff --git a/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc b/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc index 2821e4f..3ec6ed9 100644 --- a/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc +++ b/chrome/browser/ui/gtk/hung_renderer_dialog_gtk.cc @@ -32,6 +32,7 @@ namespace { class HungRendererDialogGtk { public: HungRendererDialogGtk(); + virtual ~HungRendererDialogGtk() {} void ShowForTabContents(TabContents* hung_contents); void EndForTabContents(TabContents* hung_contents); diff --git a/chrome/browser/ui/gtk/importer/import_lock_dialog_gtk.h b/chrome/browser/ui/gtk/importer/import_lock_dialog_gtk.h index 23c50b8..99c6592 100644 --- a/chrome/browser/ui/gtk/importer/import_lock_dialog_gtk.h +++ b/chrome/browser/ui/gtk/importer/import_lock_dialog_gtk.h @@ -23,7 +23,7 @@ class ImportLockDialogGtk { private: ImportLockDialogGtk(GtkWindow* parent, ImporterHost* importer_host); - ~ImportLockDialogGtk(); + virtual ~ImportLockDialogGtk(); CHROMEGTK_CALLBACK_1(ImportLockDialogGtk, void, OnResponse, int); diff --git a/chrome/browser/ui/gtk/instant_confirm_dialog_gtk.h b/chrome/browser/ui/gtk/instant_confirm_dialog_gtk.h index 98d427f..637c002 100644 --- a/chrome/browser/ui/gtk/instant_confirm_dialog_gtk.h +++ b/chrome/browser/ui/gtk/instant_confirm_dialog_gtk.h @@ -19,7 +19,7 @@ typedef struct _GtkWindow GtkWindow; class InstantConfirmDialogGtk { public: InstantConfirmDialogGtk(GtkWindow* parent, Profile* profile); - ~InstantConfirmDialogGtk(); + virtual ~InstantConfirmDialogGtk(); private: CHROMEGTK_CALLBACK_1(InstantConfirmDialogGtk, void, OnResponse, int); diff --git a/chrome/browser/ui/gtk/menu_gtk.h b/chrome/browser/ui/gtk/menu_gtk.h index ecfd364..79a59ff 100644 --- a/chrome/browser/ui/gtk/menu_gtk.h +++ b/chrome/browser/ui/gtk/menu_gtk.h @@ -55,7 +55,7 @@ class MenuGtk { }; MenuGtk(MenuGtk::Delegate* delegate, ui::MenuModel* model); - ~MenuGtk(); + virtual ~MenuGtk(); // Initialize GTK signal handlers. void ConnectSignalHandlers(); diff --git a/chrome/browser/ui/gtk/update_recommended_dialog.h b/chrome/browser/ui/gtk/update_recommended_dialog.h index 8c4955a..2c24a7e 100644 --- a/chrome/browser/ui/gtk/update_recommended_dialog.h +++ b/chrome/browser/ui/gtk/update_recommended_dialog.h @@ -19,7 +19,7 @@ class UpdateRecommendedDialog { private: explicit UpdateRecommendedDialog(GtkWindow* parent); - ~UpdateRecommendedDialog(); + virtual ~UpdateRecommendedDialog(); CHROMEGTK_CALLBACK_1(UpdateRecommendedDialog, void, OnResponse, int); diff --git a/chrome/renderer/chrome_content_renderer_client.h b/chrome/renderer/chrome_content_renderer_client.h index 6e22973..794c9e6 100644 --- a/chrome/renderer/chrome_content_renderer_client.h +++ b/chrome/renderer/chrome_content_renderer_client.h @@ -32,7 +32,7 @@ namespace chrome { class ChromeContentRendererClient : public content::ContentRendererClient { public: ChromeContentRendererClient(); - ~ChromeContentRendererClient(); + virtual ~ChromeContentRendererClient(); virtual void RenderThreadStarted() OVERRIDE; virtual void RenderViewCreated(RenderView* render_view) OVERRIDE; diff --git a/content/browser/device_orientation/provider_unittest.cc b/content/browser/device_orientation/provider_unittest.cc index facfc0c..d991d5a 100644 --- a/content/browser/device_orientation/provider_unittest.cc +++ b/content/browser/device_orientation/provider_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -23,6 +23,8 @@ class UpdateChecker : public Provider::Observer { : expectations_count_ptr_(expectations_count_ptr) { } + virtual ~UpdateChecker() {} + // From Provider::Observer. virtual void OnOrientationUpdate(const Orientation& orientation) { ASSERT_FALSE(expectations_queue_.empty()); diff --git a/content/browser/renderer_host/render_message_filter.cc b/content/browser/renderer_host/render_message_filter.cc index 42806c0e..a20698a 100644 --- a/content/browser/renderer_host/render_message_filter.cc +++ b/content/browser/renderer_host/render_message_filter.cc @@ -196,6 +196,8 @@ class OpenChannelToPpapiBrokerCallback : public PpapiBrokerProcessHost::Client { request_id_(request_id) { } + virtual ~OpenChannelToPpapiBrokerCallback() {} + virtual void GetChannelInfo(base::ProcessHandle* renderer_handle, int* renderer_id) { *renderer_handle = filter_->peer_handle(); diff --git a/content/ppapi_plugin/ppapi_webkitclient_impl.cc b/content/ppapi_plugin/ppapi_webkitclient_impl.cc index 7908e0c..d423776 100644 --- a/content/ppapi_plugin/ppapi_webkitclient_impl.cc +++ b/content/ppapi_plugin/ppapi_webkitclient_impl.cc @@ -28,6 +28,8 @@ using WebKit::WebUChar; class PpapiWebKitClientImpl::SandboxSupport : public WebSandboxSupport { public: + virtual ~SandboxSupport() {} + #if defined(OS_WIN) virtual bool ensureFontLoaded(HFONT); #elif defined(OS_MACOSX) diff --git a/content/renderer/content_renderer_client.h b/content/renderer/content_renderer_client.h index 19d7ebd..5c87bc6 100644 --- a/content/renderer/content_renderer_client.h +++ b/content/renderer/content_renderer_client.h @@ -30,6 +30,8 @@ namespace content { // Embedder API for participating in renderer logic. class ContentRendererClient { public: + virtual ~ContentRendererClient() {} + // Notifies us that the RenderThread has been created. virtual void RenderThreadStarted(); diff --git a/content/renderer/external_popup_menu.h b/content/renderer/external_popup_menu.h index ef83d071..b3eebe8 100644 --- a/content/renderer/external_popup_menu.h +++ b/content/renderer/external_popup_menu.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -20,6 +20,8 @@ class ExternalPopupMenu : public WebKit::WebExternalPopupMenu { const WebKit::WebPopupMenuInfo& popup_menu_info, WebKit::WebExternalPopupMenuClient* popup_menu_client); + virtual ~ExternalPopupMenu() {} + // Called when the user has selected an item. |selected_item| is -1 if the // user canceled the popup. void DidSelectItem(int selected_index); diff --git a/content/renderer/render_widget_fullscreen_pepper.cc b/content/renderer/render_widget_fullscreen_pepper.cc index b078f24..eace6b0 100644 --- a/content/renderer/render_widget_fullscreen_pepper.cc +++ b/content/renderer/render_widget_fullscreen_pepper.cc @@ -43,6 +43,8 @@ class PepperWidget : public WebWidget { cursor_(WebCursorInfo::TypePointer) { } + virtual ~PepperWidget() {} + // WebWidget API virtual void close() { delete this; diff --git a/content/renderer/renderer_webkitclient_impl.cc b/content/renderer/renderer_webkitclient_impl.cc index 55db5f7..86622d0 100644 --- a/content/renderer/renderer_webkitclient_impl.cc +++ b/content/renderer/renderer_webkitclient_impl.cc @@ -110,6 +110,8 @@ class RendererWebKitClientImpl::FileUtilities class RendererWebKitClientImpl::SandboxSupport : public WebKit::WebSandboxSupport { public: + virtual ~SandboxSupport() {} + #if defined(OS_WIN) virtual bool ensureFontLoaded(HFONT); #elif defined(OS_MACOSX) diff --git a/ppapi/proxy/plugin_resource_tracker.h b/ppapi/proxy/plugin_resource_tracker.h index c00d3f2..0a18dff 100644 --- a/ppapi/proxy/plugin_resource_tracker.h +++ b/ppapi/proxy/plugin_resource_tracker.h @@ -69,7 +69,7 @@ class PluginResourceTracker : public ::ppapi::TrackerBase { friend class PluginProxyTest; PluginResourceTracker(); - ~PluginResourceTracker(); + virtual ~PluginResourceTracker(); struct ResourceInfo { ResourceInfo(); diff --git a/ppapi/proxy/ppb_flash_file_proxy.cc b/ppapi/proxy/ppb_flash_file_proxy.cc index 9e2aab66..508a6d2 100644 --- a/ppapi/proxy/ppb_flash_file_proxy.cc +++ b/ppapi/proxy/ppb_flash_file_proxy.cc @@ -103,7 +103,7 @@ class ModuleLocalThreadAdapter std::set<int> pending_requests_for_filter_; }; - virtual void SendFromIOThread(Dispatcher* dispatcher, IPC::Message* msg); + void SendFromIOThread(Dispatcher* dispatcher, IPC::Message* msg); // Internal version of OnModuleLocalMessageFailed which assumes the lock // is already held. diff --git a/remoting/jingle_glue/jingle_client.h b/remoting/jingle_glue/jingle_client.h index cb7e2d3..e3abbed 100644 --- a/remoting/jingle_glue/jingle_client.h +++ b/remoting/jingle_glue/jingle_client.h @@ -145,7 +145,7 @@ class JingleClient : public base::RefCountedThreadSafe<JingleClient>, talk_base::PacketSocketFactory* socket_factory, PortAllocatorSessionFactory* session_factory, Callback* callback); - ~JingleClient(); + virtual ~JingleClient(); // Starts the XMPP connection initialization. Must be called only once. // |callback| specifies callback object for the client and must not be NULL. diff --git a/remoting/protocol/protocol_test_client.cc b/remoting/protocol/protocol_test_client.cc index d1a6f7e..89d151c 100644 --- a/remoting/protocol/protocol_test_client.cc +++ b/remoting/protocol/protocol_test_client.cc @@ -52,6 +52,8 @@ class ProtocolTestConnection closed_event_(true, false) { } + virtual ~ProtocolTestConnection() {} + void Init(Session* session); void Write(const std::string& str); void Read(); diff --git a/tools/clang/scripts/update.sh b/tools/clang/scripts/update.sh index f56ce60..dab5c86 100755 --- a/tools/clang/scripts/update.sh +++ b/tools/clang/scripts/update.sh @@ -5,7 +5,7 @@ # This script will check out llvm and clang into third_party/llvm and build it. -CLANG_REVISION=131935 +CLANG_REVISION=132017 THIS_DIR="$(dirname "${0}")" LLVM_DIR="${THIS_DIR}"/../../../third_party/llvm diff --git a/ui/base/x/active_window_watcher_x.h b/ui/base/x/active_window_watcher_x.h index 73ae36f..9a253f5 100644 --- a/ui/base/x/active_window_watcher_x.h +++ b/ui/base/x/active_window_watcher_x.h @@ -42,7 +42,7 @@ class ActiveWindowWatcherX { friend struct DefaultSingletonTraits<ActiveWindowWatcherX>; ActiveWindowWatcherX(); - ~ActiveWindowWatcherX(); + virtual ~ActiveWindowWatcherX(); void Init(); diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h index f489490..b0e23cf 100644 --- a/webkit/fileapi/file_system_file_util.h +++ b/webkit/fileapi/file_system_file_util.h @@ -240,6 +240,7 @@ class FileSystemFileUtil { protected: FileSystemFileUtil() { } + virtual ~FileSystemFileUtil() { } // Deletes a directory and all entries under the directory. // diff --git a/webkit/fileapi/quota_file_util.h b/webkit/fileapi/quota_file_util.h index 4a9c952..eb34541 100644 --- a/webkit/fileapi/quota_file_util.h +++ b/webkit/fileapi/quota_file_util.h @@ -14,7 +14,7 @@ namespace fileapi { class QuotaFileUtil : public FileSystemFileUtil { public: static QuotaFileUtil* GetInstance(); - ~QuotaFileUtil() {} + virtual ~QuotaFileUtil() {} static const int64 kNoLimit; diff --git a/webkit/glue/resource_fetcher.h b/webkit/glue/resource_fetcher.h index ca666c7..0a9de89 100644 --- a/webkit/glue/resource_fetcher.h +++ b/webkit/glue/resource_fetcher.h @@ -45,7 +45,7 @@ class ResourceFetcher : public WebKit::WebURLLoaderClient { ResourceFetcher( const GURL& url, WebKit::WebFrame* frame, WebKit::WebURLRequest::TargetType target_type, Callback* callback); - ~ResourceFetcher(); + virtual ~ResourceFetcher(); // Stop the request and don't call the callback. void Cancel(); diff --git a/webkit/glue/resource_fetcher_unittest.cc b/webkit/glue/resource_fetcher_unittest.cc index 2888a94..9ef0fc6 100644 --- a/webkit/glue/resource_fetcher_unittest.cc +++ b/webkit/glue/resource_fetcher_unittest.cc @@ -40,6 +40,8 @@ class FetcherDelegate { StartTimer(); } + virtual ~FetcherDelegate() {} + ResourceFetcher::Callback* NewCallback() { return ::NewCallback(this, &FetcherDelegate::OnURLFetchComplete); } @@ -173,12 +175,14 @@ TEST_F(ResourceFetcherTests, ResourceFetcherTimeout) { class EvilFetcherDelegate : public FetcherDelegate { public: + virtual ~EvilFetcherDelegate() {} + void SetFetcher(ResourceFetcher* fetcher) { fetcher_.reset(fetcher); } - void OnURLFetchComplete(const WebURLResponse& response, - const std::string& data) { + virtual void OnURLFetchComplete(const WebURLResponse& response, + const std::string& data) { // Destroy the ResourceFetcher here. We are testing that upon returning // to the ResourceFetcher that it does not crash. fetcher_.reset(); diff --git a/webkit/plugins/ppapi/mock_plugin_delegate.h b/webkit/plugins/ppapi/mock_plugin_delegate.h index db2db0c..940dece 100644 --- a/webkit/plugins/ppapi/mock_plugin_delegate.h +++ b/webkit/plugins/ppapi/mock_plugin_delegate.h @@ -13,7 +13,7 @@ namespace ppapi { class MockPluginDelegate : public PluginDelegate { public: MockPluginDelegate(); - ~MockPluginDelegate(); + virtual ~MockPluginDelegate(); virtual void PluginFocusChanged(bool focused); virtual void PluginCrashed(PluginInstance* instance); diff --git a/webkit/plugins/ppapi/ppapi_webplugin_impl.h b/webkit/plugins/ppapi/ppapi_webplugin_impl.h index 5644e94..e35471f 100644 --- a/webkit/plugins/ppapi/ppapi_webplugin_impl.h +++ b/webkit/plugins/ppapi/ppapi_webplugin_impl.h @@ -35,7 +35,7 @@ class WebPluginImpl : public WebKit::WebPlugin { private: friend class DeleteTask<WebPluginImpl>; - ~WebPluginImpl(); + virtual ~WebPluginImpl(); // WebKit::WebPlugin implementation. virtual bool initialize(WebKit::WebPluginContainer* container); diff --git a/webkit/tools/test_shell/test_webview_delegate.h b/webkit/tools/test_shell/test_webview_delegate.h index bacdd40..5673443 100644 --- a/webkit/tools/test_shell/test_webview_delegate.h +++ b/webkit/tools/test_shell/test_webview_delegate.h @@ -251,7 +251,7 @@ class TestWebViewDelegate : public WebKit::WebViewClient, virtual WebKit::WebCookieJar* GetCookieJar(); TestWebViewDelegate(TestShell* shell); - ~TestWebViewDelegate(); + virtual ~TestWebViewDelegate(); void Reset(); void SetSmartInsertDeleteEnabled(bool enabled); |