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

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added a comment.

A general comment about the tests: You've got a bunch of methods marked "noexcept", but nowhere to I see `ASSERT_NOEXCEPT` or `ASSERT_NOT_NOEXCEPT` in the tests to check that.



================
Comment at: libcxx/include/iosfwd:86
 
 template <class state> class fpos;
 typedef fpos<char_traits<char>::state_type>    streampos;
----------------
You need to update the synopsis as well.


================
Comment at: libcxx/include/syncstream:225
+inline _LIBCPP_INLINE_VISIBILITY
+basic_syncbuf<_CharT, _Traits, _Allocator>::~basic_syncbuf()
+{
----------------
These one or two line functions can be just put inline.
```
    _LIBCPP_INLINE_VISIBILITY
    ~basic_syncbuf() { emit(); }
```



================
Comment at: libcxx/include/syncstream:425
+{
+    try {
+        emit();
----------------
These need to be wrapped in:
```
#ifndef _LIBCPP_NO_EXCEPTIONS
        try
        {
#endif  // _LIBCPP_NO_EXCEPTIONS
        emit();
#ifndef _LIBCPP_NO_EXCEPTIONS
        } catch(...) { }
#endif  // _LIBCPP_NO_EXCEPTIONS
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67086