Closed onihusube closed 1 year ago
@onihusube 4818ff7a040a83267ebc5ee506299e44ea6195a3 時点の記事を見て気になったところのお知らせです。
// 先頭バイトの状態によって適切なオブジェクトがStream::read()内で構築されている
if (buffer[0] == FOO) {
process_foo(reinterpret_cast<Foo*>(buffer.get())); // ✅ ok
オブジェクトが構築されるというところまではいいんですが、その構築された オブジェクトを指すポインタを配列先頭要素を指す buffer.get() からの reinterpret_cast だけで得られるという記述が確認できないので、ここは 厳密には std::launder() が必要になっちゃうものと考えています。 expr.reinterpret.cast#7, expr.static.cast#13, basic.compound#note-4
このへんの解決には P1839 Accessing Object Representations が期待される ところですが、最新版 P1839R5 の Known issues を見るに解決はまだ遠そうでした。
同様の問題が↓の箇所にも見られます。
// newbufにはT[]のオブジェクトが生存期間内にあるため、ポインタT*をイテレータとして使用可能となる
// ここで、T[]の要素のTのオブジェクトが構築される(明示的)
std::uninitialized_copy(begin(), end(), (T*)newbuf); // #a ✅ ok
::operator delete(buf, std::align_val_t(alignof(T)));
// newbufにはchar[]のオブジェクトが生存期間内にあるため、newbuf(char*)をイテレータとして使用可能となる
buf_end_size = newbuf + sizeof(T) * size(); // #b ✅ ok
(別件になりますが、 size() で buf が使用されるので operator delete より先にしないとマズそう。)
P0593 の解説記事としては、これら未解決と思われる例を除いても単体で解決 された問題は残るので、そこだけ残す形にしておくのがいいんじゃないかなー と思いました。記事の文量的にもそっちのほうが読みやすそうな気もします。
書いていて思ったのですが
「動的配列の実装」の例は複数のオブジェクトが同時に構築されている(っぽい)のでその適切なオブジェクトへのポインタを取得するにはstd::launder()
が必要はおそらくそうだと思います。
「バイト配列の読み込み」の例は先頭バイトの状態によって適切なオブジェクトが1つ構築されるとのことなので、new
式内operator new()
はFoo/Bar
どちらかのオブジェクトを暗黙に構築した上でそれへのポインタを適切に返していて、process()
内ではstd::unique_ptr
への格納などによる変換でchar
配列ポインタになっているものをreinterpret_cast
で元のポインタに戻しているだけなので、戻されたポインタはoperator new()
が返した暗黙に構築されたオブジェクトへのポインタとして有効になるのでは?
と考えました。なんかそんな気がするというふわっとしたものですし問題が複雑すぎて正しさがわかりませんが・・・
@onihusube
...new式内operator new()はFoo/Barどちらかのオブジェクトを暗黙に構築した上でそれへのポインタを適切に返していて...
new 式が返し unique_ptr に格納されているポインタは reinterpret_cast の前に
buffer[0] == FOO
で char オブジェクトへのアクセスに使われているため、
その解釈はできないと思ってます。
@onihusube 16e1d9768ef71370827c7cdb4a2a6c7d338d4914 を見て気付いたことのお知らせです。 std::launder() を加えて修正する方向となってましたが、 P0593 の変更について理解するために std::launder() の理解も必要となって 記事の難解さがレベルアップしちゃってそうで、やはり削れるところは削って 済ませたほうがよいのでは・・・と思ってます。
::operator delete(buf, std::align_val_t(alignof(T))); + buf = newbuf; buf_end_size = newbuf + sizeof(T) * size(); // #b 💀 UB
size() は (T*)buf_end_size - (T*)buf
を行うので、この処理順もマズそうです。
size() を先に変数に取るか、古い buf を変数にとって delete を最後にするか、でしょうか。
- process_foo(reinterpret_cast<Foo*>(buffer.get())); // ✅ ok + process_foo(std::launder<Foo>(buffer.get())); // ✅ ok
std::launder() は reinterpret_cast を含まないので以下のような記述になります。 この他の箇所も同様です。
process_foo(std::launder(reinterpret_cast<Foo*>(buffer.get()))); // ✅ ok
+...`reinterpret_cast`はポインタの変換のみを行うため、この場合に
文章が途切れていました。(気持ちはわかる気がします。)
+ T *end() { return std::launder<T>(buf_end_size); }
残念ながら std::launder() の要件として引数の指すアドレスに適切な型で 生存期間内にあるオブジェクトが置かれていることがあるので、 現状 past-the-end なポインタを得るのには使うと要件違反になってしまいそうでした。 ( p1839r5 での reinterpret_cast の拡張についても似た問題が Known issue として挙がっていて、こちらも意図された制限ではなさそうですけども。)
書いてる人がこういう間違いを犯している時点でもうおっしゃる通りという感じですね・・・
とりあえず一通り完了しました。何かツッコミどころがあれば・・・
@onihusube 「問題となる他の例」のうちの2つ「バイト配列の読み込み」「動的配列の実装」は P0593R6 適用後もまだ厳密には未定義動作のままで「この変更によって解決される例」に含めるのは 正しくないもの思っているので、これら2つの例は文書から削除してしまうのがいいだろうと思ってました。 そうはいかなかったでしょうか?
削除はできず何かしらの理由で必要となる場合は、たとえば上記の箇所を 「この変更によって改善される例である。」とするなど、さらなる変更を加えて誤りではない形に 持って行くことはできるかもしれないとは思います。 (「動的配列の実装」は char で持ってるところを T で持つようにしておけばいけそうな・・・。)
私としては
と思っているので、削るのは避けたいと考えています。
例というかサンプルコードは基本的に提案から持ってきているので、問題とならないように書き換えるのはありかもしれません。
@onihusube なるほど確かに。となると、これらの例の有用性を維持しながら 未定義動作を回避できる例としても成立するようにできたらいいなという感じで変更を 考えてみようと思います。
すぐできるとは言えないので、こちらのタスク issue は他に問題が無ければ クローズできるものとして、僕からの変更は用意できたときに PR とさせてもらおうと思います。
了解しました、お願いします🙏
他何もなければ、しばらく後クローズします
やります