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ビルドのエラーに対処する #559

Merged
merged 7 commits into from
Nov 1, 2018

Conversation

berryzplus
Copy link
Contributor

#548 (comment)

ビルドエラーの原因を確認しました。

どう対処すべきかが微妙な原因ばかり3件ありました。

それぞれ「どうすればビルドが通るか」の方法は見付けています。

見つけた対策それぞれにコメント書いておきますので、
こうした方がいい、とかあればご意見ください。

(実際にマージするときはコミットコメントを変えると思います。)

msvcではvoid*にポインタ型を代入するときにキャストを書かなくても変換できる。
gccではvoid*への代入であってもキャストが必要っぽい。
とりあえずキャストを書いておけばビルドは通るが、正攻法があるような気もする。
プロパティシート用の構造体サイズPROPSHEETHEADER_V2_SIZEの定義が壊れている?
構造体の定義自体は正しいようなのでsizeofで書き換えればビルドはできる。
該当箇所を示すためにとりあえず該当箇所を修正してあるが、
PROPSHEETHEADER_V2_SIZEを再定義する作戦にすれば修正は一か所で済み、今後プロパティシートが増えても影響ない。
どうするべきか微妙。
TagIterator::eachの引数(非const lvalue&)に対して関数の戻り値(rvalue)を指定しているのが怒られている。
対策としてはeachの引数をconstにするか、rvalueにするか。
修正量はrvalueにするほうが少ないのでとりあえずrvalueにしてみた。
@berryzplus berryzplus changed the title [WIP] MinGWビルドのエラーに対処する MinGWビルドのエラーに対処する Oct 18, 2018
@berryzplus
Copy link
Contributor Author

ビルドが走らんのでWIP外しました。

内容的にはこのままマージしてもいいはずなんで、いいか、と。

@@ -260,8 +260,8 @@ INT_PTR CPropCommon::DoPropertySheet( int nPageNum, bool bTrayProc )
}
// To Here Jun. 2, 2001 genta

