error_result_to_message関数で返ってくる文字列がnull終端文字列になってしまっている

[qwerty2501]

内容

C APIとの兼ね合いで voicevox_core crate にある error_result_to_message がnull終端文字列を返す実装になってしまっている

null終端文字列をなくしたいが、そうしてしまうと C API側の voicevox_error_result_to_message の実装に支障が出てしまい、かと言って error_result_to_message の実装をC API側に持っていってしまうとこんどは内部エラー型の一部共通化ができなくなってしまう

できることならエラーメッセージの共通化を実現しつつ、 null終端文字列を付け加える処理のみをC API側に持っていくような感じの実装にしたい

Pros

null終端文字列というCの仕様に依存した実装を純粋なRust実装から取り除くことができる

Cons

具体的な改善実装案がない(macroでやる？)

[Hiroshiba]

ちょっと設計的にきれいなのか自信がないですが、error_result_to_messageはRust用にして、C API用に何かしら別のラッパー関数error_result_to_message_cみたいなのを作ってそこでnull終端文字列を足す、みたいな手もあるかもと思いました！

[qwerty2501]

error.rsのbase_error_message のような実装に error_result_to_message を修正する感じでしょうか？
できなくはないと思いますが、Rust実装側にCの言語仕様に依存する実装が入ってしまうのはどうなんだろうと思いました。
いまはないですが、将来的にRust実装を crates.io に公開する可能性もあるので error_result_to_message_c がpublicな状態で見えてしまうのはよくなさそうだなと思いました。

[qwerty2501]

macro使用するのに抵抗があるのであれば、エラーメッセージの一部共通化をあきらめて別々に実装してしまうのも手かもしれませんね。
もしくはすでにErrorからVoicevoxResultCodeへの変換の際に標準エラーにエラーメッセージを出力しているのでそもそも error_result_to_message を削ってしまうのも選択肢に入るかもしれません。

[Hiroshiba]

macro実装は特に抵抗ないです！
ラッパー関数作るほうがきれいかなと直感的に思っただけだったので、どちらでもという感じです！

[qryxip]

マクロ実装って、voicevox_core::error_message!(VOICEVOC_RESULT_LOAD_MODEL_ERROR)が"modelデータ読み込み中にOnnxruntimeエラーが発生しました"に展開され、_c_api側でそれをconcat!するようなのをイメージしてますがそんな感じですか?

[qwerty2501]

@qryxip そうですね。そんな感じです。
あとmacroにするとRust実装側にはresult code のmessage変換関数は不要かもです。

[qryxip]

このissueにラベルを付けました (マクロ実装でやることにはみんな同意してそうなので)

[qryxip]

#553 をやってて思ったのですが、Python APIやJava APIが増えた今、C APIのvoicevox_error_result_to_messageとErrorのDisplayとで文言を無理に合わせる必要は無いんじゃないかと最近思っています。

かと言って error_result_to_message の実装をC API側に持っていってしまうとこんどは内部エラー型の一部共通化ができなくなってしまう

の「共通化」(base_error_message)はもう要らないんじゃないかと。

base_error_messageは #126 (議論は #126 (comment))からある仕組みではあるんですが、エラー周りに手を加えることがあればそのときに解体しようかなと思っています。

[Hiroshiba]

それもそうだなと思いました。エラーメッセージはわかりやすいほうが良いけど、constだとどうしても限界ありますし。

C APIとか用のconstエラーメッセージとそれ以外用のエラーメッセージをRust API内で両方定義しておけば、コピペの手間を一番削減できるかもとか思いました。
（エラー追加が大変になると歪なコードになってきそうな気がしたので簡単にしておくと良いのかな〜と）