[libcxx-commits] [PATCH] D119112: [libc++] Implement P1165R1 (Make stateful allocator propagation more consistent)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

We need to update the status page, and also please add a release note!



================
Comment at: libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/allocator_propagation.pass.cpp:10
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+#include <cassert>
----------------
Add:

```
// This test ensures that we properly propagate allocators, per https://wg21.link/p1165r1
```


================
Comment at: libcxx/test/std/strings/basic.string/string.nonmembers/string_op+/allocator_propagation.pass.cpp:44
+
+bool test() {
+  using S = std::basic_string<char, std::char_traits<char>, soccc_allocator<char>>;
----------------
Can we template that on `CharT` and run the test for both `char` and `wchar_t` (if `!defined(TEST_HAS_NO_WIDE_CHARACTERS)`)? The reason I'm asking is that we explicitly instantiate some `string` methods in the dylib for `char`, but not for `wchar_t`. We expect the behavior for `char` and `wchar_t` to be the same, however I could imagine a case where they don't match (for example in back-deployment cases for `char`, since `operator+` is in the dylib). Adding `wchar_t` will help shake out these potential differences.

Incidentally, I don't understand why this test passes on Apple backdeployment, since I would expect it to use `operator+` in the dylib, which doesn't do SOCCC properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119112