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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/syncstream:226
+basic_syncbuf<_CharT, _Traits, _Allocator>::~basic_syncbuf()
+{
+    emit();
----------------
mclow.lists wrote:
> What's the expected behavior here if `emit` throws an exception?
> 
> Nothing. If an exception is thrown from emit(), the destructor catches and ignores that exception.



================
Comment at: libcxx/include/syncstream:317
+    {
+        if (emit() == false) return -1;
+    }
----------------
mclow.lists wrote:
> We don't usually test `== true` or `== false`.
> How about `if (!emit()) return -1;` instead?
> Or even `if (__emit_on_sync && !emit()) return -1;`
I like the verbosity of saying `== false`. I'll update it, though. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67086