[libcxx-commits] [libcxx] [libc++] Implement P0429R9 `std::flat_map` (PR #98643)

[Louis Dionne via libcxx-commits]

================
@@ -0,0 +1,110 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// <flat_map>
+
+// flat_map& operator=(flat_map&&);
+
+#include <algorithm>
+#include <deque>
+#include <flat_map>
+#include <functional>
+#include <memory_resource>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "test_macros.h"
+#include "MoveOnly.h"
+#include "../../../test_compare.h"
+#include "test_allocator.h"
+#include "min_allocator.h"
+
+int main(int, char**) {
+  {
+    using C  = test_less<int>;
+    using A1 = test_allocator<int>;
+    using A2 = test_allocator<char>;
+    using M  = std::flat_map<int, char, C, std::vector<int, A1>, std::vector<char, A2>>;
+    M mo     = M({{1, 1}, {2, 3}, {3, 2}}, C(5), A1(7));
+    M m      = M({}, C(3), A1(7));
+    m        = std::move(mo);
+    assert((m == M{{1, 1}, {2, 3}, {3, 2}}));
+    assert(m.key_comp() == C(5));
+    auto [ks, vs] = std::move(m).extract();
+    assert(ks.get_allocator() == A1(7));
+    assert(vs.get_allocator() == A2(7));
+    assert(mo.empty());
+  }
+  {
+    using C  = test_less<int>;
+    using A1 = other_allocator<int>;
+    using A2 = other_allocator<char>;
+    using M  = std::flat_map<int, char, C, std::deque<int, A1>, std::deque<char, A2>>;
+    M mo     = M({{4, 5}, {5, 4}}, C(5), A1(7));
+    M m      = M({{1, 1}, {2, 2}, {3, 3}, {4, 4}}, C(3), A1(7));
+    m        = std::move(mo);
+    assert((m == M{{4, 5}, {5, 4}}));
+    assert(m.key_comp() == C(5));
+    auto [ks, vs] = std::move(m).extract();
+    assert(ks.get_allocator() == A1(7));
+    assert(vs.get_allocator() == A2(7));
+    assert(mo.empty());
+  }
+  {
+    using A = min_allocator<int>;
+    using M = std::flat_map<int, int, std::greater<int>, std::vector<int, A>, std::vector<int, A>>;
+    M mo    = M({{5, 1}, {4, 2}, {3, 3}}, A());
+    M m     = M({{4, 4}, {3, 3}, {2, 2}, {1, 1}}, A());
+    m       = std::move(mo);
+    assert((m == M{{5, 1}, {4, 2}, {3, 3}}));
+    auto [ks, vs] = std::move(m).extract();
+    assert(ks.get_allocator() == A());
+    assert(vs.get_allocator() == A());
+    assert(mo.empty());
+  }
+  {
+    // A moved-from flat_map maintains its class invariant in the presence of moved-from elements.
+    using M =
+        std::flat_map<std::pmr::string, int, std::less<>, std::pmr::vector<std::pmr::string>, std::pmr::vector<int>>;
+    std::pmr::monotonic_buffer_resource mr1;
+    std::pmr::monotonic_buffer_resource mr2;
+    M mo = M({{"short", 1},
+              {"very long string that definitely won't fit in the SSO buffer and therefore becomes empty on move", 2}},
+             &mr1);
+    M m  = M({{"don't care", 3}}, &mr2);
+    m    = std::move(mo);
+    assert(m.size() == 2);
+    assert(std::is_sorted(m.begin(), m.end(), m.value_comp()));
+    assert(m.begin()->first.get_allocator().resource() == &mr2);
+
+    assert(std::is_sorted(mo.begin(), mo.end(), mo.value_comp()));
+    mo.insert({"foo", 1});
+    assert(mo.begin()->first.get_allocator().resource() == &mr1);
+  }
+  {
+    // A moved-from flat_map maintains its class invariant in the presence of moved-from comparators.
----------------
ldionne wrote:

I think the intent of this test was to test that we maintain the invariant when we have a moved-from comparator. But that test is incorrectly checking that, because the comparator fits into the small buffer of the `std::function`, leading to it being trivially movable.

If we fixed this test to check what it intends to check, I would expect it to fail because we don't maintain the invariant (can you check that by using a very large comparator instead?). Something like

```
struct Large : std::less<int> {
  char foo[99999];
};
```

Assuming the explanation above is correct, this should crash cause we don't maintain the invariant.

That being said, instead of trying to fix that test and maintain the invariant, I think we should move the comparator and not attempt to maintain that invariant. A moved-from object can be destroyed or assigned to in the general case, and I'm not aware of anything in the spec that would require that we do more than that for `flat_map`.

https://github.com/llvm/llvm-project/pull/98643