[libcxx-commits] [PATCH] D92790: [libc++][P0053R7] Add <syncstream> header and implement std::basic_syncbuf

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius added a comment.

FYI, I've just landed D92656 <https://reviews.llvm.org/D92656>, so now you can use `utils/generate_header_tests.py` to generate (some) tests for a new header. You might need to modify the script to add condition on `_LIBCPP_HAS_NO_THREADS`. Anyway, what you did on tests looks good.
Also, notes are now at https://libcxx.llvm.org/docs/Contributing.html.

Just a few comments for the moment, I'll take time to review it a bit later.



================
Comment at: libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.no_new_in_libcxx.abilist:35-43
+{'is_defined': False, 'name': '_ZTVSt9exception', 'size': 0, 'type': 'OBJECT'}
 {'is_defined': False, 'name': '_ZdaPv', 'type': 'FUNC'}
 {'is_defined': False, 'name': '_ZdaPvSt11align_val_t', 'type': 'FUNC'}
 {'is_defined': False, 'name': '_ZdlPv', 'type': 'FUNC'}
+{'is_defined': False, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'}
 {'is_defined': False, 'name': '_Znam', 'type': 'FUNC'}
 {'is_defined': False, 'name': '_ZnamSt11align_val_t', 'type': 'FUNC'}
----------------
abrachet wrote:
> Still not sure why these are here though.
It all depends on your environment and link flags. Once CI gets triggered you'd be able to get updated ABI lists from artifacts on failed build steps (in Buildkite). You'll **probably** need to remove these unrelated changes (exception, align_val_t).


================
Comment at: libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.no_new_in_libcxx.abilist:558-571
+{'is_defined': True, 'name': '_ZNSt3__110adopt_lockE', 'size': 64, 'type': 'OBJECT'}
+{'is_defined': True, 'name': '_ZNSt3__110ctype_base5alnumE', 'size': 64, 'type': 'OBJECT'}
+{'is_defined': True, 'name': '_ZNSt3__110ctype_base5alphaE', 'size': 64, 'type': 'OBJECT'}
+{'is_defined': True, 'name': '_ZNSt3__110ctype_base5blankE', 'size': 64, 'type': 'OBJECT'}
+{'is_defined': True, 'name': '_ZNSt3__110ctype_base5cntrlE', 'size': 64, 'type': 'OBJECT'}
+{'is_defined': True, 'name': '_ZNSt3__110ctype_base5digitE', 'size': 64, 'type': 'OBJECT'}
+{'is_defined': True, 'name': '_ZNSt3__110ctype_base5graphE', 'size': 64, 'type': 'OBJECT'}
----------------
abrachet wrote:
> mclow.lists wrote:
> > abrachet wrote:
> > > Not sure why `make generate-cxx-abilist` touched these, does anyone have any ideas? `LLVM_USE_SANITIZER` perhaps?
> > If these things have changed sizes, that's an ABI break.
> Looks like it was asan which did this.
Just to help reviewing, please mark addressed comments as "Done".


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

https://reviews.llvm.org/D92790