[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.

@devin.jeanpierre re `__is_default_allocator`: You should take a look at https://github.com/Quuxplusone/llvm-project/compare/main...trivially-relocatable and specifically https://github.com/Quuxplusone/llvm-project/commit/dfbfc8b326e0f5a3bcc284e3a28893310b30d0bb , because that's the current source of truth for https://p1144.godbolt.org/ . That already includes some stuff like the memmove bugfix and the switch to `__is_default_allocator`.



================
Comment at: libcxx/test/libcxx/utilities/meta/meta.unary.prop/is_trivially_relocatable.pass.cpp:90
+  static_assert(std::__libcpp_is_trivially_relocatable<TrivialMoveOnly>::value, "");
+  static_assert(!std::__libcpp_is_trivially_relocatable<const TrivialMoveOnly>::value, "");
+#endif
----------------
I would say that in a perfect world `const TrivialMoveOnly` ought to be trivially relocatable, because `TrivialMoveOnly` is a trivially relocatable type, and if you're moving-and-destroying it, then you don't really care that the source object was const-qualified.
However, I see that even under p1144r5, `is_relocatable_v<const TrivialMoveOnly>` would be `false`, and I did not intend that `is_trivially_relocatable_v<T> && !is_relocatable_v<T>` should ever be true for any `T`. So I'll have to change this part of p1144. Most library users, then, should probably use `is_trivially_relocatable<remove_cvref_t<T>>` for their purposes.

Relevant question for this PR: Should we permit the libc++ extension type `std::vector<const X>` to actually `push_back` and reallocate when `is_trivially_relocatable_v<X>`? In the past that was obviously impossible, but now it's kinda possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119385