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

[x64対応] C4477 の警告を修正する #88

Closed
m-tmatma opened this issue Jun 10, 2018 · 11 comments
Closed

[x64対応] C4477 の警告を修正する #88

m-tmatma opened this issue Jun 10, 2018 · 11 comments
Assignees
Labels
🐛bug🦋 ■バグ修正(Something isn't working) x64 x64 対応
Milestone

Comments

@m-tmatma
Copy link
Member

m-tmatma commented Jun 10, 2018

[x64対応] C4477 の警告を修正する

https://msdn.microsoft.com/ja-jp/library/tcxf1dw6.aspx

size_t (つまり、32 ビット プラットフォーム上では unsigned __int32、64 ビット プラットフォーム上では unsigned __int64) → I

ptrdiff_t (つまり、32 ビット プラットフォーム上では __int32、64 ビット プラットフォーム上では __int64) → I

@m-tmatma
Copy link
Member Author

#89 を送りました。

@m-tmatma m-tmatma self-assigned this Jun 10, 2018
@m-tmatma m-tmatma added the x64 x64 対応 label Jun 10, 2018
@berryzplus
Copy link
Contributor

一度何人かで話したいです。

@kobake
Copy link
Member

kobake commented Jun 11, 2018

一度何人かで話したいです。

論点は何でしょう。
PR へのレビューコメントでは足りない感じですか?

@berryzplus
Copy link
Contributor

対応すべきか否か

Disable warningも選択の1つです。

@m-tmatma
Copy link
Member Author

それは、誤検出か、コンパイラのヘッダが原因の場合にする対策だと思います。

無効にするにしても、個別に判断することだと思います。

@berryzplus
Copy link
Contributor

本業があるんで細切れですみません。

X86版では主要な警告のいくつかをstdafx.hで切り捨てています。同じレベルの対応とすることは可能と思ってます。

修正眺めた感じかなりの量ありそうでしたが、これは今やるべきか?ということを言ってます。

今やるかどうかです。

@kobake
Copy link
Member

kobake commented Jun 11, 2018

x64 関連の Warning はスルーすると普通に動作不具合に繋がる系が多いと思っています。

たとえばこの Issue とは違う警告ですが warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data 等は明らかにデータが失われる状況ありますよね。こういうのは無視しちゃいけないやつです。

他 Warning についても同様に慎重に精査したほうが良いと思っています。

@kobake
Copy link
Member

kobake commented Jun 11, 2018

補足です。

X86版では主要な警告のいくつかをstdafx.hで切り捨てています。同じレベルの対応とすることは可能と思ってます。

x86 版でのキャスト処理等は割と問題起こらなかったりします。
たとえば上に挙げた warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data については
x86 版だと sizeof(size_t) == sizeof(int) == 4 なので問題なし、
x64 版だと sizeof(size_t) == 8, sizeof(int) == 4 なので問題あり、
という感じです。

@m-tmatma m-tmatma added this to the next release milestone Jun 11, 2018
@m-tmatma m-tmatma changed the title [x64対応] (着手済) C4477 の警告を修正する [x64対応] C4477 の警告を修正する Jun 11, 2018
@berryzplus
Copy link
Contributor

対応には賛成で、何件か見た限り修正は適切でした

いまこのprは特急でx64対応を進める作業の一環だと思っています。

特急で仕上げるにはみるべきことが少し多いように感じています。

リアルタイムでの参加は難しそうなので、一旦はそのまま進めてください。何かあれば後追いで質問するかも知れませんのでその時はよろしくお願いします。

@kobake
Copy link
Member

kobake commented Jun 11, 2018

自分としてはx64対応は特急じゃなくて良いんじゃないかなーと思っています。慎重にやらないと事故りそう。

m-tmatma added a commit to m-tmatma/sakura that referenced this issue Jun 11, 2018
※ MakefileMake の x64 のビルド構成はないので表面化しないが潜在バグなので修正しておく
@m-tmatma
Copy link
Member Author

#95 を追加しました。

@m-tmatma m-tmatma added the 🐛bug🦋 ■バグ修正(Something isn't working) label Jun 11, 2018
berryzplus pushed a commit to berryzplus/sakura-editor that referenced this issue Jul 5, 2018
※ MakefileMake の x64 のビルド構成はないので表面化しないが潜在バグなので修正しておく
@ds14050 ds14050 added 🐛bug🦋 ■バグ修正(Something isn't working) x64 x64 対応 labels Sep 18, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this issue Jun 11, 2019
※ MakefileMake の x64 のビルド構成はないので表面化しないが潜在バグなので修正しておく
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working) x64 x64 対応
Projects
None yet
Development

No branches or pull requests

4 participants