-
Notifications
You must be signed in to change notification settings - Fork 163
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
[WIP] mcpp を使用してCプリプロセッサで無効化された箇所の色分け機能を強化 #1084
Conversation
✅ Build sakura 1.0.2364 completed (commit bfc6cfca64 by @) |
👍 なんですが、マージして「お試し」するには重い変更なのかな?という感じです。 サクラエディタには自前で構文解析を行う機構(アウトライン解析だったかな?)があるので、そこを強化する or 代替実装を提供するというカタチにすると実現しやすいかもです。 experimental ブランチの作成をマジメに考えた方がいいのかも。 |
mcpp を https://github.com/sakura-editor 以下のリポジトリとして追加しても構わなければ、そこに改造版を入れたいです。sakura 本体のリポジトリに mcpp を入れてしまうのはやはり良くないかもしれないですね、mcpp のライセンスは BSD ライセンスで、sakura 側は zlib ライセンスなので、混ぜるとなんかドキュメントの記載等がややこしそうだし…。 アウトライン解析は実行タイミングが限定されているので同じ事の実現は今のままでは無理じゃないかと思います。今回の変更は mcpp 側のコードを少し変更して無効行のフラグをメモリに書き出すようにして、サクラエディタ側の色切り替え処理のバリエーションを追加して対応しています。 本体の字句・構文解析を強化したいとも思うんですがなかなか簡単にはいかなそうです。が、それをやらないと実現出来ない事も色々ありますね。 カーソルが括弧の上にいる時に Ctrl + ] キーを押すと対応する括弧の位置に移動しますが、#ifdef 等のプリプロセッサでも同じように位置を切り替えられたら良いなと思ってます。Visual Studio のエディタだとそれが出来ます。 |
メモリをファイルとして扱う方法を調べてみました。 https://stackoverflow.com/a/50087392/4699324 CreateFile する際に |
✅ Build sakura 1.0.2365 completed (commit f93e9cf0e0 by @beru) |
|
リポジトリの話 => issue 立ててきました。
小ネタ・・・ プログラマが「メモリ」と認識するものは、「物理メモリにマップされた仮想メモリ」という「仮想メモリ」を特殊化した概念です。仮想メモリの作成時に具体的な「マップされるファイルのファイルハンドル」を指定しないと、windowsが「閉じるとき削除する一時ファイル」を作成します。なので、プログラマが「メモリ」と認識するものの実体は、実はファイルです。。。 この辺は boost の inter process communication 周辺が参考になると思います。 |
何を試みてるかが分かれば打開策を提案できるかも・・・。 |
下記のIssueの作成ありがとうございます。
アウトライン解析の実装って 最初はC言語か何かの関数名一覧をリストアップする実装だったんですかね?今は色々な種類の文書に対してのアウトライン解析処理が個別に入ってますね。 実行タイミングが正しい正しくない、については元々のデザインというか仕様ではそのタイミングで行うって決めた事なので、正解というのは無いかもしれません。
レイアウト処理で色分け処理が呼び出されてそこでも各種の字句解析は行われていますが、テキストエディタ用のある程度単純な処理が主だと思います。このPRではそこの仕組みを無理やり使っているようなやり方で実装してます。 このPRでやっている事はアウトライン解析処理を強化するやり方ではなくて、 ただし色分けの開始チェック呼び出しの
無効行フラグをCDocLine にメンバ追加して結び付けるべきかどうかは悩み処ですね。Cプリプロセッサの記述によっては1文字記述を変えるだけで、広範囲の行の有効無効が一気に切り替わってしまう事があり得ます。でも全ての編集操作でそういう事態が生じるわけではないし、結び付けた方が良いかな。。 サクラエディタ本体側の文書を解析する機能を強化していくのかは難しいと思いますが、でもちゃんと本体側を機能強化した方が自然でいびつさが無いですね。例えば無効行のコードに関してアウトライン解析での関数一覧抽出が働くべきかどうかという事もあります。
そういえば折り畳み機能もありましたね。要望のIssueは #824 ですね。
Windows の 32bit 版でもアドレス空間のサイズを超えた大きさのファイルを扱う事は出来たので、その説明はMMR並みに強引ではないでしょうか?
WindowsのどのAPIの事を言っているんでしょうか?仮想メモリの作成って通常は VirtualAlloc で メモリマップは CreateFileMapping や MapViewOfFile ですけれど。
え、それって本当ですか?それはファイルシステムから見えるものですか?
一体どこのページに書かれているんでしょうか? |
このPRでは mcpp を利用していますが、mcpp の入力がファイル名(argc, argv)なので、現在編集している文書を一時ファイルに書き出しているんです。書き出すのに利用している処理が 用途としては一時ファイルなので実際にドライブ上にファイルを作成をしてしまうと無駄が多いので、ドライブには書き出さずにメモリ上だけにデータが置かれているファイルを作成出来ればと思いました。 調べたところ WindowsAPI の CreateFile に FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE を指定する事でなるべくドライブに書き出しがされない事が分かりました。しかしそうして作成したファイルは share する事が出来ないようなので、 まぁこうなったら |
編集の度に mcpp の呼び出しを行っているとファイルの規模が少し大きくなるとレスポンスが悪化するのが体感出来ちゃいます。という事でアウトライン解析みたいにたまに呼び出す分には問題無いと思いますが、文字入力毎に呼び出すのには無理が有りそうです。 考えられる簡易的な対策としては、行位置が変わるような変更やプリプロセッサ関連の記述が変わったタイミングに限定して呼び出す事ですが、、、まぁそもそも mcpp で丸ごとファイルを処理するのを高頻度で呼び出す事に無理があるのかも。。。 内部実装でCプリプロセッサの記述の解析を編集内容に応じた差分処理で行うようにしないと処理時間の問題は解決できないかな。。 |
❌ Build sakura 1.0.2370 failed (commit 194b3ec5f1 by @beru) |
コードはまだ見てないっす(汗; シグニチャ的にmain関数にあたるものを利用してる気がしました。 C言語は比較的新しい言語ので、「入力の抽象化」という概念が存在しています。 CreatePipe関数 でパイプを作って、open_osfhandle で fd 作って _fdopen したら 読み取り側のグルーコードを書いてやれば、パイプ経由でデータを受け渡しできるようになるのでファイルを作らなくてもよくなります。 |
たぶんCOBOL向けじゃないっすかね。
それはもっともな意見だと思います。
Windowsの仮想メモリは、全体の一部を必要なときにコミットして使うようになってます。
windowsのPEローダーはプロセス空間を作るときに仮想メモリの枠組みを使います。
たしか
これっす。 https://www.boost.org/doc/libs/1_63_0/doc/html/interprocess.html |
そこから呼び出しているメインループにあたるのは 現時点で最新のコミット 29d0043 では mcpp 側の記述を更新してファイル入力処理の関数を外部から指定したコールバック関数を使えるようにしました。そして一時ファイルを作らないように変更しました。 具体的には新たに追加した公開関数
http://www.langedge.jp/blog/index.php?itemid=652 に書かれているやり方に近いですね。そのやり方の場合は mcpp 側を改造して、入力ファイルの FILE* を引数とする公開関数を用意しないといけないです。mcppのライブラリとしてのI/Fは色々なケースを考慮して整備されているわけじゃないので、しゃあないですね。 |
おー実務向けですね。しかし今となっては結構力不足感がありますね。C++の複雑な構文とかには対応出来てないし。いやまぁ対応するのが結構無理な感じがありますが…。
正誤というわけではなくて、やりたい事を実現するのに適した枠組みではない、という事ですよね? どこまでの機能を実現したいかを定義して、それを実現させる為にはどういうデザインにするかを決めて I/F を整備して、そこから様々な文書種別に応じた実装を進める、となると長い道のりに見えますね…。
それはファイルの内容をマップする場合にそういう見方も出来るよ的な限定したケースの話ですよね。ファイルの内容をDRAMにキャッシュせずにシーケンシャルに読み書きする使い方も出来るので、そういう場合には メモリ = ファイル という概念的な見方が出来なくなるかなと。。
それはそうなんですけど、それがどうして メモリ = ファイル になるんでしょうか?
仮想メモリの確保ってページ単位で行えるのでその度にファイルシステムへのエントリは作成は出来ないでしょうね。なので下記の内容はちょっと理解出来てません。
いやまぁ再現するプログラムコードがあって動かしたら本当にそうなるなら信じざるを得ませんけれど。。でもそのまえにMicrosoftのサイトのWindowsAPIの解説部分にそういう説明が有る筈。
ぴゃー、長い。 |
このPRでは Cプリプロセッサの解析を mcpp という外部ライブラリに任せるやり方にしてますが、編集操作で毎回文書丸ごとを処理させると、連続操作時のレスポンスが悪くなってしまうので、このやり方は良くなさそうです。 あとタイプ別設定のカラーの色指定のリストに これからどう進めるか悩みどころなので対策案を書き連ねてみました。 対策案0 このままの無理やり実装のまま進める
対策案 1 やっつけ対応
メリット
デメリット
対策案 2 ガチ実装Cプリプロセッサの解析処理を mcpp を参考にして書き起こしサクラエディタ本体に組み込んで、色付けやアウトライン解析を含めて強化する。やりたい事を実現するにあたって正面突破する方法。 メリット
デメリット
|
✅ Build sakura 1.0.2373 completed (commit 4f134c40b4 by @) |
グローバル変数 mcpp_ifdef_false_lines を削除 各行毎のフラグを mcpp_ifdef_false_lines 配列変数に持つのではなく CDocLine::MarkType構造体に m_bExcludedByCPreprocessor メンバーを追加してそこに記録するように変更 CLayoutMgr::DoLayout_Range で CColor_CPreprocessor::Update を呼び出すのは止める。文字入力毎にCプリプロセッサを実行すると重過ぎる為 グローバル変数 g_CColorStrategy_nCurLine を削除 CLayoutMgr::_MakeOneLine において CheckColorMODE を呼び出す前にグローバル変数 g_CColorStrategy_nCurLine を設定していたのは止める。 後述する方法により色変えを出来ると判断 グローバル変数 g_pDocLineDrawing を追加 CEditView::DrawLogicLine で新規追加したグローバル変数 を g_pDocLineDrawing 設定 描画時に SColorStrategyInfo::CheckChangeColor が呼び出される事を利用した仕組み mcpp/system.c の norm_path 関数で正規化したパスが元のファイル名と同じ場合はメモリ解放して NULL を返すように変更 メモリリークへの対処
❌ Build sakura 1.0.2376 failed (commit a2fafd23f1 by @) |
</PropertyGroup> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> | ||
<OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> | ||
<IntDir>$(Platform)\$(Configuration)\</IntDir> | ||
<LinkIncremental>true</LinkIncremental> | ||
<IncludePath>C:\Program Files (x86)\Visual Leak Detector\include;$(IncludePath)</IncludePath> | ||
<LibraryPath>C:\Program Files (x86)\Visual Leak Detector\lib\Win64;$(LibraryPath)</LibraryPath> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
こういうことなんじゃないかな?と思ったり・・・。
<PropertyGroup>
<IncludePath>$(ProgramFiles)\Visual Leak Detector\include;$(IncludePath)</IncludePath>
</PropertyGroup>
<PropertyGroup Condition="'$(Platform)'=='x64'">
<LibraryPath>$(ProgramFiles)\Visual Leak Detector\lib\Win64;$(LibraryPath)</LibraryPath>
</PropertyGroup>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
それは確かに。Win64ビルドでしか確認していないのでこんな設定になっています。。
まぁ本来は Visual Leak Detector に関する変更はコミットから外すべきなのですが…。
しかしこのPRの変更内容が実験的なものになってしまってます。いつかはまともなものにしたい。:cry:
❌ Build sakura 1.0.2377 failed (commit b51da4312e by @beru) |
❌ Build sakura 1.0.2378 failed (commit 7c47cbb139 by @) |
失敗時に mcpp が中断してしまう事が無いようにする為
❌ Build sakura 1.0.2379 failed (commit 5c40efc2d6 by @beru) |
Cプリプロセッサで無効化された複数行のコメントの色分けが適切に行われるように処理追加 行挿入時に隣接行のCプリプロセッサで無効化フラグをコピーする処理追加 CLayoutMgr::_MakeOneLine メソッドで g_pDocLineDrawing を設定する記述追加、ここに入れないと編集時に正しく描画されない
❌ Build sakura 1.0.2380 failed (commit 9c87363744 by @beru) |
PR の目的
Cプリプロセッサで無効化された箇所を色分けして表示する機能を強化するのが目的です。
カテゴリ
PR の背景
C/C++タイプ別設定のカラーのコメントスタイルのブロック型に
#if 0
と#endif
の指定が存在します。しかしこの簡易的な機能ではCプリプロセッサで無効化された箇所の色分けが正確に行えない場合があります。C/C++言語のソースコードをサクラエディタで開いて閲覧したり編集する際に、色分けが不正確だとどの部分のコードが有効化/無効化されているのかを把握するのが大変なので改善を行いたいです。
PR のメリット
プリプロセッサの色分け表示が以前より正確になります。
PR のデメリット (トレードオフとかあれば)
mcpp
というライブラリをプロジェクトに取り込む形になっているmcpp
にはメモリリークする不具合があります。繰り返し呼び出す使い方を想定していなかったのかもしれません。mcpp
の処理を呼び出すので負荷が大きいmcpp
側の入力がファイルな事もあり、やっつけ実装にしています。-I
で指定してmcpp
を実行しているので、相対パスの #include は働きます。g_CColorStrategy_nCurLine
を追加しており、実装方法が汚いPR の影響範囲
タイプ別設定のカラーの色指定リストの一番末尾に
Cプリプロセッサ
の項目が追加されます。プリプロセッサで無効化された箇所の色を指定する事が出来ます。
プリプロセッサで無効化されていない箇所の色については指定出来ません。
レイアウト処理
PR のデメリットに色々と記載しましたが、Cプリプロセッサを色分け表示する設定にすると、レイアウト処理の度に現在編集している文書の内容を一時ファイルに書き出し mcpp ライブラリを呼び出しています。
関連チケット
#413
参考資料
http://mcpp.sourceforge.net/index-jp.html
https://www.ipa.go.jp/files/000005716.pdf