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.

Sign up for GitHub

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

Jump to bottom

ビルド時警告の削減 #48

Merged
merged 2 commits into from
Jun 5, 2018
Merged

ビルド時警告の削減 #48

merged 2 commits into from
Jun 5, 2018

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Jun 3, 2018 •
edited
Loading

最新のビルド結果に残っている警告を消すための対応です。(関連: #44)

対応内容は3種類で、各コミットのコミットメッセージに内容を書いています。
これを適用すると警告が全部消えてきれいな状態になる見込みです。

berryzplus added a commit to berryzplus/sakura-editor that referenced this pull request Jun 3, 2018
@berryzplus
ビルド時警告の削減 #48
b88f772 
sakura-editor#48
(appveyorでビルドされていないので空コミット入れてみます。)
@berryzplus berryzplus mentioned this pull request Jun 3, 2018
Closed
@kobake kobake closed this Jun 3, 2018
@kobake kobake reopened this Jun 3, 2018
@kobake
Copy link
Member

kobake commented Jun 3, 2018

細かいところですが、コミットメッセージを複数行で記述するときには2行目を空行にするのが慣習です。

empty-line

参考: Gitのコミットメッセージの2行目に何か書くとどうなるのか？ - Qiita

kobake
kobake reviewed Jun 3, 2018
View reviewed changes
sakura_core/config/build_config.h Outdated
#define FILL_STRANGE_IN_NEW_MEMORY
#endif
#if defined( _DEBUG ) && !( defined( _MSC_VER ) || defined( __MINGW32__ ) )
#define FILL_STRANGE_IN_NEW_MEMORY 1
Copy link
Member

Choose a reason for hiding this comment

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

FILL_STRANGE_IN_NEW_MEMORY は #ifdef 判定されているだけなので値は不要です

Copy link
Member

Choose a reason for hiding this comment

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

#51 で対応するのであれば conflict するので　revert したほうがいいです。

Copy link
Member

Choose a reason for hiding this comment

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

すみません。間違いです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

お任せします。
あえて1にした理由は間違って #if と書いても同じ動きになるように、です。

@kobake
Copy link
Member

kobake commented Jun 3, 2018

VC++ランタイムのデバッグ版newには初期値0xcdcdcdcdを設定するコードが組み込まれているので自前でゴミを入れる処理は不要。
対処として、msvcrt.dllを使う環境ではFILL_STRANGE_IN_NEW_MEMORYを定義しないようにした。

0xcdcdcdcd だとメモリビューでの表示が「ﾍﾍﾍﾍ」となって視認性がそれほど良くないので、あえて「ﾆｭｰ」っていう文字列を突っ込む処理を入れていた、という経緯があります。なので、個人的にはこの処理は走るようにしておきたいお気持ちがあります。「ﾆｭｰ」っていう文字列はさすがにアレなんですけど。

@kobake
Copy link
Member

kobake commented Jun 3, 2018

思い出してきました。

  • malloc で確保された領域 … ﾍﾍﾍﾍ.... で初期化
  • new で確保された領域 … ﾆｭｰﾆｭｰﾆｭｰ.... で初期化
  • new[] で確保された領域 … ｷﾞｭｰｷﾞｭｰｷﾞｭｰ.... で初期化

という感じに区別が付くようにしてあります。

@kobake kobake mentioned this pull request Jun 3, 2018
Merged
@kobake
Copy link
Member

kobake commented Jun 3, 2018

operator new/delete のところについては、ちょっとややこしいので独立して #51 で対応してみました

berryzplus added a commit to berryzplus/sakura-editor that referenced this pull request Jun 3, 2018
@berryzplus
Revert "ビルド時の警告削減(グローバルなnew/delete演算子はinline化されないという警告への対処)"
4444bbb 
This reverts commit db5b838.
sakura-editor#48
sakura-editor#51
@berryzplus
Copy link
Contributor Author

微修正のつもりだったんですが、色々やらかしてしまい申し訳ありません。
appveyorの件 #49 は確認できたので閉じるようにします。
new/deleteの件は @kobake さんにお任せします。 #51
コミットログ2行目空白の件は、まず今後はやらないように気を付けます。
今あげている分の修正方法がわからないです。一旦PR自体キャンセルしてやり直すべきでしょうか。

@kobake
Copy link
Member

kobake commented Jun 4, 2018

コミットログ2行目空白の件は、まず今後はやらないように気を付けます。
今あげている分の修正方法がわからないです。一旦PR自体キャンセルしてやり直すべきでしょうか。

今後絶対に必要な知識になるものとして git rebase をいう技術があります。

git rebase により

  • ブランチ内コミットメッセージの書き換え
  • コミット内容の修正
  • コミット順序の入れ替え
  • 複数コミットの統合
  • 特定コミットの除外

等々ができるようになります。

berryzplus さんが使っているツールは TortoiseGit でしたっけ。
僕が知っているのはコマンドラインベースでの git rebase だけなのですが……。

コマンドラインベースで良ければ手順書きますけど、どうでしょう。
TortoiseGit でもうまく rebase できる機構があればそちらのほうが楽な可能性もありますが。

@berryzplus
Copy link
Contributor Author

berryzplus commented Jun 4, 2018 •
edited
Loading

ありがとうございます。やれそうな気がしてきたので調べてやってみます。

基本は日本語化されたメニューの中からそれっぽいものを選んで操作しています。
困ったときはman gitしてそれっぽいオプションをググります。
分からない部分に妙な偏りがあるのはコマンドも見てるからだと思います。

う～む、ローカルは書き換えられたんだけどpushできてない。
でけた 😄

@berryzplus berryzplus force-pushed the feature/reduce_build_warnings branch from 4444bbb to 3a1e21a Compare June 4, 2018 01:42
berryzplus added a commit to berryzplus/sakura-editor that referenced this pull request Jun 4, 2018
@berryzplus
Revert "ビルド時の警告削減(グローバルなnew/delete演算子はinline化されないという警告への対処)"
c1b38d1 
This reverts commit db5b838.
sakura-editor#48
sakura-editor#51
berryzplus added a commit to berryzplus/sakura-editor that referenced this pull request Jun 4, 2018
@berryzplus
ビルド時警告の削減 #48 (rebase)
3a1e21a 
sakura-editor#48
sakura-editor#51
@m-tmatma m-tmatma added this to the next release milestone Jun 4, 2018
@kobake
Copy link
Member

kobake commented Jun 4, 2018

d118175, c1b38d1 については revert によりかき消すよりも rebase により丸々削除するのが良いです。
rebase はコマンドラインからやっていますか？それとも TortoiseGit にも rebase 機能ある感じですか？
コマンドラインから rebase やる場合には pick 行を単に消すだけで該当コミットが除外されます。

@berryzplus
Copy link
Contributor Author

d118175, c1b38d1 については revert によりかき消すよりも rebase により丸々削除するのが良いです。

了解です。

rebase はコマンドラインからやっていますか？それとも TortoiseGit にも rebase 機能ある感じですか？

「リベース（ブランチの付け替え）」という機能がありました。
pushのほうはguiからだとうまくいかなかったのでコマンドライン利用です。
git本体は普通のwindows向けgnu gitをインストールしています。(TortoiseGitはGUIのみなので

@berryzplus berryzplus force-pushed the feature/reduce_build_warnings branch from 3a1e21a to 5246ee3 Compare June 4, 2018 11:37
berryzplus added a commit to berryzplus/sakura-editor that referenced this pull request Jun 4, 2018
@berryzplus
ビルド時警告の削減 #48 (rebase2)
5246ee3 
sakura-editor#48
sakura-editor#51
@berryzplus
Copy link
Contributor Author

問題なさそうであればマージお願いします。

@kobake
Copy link
Member

kobake commented Jun 4, 2018 •
edited
Loading

概ね問題なさそうです。 5246ee3 が空コミットになっているので（意図的に残しているものでなければ）消しちゃいましょう。細かくてすみません

@m-tmatma
Copy link
Member

m-tmatma commented Jun 5, 2018

できたら rebase を行った時の手順を wiki に
共有していただきたいです。

wiki に書いてあったら、次にやる人が調べなくて良くなります。

berryzplus added 2 commits June 5, 2018 19:49
@berryzplus
ビルド時の警告削減(Debug版をエディットコンテニューできるようにSAFESEHを外す。)
240dd1c
@berryzplus
ビルド時の警告削減(WindowsSDKにある無名enumのtypedefがvs2017に怒られる対応)
e95b024 
ImageHlp.Hでtypedef enumで無名列挙型を宣言し、代替名を指定していないコードが怒られている。
互換ライブラリのDbgHelp.Hに差し替えてみても同じところで怒られてしまうため、#pragmaで一時的に警告を無効にするように対処した。
@berryzplus berryzplus force-pushed the feature/reduce_build_warnings branch from 5246ee3 to e95b024 Compare June 5, 2018 10:53
@berryzplus
Copy link
Contributor Author

5246ee3 が空コミットになっているので（意図的に残しているものでなければ）消しちゃいましょう。細かくてすみません

@kobake さん、お待たせしました。
わかってない人なので細かい指摘感謝です。
また何かあれがご遠慮なく指摘してください。

できたら rebase を行った時の手順を wiki に
共有していただきたいです。

@m-tmatma さん、手順共有いいですね。
今回はややこしいところがもう再表示されなくなってたので別の機会に作ろうと思います。
プッシュ含めてGUI(TortoiseGit)だけでやれることを確認したので一通りの手順は作れそうです。

kobake
kobake approved these changes Jun 5, 2018
View reviewed changes
Copy link
Member

@kobake kobake left a comment

Choose a reason for hiding this comment

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

LGTMです。お疲れ様でした！

@kobake kobake merged commit d604be1 into sakura-editor:master Jun 5, 2018
@berryzplus berryzplus deleted the feature/reduce_build_warnings branch July 8, 2018 04:13
This was referenced Aug 30, 2018
Closed
Closed
@KENCHjp KENCHjp added the CI appveyor など CI 関連 【ChangeLog除外】 label Dec 5, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
@kobake
Merge pull request sakura-editor#48 from berryzplus/feature/reduce_bu…
79b62e4 
…ild_warnings

ビルド時警告の削減
@berryzplus berryzplus mentioned this pull request Sep 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m-tmatma m-tmatma m-tmatma left review comments

@kobake kobake kobake approved these changes

Assignees
No one assigned
Labels
CI appveyor など CI 関連 【ChangeLog除外】
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@berryzplus @kobake @m-tmatma @KENCHjp