[libcxx-commits] [PATCH] D119385: Use trivial relocation operations in std::vector, by porting D67524 and part of D61761 to work on top of the changes in D114732.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

- I think `UNSUPPORTED: c++98, c++03` should be just `UNSUPPORTED: c++03`; we don't seem to use `c++98` anywhere else in libcxx/test/ except two `REQUIRES:`es that should probably also be fixed.
- I think you're selling `std::swap` short; it's used by `rotate` and `sort` and all kinds of cool algorithms that would be nice to speed up. We should figure out how to get `std::swap` to bitwise-swap trivially relocatable things, either by solving the problems or by decreeing that they shall not matter. :)
- Credit-wise, I don't care; mention me in the commit message (as you've done) and that seems fine.
- Might make sense to split out the addition of `__libcpp_is_trivially_relocatable` into its own PR, and then add the vector optimization as a child PR of that one, as I did with the D50119 <https://reviews.llvm.org/D50119> patch series. However, it could also be argued that you shouldn't add `__libcpp_is_trivially_relocatable` without any consumers of it. So I'm ambivalent, and would wait to see what others think. (Similarly, there's the question of how to land it: one commit or two? If we end up having to revert it unexpectedly, should we have the option to revert just the vector optimization, or should we force ourselves to revert //all// of it to avoid leaving `__libcpp_is_trivially_relocatable` as a loose end with no consumers?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119385