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
Copy Markdown
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
Copy Markdown

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.

@sonarqubecloud
Copy link
Copy Markdown

@berryzplus
Copy link
Copy Markdown
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants