Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MinGW の Debug で CNativeW.Clear のテストに失敗する #782

Closed
m-tmatma opened this issue Feb 4, 2019 · 9 comments
Closed

MinGW の Debug で CNativeW.Clear のテストに失敗する #782

m-tmatma opened this issue Feb 4, 2019 · 9 comments
Labels
🐛bug🦋 ■バグ修正(Something isn't working) MinGW MinGW UnitTest
Milestone

Comments

@m-tmatma
Copy link
Member

m-tmatma commented Feb 4, 2019

バグ内容

MinGW の Debug で CNativeW.Clear のテストに失敗する
https://ci.appveyor.com/project/sakuraeditor/sakura/builds/22084234/job/l5hd5e6yvtkp2nak/tests

再現手順

MinGW の Debug構成で単体テストをビルドして CNativeW.Clear を実行する。

関連 PR

#776 (comment)
#776 で発見されましたが、#776 なしでも発生します。

再現頻度

100%

メモ

#781 の問題のためにテストに失敗しても appveyor は失敗しません。

@m-tmatma
Copy link
Member Author

m-tmatma commented Feb 4, 2019

以下参照

#776 (comment)
#776 (comment)

@berryzplus
Copy link
Contributor

ぶっちゃけ、コイツのせいです。

#if _DEBUG
private:
typedef wchar_t* PWCHAR;
PWCHAR& m_pDebugData; //デバッグ用。CMemoryの内容をwchar_t*型でウォッチするためのモノ
#endif

というコメントをしたら @kobake さんは怒るでしょうか・・・ 😄

テストが通らなかった原因は CNativeW のサイズが _DEBUGNDEBUG で異なることで、
テスト用Exeをビルドするプリプロセッサシンボル定義に _DEBUG がなかったことでした。
ある意味で、こういう型が存在していることが悪い(何故か全否定)とも言えます。

とはいえ、こういうメンバ変数が必要だった事情はよく分かります。

visual studio を使ってデバッグする場合、メンバ変数の情報は自動的に解析されて適切なデータ型でデバッグ変数ウインドウ(ローカルとか自動変数とか)に表示されます。CNativeWはCMemoryを継承するクラスですが、CMemoryのデータバッファは char*型 です。visual studio は優秀なので、CNativeWがCMemoryを継承していることを検出し、データバッファを char*型 で表示しようとします。CNativeWを使うときに m_pRawData に入っているのは wchar_t*型 のデータです。 標準状態では表示できないデータを適切に表示させるためには、 m_pDebugData のような参照変数が必要でした。

デバッグを便利にする方法も進化しています。

最新の visual studio には、参照変数を用意しなくても char*型 の変数 m_pRawData の中身を wchar_t*型 として表示する方法があります。
https://blogs.msdn.microsoft.com/vcblog/2015/09/28/debug-visualizers-in-visual-c-2015/

問題解決&再発防止の方策として、m_pDebugData を廃止する、というのも一案だと思っています。

@m-tmatma
Copy link
Member Author

m-tmatma commented Feb 4, 2019

↑ はなぜコンパイルオプションが混ざるとダメかという理由であって
この問題はコンパイルオプションが混ざっているのが原因だと思います。

なので別の問題です。

@ds14050
Copy link
Contributor

ds14050 commented Feb 5, 2019

コンパイルオプションの修正を PR にしました> #783

ツールや環境の進化に合わせた必要性の再考や、よりよい代替手段の模索はあっていいと思いますが、手法の禁止ありきの縛りプレイには参加できません。現在の基準で過去の選択を批判するのもアンフェアだと思います。

@ds14050
Copy link
Contributor

ds14050 commented Feb 5, 2019

調べて結果をまだ書いていませんでした。

@berryzplus さんが #782 (comment) で引用したのはヘッダファイルです。テストプログラムのコンパイル時に(デバッグビルドであっても) _DEBUG が未定義の状態で評価されたため、メンバ変数 m_pDebugData が定義されませんでした。

しかしそこから呼び出される CNativeW のコンストラクタはエディタの一部として(正しく) _DEBUG が定義された状態でコンパイルされたものなので、m_pDebugData が存在するものとして値を初期化しようとします。

その結果テストプログラムのスタック上で CNativeW と近接する変数の値が破壊されてしまいました。

@m-tmatma m-tmatma added this to the next release milestone Feb 5, 2019
@berryzplus
Copy link
Contributor

対応方法はいろいろ考えられたと思うんですが、
個人的には #378(CMake対応) の運用開始を目指したいです。

CNativeWの問題は、エディタ本体とテストのビルドを統合すれば起きないことなので。

@ds14050
Copy link
Contributor

ds14050 commented Feb 8, 2019

CNativeWの問題は、エディタ本体とテストのビルドを統合すれば起きないことなので。

これはまったくその通りです。

しかし CMake 対応の行き着く先が望むべきものか、やや疑問があります。つまり、CMake は Visual Studio のソリューションファイルとプロジェクトファイルを作成しますけれど、そこには CMake を実行した環境と結びついたフルパスが埋め込まれます。生成されたファイルはポータブルではなくあくまでも中間生成物としての寿命しか想定されません。自分はもはや Visual Studio にはこだわりませんが、他の皆さんは Visual Studio を起動する前に CMake を実行することをどう考えるだろうか、と。

CMake (CMakeLists.txt) を通してプロジェクトのソースファイルをフォルダでグループ分けする方法もわかりませんでした。CMake で生成したプロジェクトを Visual Studio で開くとすべてのソースファイルがフラットに並びます。FOLDER ターゲットプロパティがそれっぽいのですが確証はありません。

@berryzplus
Copy link
Contributor

しかし CMake 対応の行き着く先が望むべきものか、やや疑問があります。つまり、CMake は Visual Studio のソリューションファイルとプロジェクトファイルを作成しますけれど、そこには CMake を実行した環境と結びついたフルパスが埋め込まれます。生成されたファイルはポータブルではなくあくまでも中間生成物としての寿命しか想定されません。自分はもはや Visual Studio にはこだわりませんが、他の皆さんは Visual Studio を起動する前に CMake を実行することをどう考えるだろうか、と。

CMake 化については、過去に色々話してきました。
issue検索したらいっぱい出てくると思います。

ポイントとして把握しておきたいのはCMakeとvs2017の連携方法は2つある、ということです。

Visual StudioはCMakeLists.txtを直接開けます。(起動してからファイル⇒開く⇒CMake。)
「Visual Studioを起動する前にCMake」はGeneratorにvisual Studioを指定する方法で、
「visual StudioでCMakeLists.txtを開く」はGeneratorにNinjaを指定する方法です。(不正確ですが。)

「CMakeLists.txtを開く(≒Ninjaを使う)」のメリットは、PCの実装コア数に比例してビルドが速くなることと設定だけでMinGW-w64ビルドにも対応できることです。当面「visual studioの前にCMake」への移行は考えていませんが、Ninjaを使う方式との共存は検討する価値があると思っています。

CMake (CMakeLists.txt) を通してプロジェクトのソースファイルをフォルダでグループ分けする方法もわかりませんでした。CMake で生成したプロジェクトを Visual Studio で開くとすべてのソースファイルがフラットに並びます。FOLDER ターゲットプロパティがそれっぽいのですが確証はありません。

visual studio で CMake を開くと、 CMakelists.txt があるフォルダ配下の物理フォルダ階層がそのままソリューションビューに表示されます。sakura.vcxproj.filters のように論理階層を切ることはできませんが、個人的には物理フォルダの仕分だけで十分な構造管理ができると思ってます。

@ds14050
Copy link
Contributor

ds14050 commented Feb 9, 2019

Visual Studio で直接開けるんですね! では CMakeLists.txt で一元管理する方向でしょうか。期待します。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working) MinGW MinGW UnitTest
Projects
None yet
Development

No branches or pull requests

3 participants