Skip to content

MinGW warning 修正 (-Wdelete-non-virtual-dtor)#2391

Open
gorogoro123 wants to merge 1 commit intosakura-editor:masterfrom
gorogoro123:feature/Wdelete_non_virtual_dtor
Open

MinGW warning 修正 (-Wdelete-non-virtual-dtor)#2391
gorogoro123 wants to merge 1 commit intosakura-editor:masterfrom
gorogoro123:feature/Wdelete_non_virtual_dtor

Conversation

@gorogoro123
Copy link
Contributor

PR対象

  • アプリ(サクラエディタ本体)

カテゴリ

  • 改善

PR の背景

MinGW build 時に warning が表示される。

C:/work/sakura/sakura_core/_os/CDropTarget.h:179:25: warning: deleting object of polymorphic class type 'CEnumFORMATETC' which has non-virtual destructor might cause undefined behavior [-Wdelete-non-virtual-dtor] 179 | delete this; | ^~~~~~~~~~~ 

仕様・動作説明

デストラクタを追加します。

PR の影響範囲

影響なし。

テスト内容

MinGW build 時に warning が表示されないことを確認する。

関連 issue, PR

#2298

参考資料

https://gcc.gnu.org/wiki/VerboseDiagnostics

@github-actions
Copy link

Test Results

710 tests  ±0   710 ✅ ±0   2m 31s ⏱️ +4s
 83 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit b7c0250. ± Comparison against base commit 9c40b00.

@berryzplus
Copy link
Contributor

言ってることは間違ってない。

struct IEnumFORMATC から直接派生する COM実装 クラスなら、この対応もアリと思う。

この問題は IUnknown::Release() を実装するクラスにありがちな話で、
IUnkown をテンプレートクラスで実装してしまうのがセオリー。

サクラエディタにも CYbInterfaceImpl<T>TComImpl<T> が存在してる。
簡易定義で「とりあえず組み込めるようにする」のが CYbInterfaceImpl<T> (20年まえからあるやつ。)、
ある程度まともに実装したのが TComImpl<T>(ここ数年で入ったやつ。)。
CEnumFORMATC は「とりあえず版」をベースにしてるけど、
OSと連携させるために「ある程度まともな実装」を入れてある状態。

「virtual デストラクターがないからdeleteすると危ないよ」の警告が出た理由は「とりあえず版」に後付けで「まともっぽい実装」を乗せたことによる弊害。

IEnumXXX 系の実装は「テンプレートクラスに一本化できる」けど、マージする時期を見計らっている感じ。

入れたらアカンのか? と言われるとそこまででもないんだけど、意味のない修正ではあるのかも。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants