[libcxx-commits] [PATCH] D61761: P1144 "Trivially relocatable" (1/3): is_trivially_relocatable, relocate_at, and uninitialized_relocate

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Context for other reviewers:

I spoke to Arthur at C++Now after his talk, and my opinion was that it would be useful to implement the trivially relocatable proposal as a conforming extension to libc++ to allow gathering some usage experience. However, I am dead against making any change that is user visible. So what I would like is for us to implement the traits and algorithms but use reserved identifiers -- this way we can implement optimizations inside the library without changing the user-visible API. And if the proposal never makes it into C++, we could even remove everything that has to do with trivially relocatable since it was only an internal optimization anyway -- that's the spirit.

Y'all know I'm against extensions, however I think gathering usage experience is useful and this is an opportunity to do so without making user-visible changes, which is one case where I think extensions are easy to sell. Please LMK if you disagree, otherwise I told Arthur I would help shepherd the patch through (the libc++ part).

> - hide names? how?

Use `__libcpp_xxx` or whatever scheme you like. Those are implementation details anyway, we can change them as we see fit.

> - any reason to gate the new names on any particular `_LIBCPP_STD_VER`, such as 17? (I think not.)

Good question. On the one hand, you want to get as much usage experience as possible, so that would mean enabling it on the oldest possible standard. On the other hand, once (if) the proposal makes it into C++, we won't want to implement the functionality in older standards. But removing the optimizations from C++03 will cause regressions, which is bad.

> - split up the tests for `uninitialized_relocate{,_n}` into three or four files each?

The tests look reasonable to me as-is. However, they should all be in `test/libcxx/` because they are libc++ specific.

> - write a test for `relocate_at`

Please do.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D61761