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

開発環境(Visual Studio)の機能をもっと活用するためにコード改善を行いたい #872

Closed
berryzplus opened this issue Apr 28, 2019 · 6 comments · Fixed by #1053
Labels
refactoring リファクタリング 【ChangeLog除外】

Comments

@berryzplus
Copy link
Contributor

前置き

サクラエディタは、1998年ごろから存在するアプリです。
この名前になったのは2001年ごろで 元々はテキストエディタだったはずだ! というツッコミもあるかと思いますが、まー結構古くからあるアプリです。

ちょうど1年くらい前 @kobakeさんが突然「GitHubに移行します!」と宣言したのが現在に至るここのプロジェクトの流れなのですが、その時から サクラエディタの標準開発環境は vs2017 ということになっています。

「なっています」というちょっとアレな言い方にしているのは、
vs2017でビルドできるvs2017での開発に対応している は違う
と思っているからです。

目的

サクラエディタを vs2017 での開発に対応させる。

提案内容

開発環境の諸元(≒細かい設定)をできるだけ vs2017 の標準設定に合わせ、vc++が本来もっている様々な機能を活用できるようにしたいです。

現状の sakura.vcxproj は VC6 時代から脈々と受け継がれてきたもので、vs2017 でビルドできるようにメンテされたものです。

気にしているのは、最近のvc++の更新で導入されたオプションが無効になっていることなんです。ここ数年の vc++ の変更は c++ 規格への準拠度を高める変更であるように思っています。最近の更新で追加されたオプションが無効になっているということは、つまりvc++に搭載されたC++規格への準拠強化機能が使えないってことだと思っています。

具体的にどんなオプションが無効になってるか?を表にまとめてみました。

オプション 機能 説明
/SDL SDLチェック セキュリティチェック機構を強化するオプションです。C4996(古い形式の関数を利用している)などの「よく見る警告」がビルドエラーになります。
/std C++規格バージョンの指定 vs2017はC++17規格にも対応していますが、この指定をしないと C++14 相当の言語機能までしか扱えません。(この項目のデフォルトは「指定なし」なので現状のままでもよいですが、このオプションの存在を知らんと規格準拠モードの理解が難しいと思うのであえて載せます。)
/permissive- 規格準拠モード C++規格に準拠しないコードを検出したらエラーにする機能です。この機能を有効にするには、かなり新しいWindows SDK(win10 Fall Creator以降)を使う必要があります。この機能を有効にできるならGCCコンパイルによるビルドチェックの重要度が下がるのでVC++一本化への道が開けると思っています。
/Ob2 関数のインライン展開(最適化) 標準ではヘッダに書かれたクラスのメソッドは自動的にインライン化されます。現在の設定だと inlineを付けない限りインライン展開されないようになっています。Release版にだけ影響するオプションですが、変えたいです。

適用時期

なるはや。(なるべく早く:smile:)

/SDL/permissive-の適用には コード変更が必要 なんですが、
実質的に同じことをチェックする機能なので変更内容は「丸被り」になります。
別々にPR出すと、何がどっちの変更なのかよく分らない結果になると思います。

いったんは こういう目的でプロジェクト設定を変更していくよ、そのためには一部コードに変更が必要だよ。 という情報を共有しとくのがこのissueを立てた意図です。

とくに反対意見が出なければ、上記4つのオプションを変更するPRを一括で出そうと思っています。

@beru
Copy link
Contributor

beru commented Apr 29, 2019

上記4つのオプションを変更するPRを一括で出そうと思っています。

一括で対処するのは止した方が良いと思います。

/SDL オプションを付けてビルドすると 792 個のエラーが出るので、これの対処を全て行うと差分の量が結構多くなると思います。

その他の変更はそこまで差分が多く出ないと思うので、分けた方が良いかなと思います。

@beru
Copy link
Contributor

beru commented Aug 23, 2019

このPRのタイトル「開発環境をvs2017に対応させる」だけを読んでいると何だか混乱するところがあって、開発環境って統合開発環境のVisualStudioの事?DEをVS2017に対応させる??

前置きには vs2017での開発に対応している と書かれているので、おそらく VS2017 以降で標準、もしくは推奨されているビルド設定で問題無くビルドが行えるようにする 的な事なんでしょうか?

@beru
Copy link
Contributor

beru commented Aug 23, 2019

単なる主観的な表現上の問題なのかもしれないですが「VS2017で標準/推奨の設定でビルドが通らないとVS2017に対応しているとは言えない」 とまでは自分は思いません。そこまで厳密に捉えないでも…と。

ただ修飾語を追加して「VS2017で標準/推奨の設定でビルドが通らないと十分にVS2017に対応しているとは言えない」という文章ならば、まぁそういう見方もあるもんかな…、と思います。

つまり言葉尻を捉える重箱の隅をつつく日本語警察の魔の手から逃れるには、少し表現を丸める事で受け流しやすいようにすると良いのかもしれません。

@berryzplus
Copy link
Contributor Author

やりたいことのニュアンスが伝わりやすいようにタイトルを変えてみます。

@berryzplus berryzplus changed the title 開発環境をvs2017に対応させる 開発環境(Visual Studio)の機能をもっと活用するためにコード改善を行いたい Aug 23, 2019
@berryzplus
Copy link
Contributor Author

before) 開発環境をvs2017に対応させる
after) 開発環境(Visual Studio)の機能をもっと活用するためにコード改善を行いたい

いま、サクラエディタのプロジェクトが vs2017 でビルドできるのは、ぼくらが頑張ったからでしょうか?

違うような気がします。

作業の成果や人の功績は正当に評価されるべきだと考えています。
「単にビルドできるようにする」を実現するためにぼくらが行う作業はあまりにも少ないと思います。
少ない作業で移行が完了できるようになってるのはマイクロソフトのVC++チームの功績です。

ぼくらが「vs2017対応を完了しました(キリッ」と言えるためには、
ただビルドできるの上を狙っていく必要がある気がします。

本文でも書きましたが、せっかく搭載されているのに使えていない機能がいくつかあります。
いま使えていない機能を使えるようにして開発環境をもっと活用しよう!
そのために行う作業を一言で説明すると何か?・・・
「対応させる」という言葉が一番しっくり来たのでこれを使っていました。

変更後のほうが意図が分かりやすいタイトルになったと思っています。

@beru
Copy link
Contributor

beru commented Aug 23, 2019

言われてみて、対応って幅広い言葉になりうるんだなぁと再認識させられました。

「USB Type-C 対応」と謳ってるけど実は規格にちゃんと適合していない機器…のような怪しい状態から脱したい、というのに近いんでしょうか…。。

実利的なのかどうかちょっと判断付きかねますが、細かいことの積み重ねが大事なのかもしれないですね。。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants