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

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added inline comments.


================
Comment at: libcxx/include/syncstream:201
+
+    static _VSTD::unordered_map<streambuf_type*, mutex_ptr_type>  __mtx_ptr_map;
+};
----------------
As a static member variable - when does this get initialized? On first use? before `main`? Does it get initialized if the user never instantiates a `basic_syncbuf`? 


================
Comment at: libcxx/include/syncstream:208
+template <class _CharT, class _Traits, class _Allocator>
+_VSTD::unordered_map<
+    typename basic_syncbuf<_CharT, _Traits, _Allocator>::streambuf_type*,
----------------
Why `unordered_map` as opposed to `map`? Does `std::hash< streambuf_type*>` have some behavior that you want?  [ Note: `unordered_map` might be the right choice. But you should say //why// it is the right choice somewhere. ]


================
Comment at: libcxx/include/syncstream:279
+    bool __result = true;
+    mutex_ptr_type __mtx = __mtx_ptr_map[__wrapped];
+
----------------
This adds a default constructed `mutex_ptr_type` to the map if it doesn't already exist.
I suspect that's not what you wanted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67086