PROPSHEETHEADER psh = { PROPSHEETHEADER_V2_SIZE };
psh.dwSize = PROPSHEETHEADER_V2_SIZE;
PROPSHEETHEADER psh = { sizeof(PROPSHEETHEADER) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROPSHEETHEADER_V2_SIZE が壊れているというのは具体的にどういうことですか?
この修正を入れない場合はどんなエラーが出ますか?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sizeof(PROPSHEETHEADER) を使うことのデメリットは、将来的に
この構造体が拡張されて、ヘッダ定義が変わると新しいSDKヘッダで
コンパイルするとコードの振る舞いが変わってしまうことです。
MinGW のコンパイルを通すために VC 側を変えてしまうのはよろしくないと思います。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinGW-w64 を使っていれば PROPSHEETHEADER_V2_SIZE は定義されているはずですが、古い MinGW では定義されていないようです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あとでログ付けます。

シンボル定義自体は存在しているのですが、Windows SDKの定義方法と違っていて「未定義のメンバを参照している」的なエラーが出ます。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ローカルビルドで確認したエラーメッセージは次の通りです。

In file included from D:/eclipse4.7/eclipse/mingw/x86_64-w64-mingw32/include/winspool.h:12:0,
                 from D:/eclipse4.7/eclipse/mingw/x86_64-w64-mingw32/include/Windows.h:102,
                 from _main/global.h:29,
                 from StdAfx.h:42:
prop/CPropCommon.cpp: In member function 'INT_PTR CPropCommon::DoPropertySheet(int, bool)':
prop/CPropCommon.cpp:263:26: error: 'PROPSHEETHEADERW {aka struct _PROPSHEETHEADERW}' has no member named 'DUMMYUNION5_MEMBER'
  PROPSHEETHEADER psh = { PROPSHEETHEADER_V2_SIZE };

おそらく 'PROPSHEETHEADERW {aka struct _PROPSHEETHEADERW}' has no member named 'DUMMYUNION5_MEMBER' という部分がエラー原因です。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かにMinGWのヘッダーが壊れているようですね。このエラーからすると、

#ifndef NONAMELESSUNION
#define DUMMYUNION5_MEMBER(x) x
#else /* NONAMELESSUNION */
#define DUMMYUNION5_MEMBER(x) DUMMYUNIONNAME5.x
#endif

こんな感じの定義がどこか(_mingw.hやwinnt.h辺り?)に必要なはずですが見当たりませんね。
どこかに自前で定義したら通りますかね?

// Workaround for PROPSHEETHEADER_V2_SIZE
#if defined(__MINGW__) && !defined(DUMMYUNION5_MEMBER)
#ifndef NONAMELESSUNION
#define DUMMYUNION5_MEMBER(x) x
#else /* NONAMELESSUNION */
#define DUMMYUNION5_MEMBER(x) DUMMYUNIONNAME5.x
#endif
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@k-takata さん

どこかに自前で定義したら通りますかね?

ローカルでやってみたら通りました。 -> fbb7f8f

本家にあたる宣言は確認できませんでしたが、元ネタっぽいものは見付けました。
(最後の方に1~5の定義がある)
http://en.verysource.com/code/281895_1/priv.h.html

どっかからコピってきた雰囲気なので、本家は別っぽいです。

DUMMYUNION5_MEMBER でググると mingw-w64 の prsht.h を取り込んだものがヒットするので、過去のどっかの時点までは生きていた実装なのかな、と思ってます。GitHubにあるMinGWのミラーで検索してみると DUMMYUNION5_MEMBERの利用箇所はここだけっぽいです・・・
https://github.com/mirror/mingw-w64/search?q=DUMMYUNION5_MEMBER&unscoped_q=DUMMYUNION5_MEMBER

@berryzplus
Copy link
Contributor Author

3つ目のエラーのログがこれです。

types/CType_Tex.cpp: In member function 'void CDocOutline::MakeTopicList_tex(CFuncInfoArr*)':
types/CType_Tex.cpp:281:19: error: cannot bind non-const lvalue reference of type 'TagProcessor<4>&' to an rvalue of type 'TagProcessor<4>'
   MakeTagProcessor(*pcFuncInfoArr, m_pcDocRef->m_cLayoutMgr, TagHierarchy)
   ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
types/CType_Tex.cpp:229:7: note:   initializing argument 1 of 'void TagIterator::each(TagProcessor<HierarchyCount>&) [with int HierarchyCount = 4]'
  void each(TagProcessor<HierarchyCount>& process)
       ^~~~

おそらく error: cannot bind non-const lvalue reference of type 'TagProcessor<4>&' to an rvalue of type 'TagProcessor<4>' の部分がエラー原因です。

非constなlvalue参照とrvalueを関連付けることができませんでした、的な意味です。
文脈からすると、関数の戻り値(=右辺値参照rvalue)を非constな参照引数に直接渡しているのが原因っぽいです。対応策はいくつか考えられますが、関数側を右辺値参照を引数に取るように修正しました。 -> 4834e8a

@m-tmatma
Copy link
Member

文脈からすると、関数の戻り値(=右辺値参照rvalue)を非constな参照引数に直接渡しているのが原因っぽいです。対応策はいくつか考えられますが、関数側を右辺値参照を引数に取るように修正しました。 -> 4834e8a

const をつけるのが自然なら const をつける対応でお願いしたいです。

@berryzplus
Copy link
Contributor Author

const をつけるのが自然なら const をつける対応でお願いしたいです。

設計的に非constな引数が欲しい場面です。
constを付けると設計全体を見直すことになるのでやめました。

修正箇所 (&&&) を ( &const & )に変えてビルドしてみると言ってる意味が分かると思います。const参照を介して呼び出せるのはconstメンバだけなので、芋づる式に修正範囲が広がります。そして、constメンバ関数ではインスタンスのメンバを変更できない制約があるので、設計を組みなおさないといけない・・・:sob:

MinGW-gccのエラー解析で邪魔になるので対処しておく。
警告の原因はmsvcとgccでNULLの定義が異なるため。
* msvcのNULLは0(数値)
* gccのNULLはnullptr(nullptr_t)
MinGWのprsht.hに含まれるDUMMYUNION5_MEMBERマクロを独自定義する。

元定義の出所は不詳。
http://en.verysource.com/code/281895_1/priv.h.html に近い定義がある。
@berryzplus
Copy link
Contributor Author

一個追加で警告対策のコミットを混ぜました。f1f3320
あと、prsht.hの対策内容を差し替えるコミットを積みました。

appveyorのMinGWビルドも通ったようなので再レビューをお願いします。

@k-takata
Copy link
Member

あと、prsht.hの対策内容を差し替えるコミットを積みました。

DUMMYUNION 関連の定義は winnt.h や _mingw.h の中でされているので、最初にいずれかのファイルが読み込まれた後に、例の定義を追加するべきだと思います。
"_main/global.h" で windows.h を読んでいて、commctrl.h は prsht.h を読んでいるので、その間が良いかと。

@berryzplus
Copy link
Contributor Author

@k-takata さん

DUMMYUNION 関連の定義は winnt.h や _mingw.h の中でされているので、最初にいずれかのファイルが読み込まれた後に、例の定義を追加するべきだと思います。
"_main/global.h" で windows.h を読んでいて、commctrl.h は prsht.h を読んでいるので、その間が良いかと。

ログ貼ったやつがリベースで消えたっぽいのでもう一回貼ります。

In file included from D:/eclipse4.7/eclipse/mingw/x86_64-w64-mingw32/include/winspool.h:12:0,
                 from D:/eclipse4.7/eclipse/mingw/x86_64-w64-mingw32/include/Windows.h:102,
                 from _main/global.h:29,
                 from StdAfx.h:52:
prop/CPropCommon.cpp:263:26: error: 'PROPSHEETHEADERW {aka struct _PROPSHEETHEADERW}' has no member named 'DUMMYUNION5_MEMBER'
  PROPSHEETHEADER psh = { PROPSHEETHEADER_V2_SIZE };

対応差し替えで挿入した位置は windows.h の include 前なんですけど、windows.hからwinspool.hが読まれてwinspool.hがprsht.hを読み込む構造になってるから、前にしたわけです。defineなんで、実際に使う直前にしてもいいのかも知れませんが、あちこち書くのが嫌だったのでstdafx.hに入れました。

@k-takata
Copy link
Member

ああそうなると、以前のように _mingw.h を明示的にインクルードして、その後に入れた方がいいかもしれませんね。たとえ本家が修正されて DUMMYUNION5_MEMBER が定義されたとしても、現状の順序だと、!defined(DUMMYUNION5_MEMBER) が用をなさないので。(もちろん本家がどのように修正されるかは分かりませんが、winnt.h と _mingw.h に修正が入る可能性が高いと思っています。)

なので、こんなのはどうでしょうか。(indentした方がよさげ?)

// Workaround for PROPSHEETHEADER_V2_SIZE
#ifdef __MINGW__
#include <_mingw.h>
#ifndef DUMMYUNION5_MEMBER
#ifndef NONAMELESSUNION
#define DUMMYUNION5_MEMBER(x) x
#else /* NONAMELESSUNION */
#define DUMMYUNION5_MEMBER(x) DUMMYUNIONNAME5.x
#endif
#endif
#endif

ログ貼ったやつがリベースで消えたっぽい

Outdated をクリックすれば見ることができます。わざわざクリックしないと見えないのは面倒ですが。

@berryzplus
Copy link
Contributor Author

ああそうなると、以前のように _mingw.h を明示的にインクルードして、その後に入れた方がいいかもしれませんね。たとえ本家が修正されて DUMMYUNION5_MEMBER が定義されたとしても、現状の順序だと、!defined(DUMMYUNION5_MEMBER) が用をなさないので。(もちろん本家がどのように修正されるかは分かりませんが、winnt.h と _mingw.h に修正が入る可能性が高いと思っています。)

DUMMYUNION5_MEMBER の利用箇所が1箇所だけで、しかも、1~4の利用がないので忘れ去られてる可能性があります。githubに https://github.com/mirror/mingw-w64 ってあるので、PRしたら見てくれるかなと思ってます。活動拠点は sourceforge.net のメーリングリストがメインっぽくて中見れないんですが、最近も活動は続いてる雰囲気なので。

なので、こんなのはどうでしょうか。(indentした方がよさげ?)

ではそんな感じで行きますw

うちの環境(eclipse4.6.3)だと __MINGW__ というシンボルの代わりに __MINGW32__ が定義されています。
定義済みマクロの確認は gcc -x c++ -dM -E - <NUL でしてます。

Outdated をクリックすれば見ることができます。わざわざクリックしないと見えないのは面倒ですが。

win8.1+IE11だとOutdatedのリンクをクリックしても表示されなかったのです...orz
最近は最新版google chromeも併用してます。
・・・chromeだと見られるし...orz ←気付いてなかった

@k-takata
Copy link
Member

githubに https://github.com/mirror/mingw-w64 ってあるので、PRしたら見てくれるかなと思ってます。

user名がmirrorなので完全な第3者がやってそうな雰囲気。

@k-takata
Copy link
Member

http://mingw-w64.org/doku.php/contribute
MinGW-w64 の開発はメーリングリストベースか…。

@berryzplus
Copy link
Contributor Author

まったく関係ない話ですがこれが気になりました。

SEH for 32bits

The patent for SEH for 32bits has now expired and while new computers are all 64 bits, Intel has continued selling Atom CPUs that only handled 32 bits very late and some applications are still 32 bits. Projects such as Wine and ReactOS will also benefit from 32bits SEH. Overall the need is still there and will continue for years to come and will outlast Microsoft's support for 32bits.

gcc on Windowsにかけられた borland の呪いはとっくに解けていたんですね・・・。

Copy link
Contributor

@ds14050 ds14050 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この PR が未だマージされていないことに驚いてしまいました。MinGW ビルドが通らないのでとりあえず入れてしまって、その後の問題は起きてから考えてもいいと思います。

@ds14050
Copy link
Contributor

ds14050 commented Oct 29, 2018

@berryzplus さんへ、予告です。

MinGW でテストをビルドすると tests\unittests\test-is_mailaddress.cpp に由来して _swprintf_p が存在しないというリンクエラーが出ました。近いうちに対応が必要になるはずです。

@berryzplus
Copy link
Contributor Author

MinGW でテストをビルドすると tests\unittests\test-is_mailaddress.cpp に由来して _swprintf_p が存在しないというリンクエラーが出ました。近いうちに対応が必要になるはずです。

自分で何個かテスト書いて知ってて、
どっちに倒すべきか考えていたとこでした。

_swprintf_p という関数はマイクロソフトの独自実装なので、MSVCRTにリンクしないと使えません。
しかし、MinGWはCランタイムとしてMSVCRTを流用するGCCの亜種です。
既にリンクしてるライブラリに含まれる関数の話なので「その気」になればMinGWで_swprintf_pを使うのは簡単なはずなんです。(たぶんdllimport属性つけて宣言すればいいだけだと思います。)

やってみたことがないことは断言できんので、フタを開けてみたら意外と大変かも知れないのが懸念です。

@k-takata
Copy link
Member

MinGWはmsvcrt.dllを使用しますが、VCのprintf系関数は規格への準拠度が低いため、printf系関数はMinGW独自のものを使用することができるようになっています。
あと、msvcrt.dllは元々VC6のランタイムでしたが、現在はOSの一部になっており、OSのバージョンによって、公開されている関数が異なっていたりします。手元のWin10 1809のmsvcrt.dllでは _swprintf_p はエクスポートされていません。(_swprintf_p_l はエクスポートされています。)

@ds14050
Copy link
Contributor

ds14050 commented Oct 30, 2018

テストコードだから凝ったことをせずに同じ引数を並べたり、長ーいリテラルを埋め込んでもいいと思うんですけど、他人のコードなのでお任せします。

@m-tmatma
Copy link
Member

修正箇所 (&&&) を ( &const & )に変えてビルドしてみると言ってる意味が分かると思います。

以下のエラーになりました。

types/CType_Tex.cpp:282:2:   required from here
types/CType_Tex.cpp:260:17: error: no match for call to '(const TagProcessor<4>) (CLogicInt&, const wchar_t* const&, const wchar_t*&, const wchar_t*&, const wchar_t*&, const wchar_t*&, const wchar_t* const&)'
      p = process(nLineNumber, pLine, pTag, pTagEnd, pTitle, pTitleEnd, pLineEnd);
          ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
types/CType_Tex.cpp:92:17: note: candidate: 'const wchar_t* TagProcessor<HierarchyCount>::operator()(CLogicInt, const wchar_t*, const wchar_t*, const wchar_t*, const wchar_t*, const wchar_t*, const wchar_t*) [with int HierarchyCount = 4; CLogicInt = int]' <near match>
  const wchar_t* operator()(
                 ^~~~~~~~
types/CType_Tex.cpp:92:17: note:   passing 'const TagProcessor<4>*' as 'this' argument discards qualifiers

@m-tmatma
Copy link
Member

何か残件ありますか?

@berryzplus
Copy link
Contributor Author

残件はないと思います。

テストコードまで含めるとまだ対応しないといけないことがあるのが分かってますが、それば別件なので。

修正箇所 (& → &&) を ( & → const & )に変えてビルドしてみると言ってる意味が分かると思います。

以下のエラーになりました。

・・・言葉足らずでした。

そのエラーは「constインスタンスからはconst じゃない operator () を呼べないよ」という意味です。

単純に「operator()」を「operator() const」にしてやれば、このエラー自体は解消します。
「operator()」の中身は const インスタンスで呼ぶことを想定しない実装なので、代わりに他のエラーがたくさん出ることになります。

どうなるかを知るには実際やってみるのが早いので、「やってみれば分かる」と書きました。

無理だ、と思うリファクタリングを強行してみるのも、意外と楽しいものだと思います。

@m-tmatma
Copy link
Member

対応しないといけない件を別 issue で作成していただけますか?

@m-tmatma
Copy link
Member

質問です。

master はこの PR なしでもビルド通ってるんですよね?
ビルドが失敗するのはどういう条件ですか?

@ds14050
Copy link
Contributor

ds14050 commented Nov 1, 2018

master の MinGW ビルドが現在も途中で止まっています。

https://ci.appveyor.com/project/sakuraeditor/sakura/branch/master/job/p3fngivmfmrtaek1

extmodule/CHtmlHelp.cpp:53:5: error: invalid conversion from 'CHtmlHelp::FnPtr_HtmlHelp' {aka 'HWND__* (*)(HWND__*, const wchar_t*, unsigned int, long long unsigned int)'} to 'void*' [-fpermissive]
   { m_pfnHtmlHelp,  "HtmlHelpW" },
     ^~~~~~~~~~~~~

よく考えれば、それなのに「Build success」としてグリーンになるのがおかしい状態ですね。

@m-tmatma m-tmatma merged commit df4ab03 into sakura-editor:master Nov 1, 2018
@m-tmatma
Copy link
Member

m-tmatma commented Nov 1, 2018

マージしました。

@berryzplus berryzplus deleted the feature/cast_to_void_ptr branch November 1, 2018 16:32
@m-tmatma m-tmatma added this to the next release milestone Dec 1, 2018
@KENCHjp KENCHjp added the CI appveyor など CI 関連 【ChangeLog除外】 label Dec 11, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…void_ptr

MinGWビルドのエラーに対処する
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI appveyor など CI 関連 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants