[libcxx-commits] [PATCH] D67086: Implement syncstream (p0053)

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added a comment.

Maybe I'm missing something, but I don't see any synchronization between two syncstreams wrapping the same stream.

Like this:

  ofstream f ("foo.txt");
  fire_off_a_thread_with(basic_osyncstream(&f));
  fire_off_a_thread_with(basic_osyncstream(&f));

How does the output to f get synchronized between the two threads?



================
Comment at: libcxx/include/syncstream:102
+        // [syncstream.osyncstream.members], member functions
+        void emit();
+        streambuf_type* get_wrapped() const noexcept;
----------------
Arrgh. Peter!  Two functions with the same name, one returns `bool` the other `void`.
Not your problem, though.



================
Comment at: libcxx/include/syncstream:226
+basic_syncbuf<_CharT, _Traits, _Allocator>::~basic_syncbuf()
+{
+    emit();
----------------
What's the expected behavior here if `emit` throws an exception?



================
Comment at: libcxx/include/syncstream:317
+    {
+        if (emit() == false) return -1;
+    }
----------------
We don't usually test `== true` or `== false`.
How about `if (!emit()) return -1;` instead?
Or even `if (__emit_on_sync && !emit()) return -1;`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67086/new/

https://reviews.llvm.org/D67086