[base] Only Compare Keys in MakeFixedFlatMap

This change modifies base::MakeFixedFlatMap to only compare keys. This
fixes a bug where repeated keys wouldn't be detected if their values
differed. Furthermore, this also now supports values that are not
comparable.

Bug: 682254
Change-Id: I598406dee0fe47a4601a122369bc3d88de520abc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2549622
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830169}

base/containers/fixed_flat_map.h

@@ -98,12 +98,14 @@ template <class Key, class Mapped, size_t N, class Compare = std::less<>>
 constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap(
     std::pair<Key, Mapped>(&&data)[N],
     const Compare& comp = Compare()) {
-  internal::InsertionSort(data, data + N, comp);
-  CHECK(internal::is_sorted_and_unique(data, comp));
+  using FixedFlatMap = fixed_flat_map<Key, Mapped, N, Compare>;
+  typename FixedFlatMap::value_compare value_comp(comp);
+  internal::InsertionSort(data, data + N, value_comp);
+  CHECK(internal::is_sorted_and_unique(data, value_comp));
   // Specify the value_type explicitly to ensure that the returned array has
   // immutable keys.
-  return fixed_flat_map<Key, Mapped, N, Compare>(
-      sorted_unique, internal::ToArray<std::pair<const Key, Mapped>>(data),
+  return FixedFlatMap(
+      sorted_unique, internal::ToArray<typename FixedFlatMap::value_type>(data),
       comp);
 }
 

base/containers/fixed_flat_map_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/ranges/algorithm.h"
 #include "base/strings/string_piece.h"
+#include "base/test/gtest_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -16,6 +17,23 @@ using ::testing::Pair;
 
 namespace base {
 
+namespace {
+
+struct Unsortable {
+  int value;
+};
+
+bool operator==(const Unsortable& lhs, const Unsortable& rhs) {
+  return lhs.value == rhs.value;
+}
+
+bool operator<(const Unsortable& lhs, const Unsortable& rhs) = delete;
+bool operator<=(const Unsortable& lhs, const Unsortable& rhs) = delete;
+bool operator>(const Unsortable& lhs, const Unsortable& rhs) = delete;
+bool operator>=(const Unsortable& lhs, const Unsortable& rhs) = delete;
+
+}  // namespace
+
 TEST(FixedFlatMapTest, MakeFixedFlatMap_SortedInput) {
   constexpr auto kSquares =
       MakeFixedFlatMap<int, int>({{1, 1}, {2, 4}, {3, 9}, {4, 16}});
@@ -45,4 +63,28 @@ TEST(FixedFlatMapTest, MutableValues) {
   EXPECT_THAT(map, ElementsAre(Pair("bar", 2), Pair("foo", 2)));
 }
 
+// Tests that even though the values are unsortable the built in sort still
+// correctly orders the keys.
+TEST(FixedFlatMapTest, UnsortableValues) {
+  using PairType = std::pair<int, Unsortable>;
+
+  constexpr auto kSquares = MakeFixedFlatMap<int, Unsortable>({
+      {4, {16}},
+      {3, {9}},
+      {2, {4}},
+      {1, {1}},
+  });
+  EXPECT_THAT(kSquares, ElementsAre(PairType(1, {1}), PairType(2, {4}),
+                                    PairType(3, {9}), PairType(4, {16})));
+}
+
+// Verifies that passing repeated keys to MakeFixedFlatMap results in a CHECK
+// failure.
+TEST(FixedFlatMapTest, RepeatedKeys) {
+  // Note: The extra pair of parens is needed to escape the nested commas in the
+  // type list.
+  EXPECT_CHECK_DEATH((MakeFixedFlatMap<StringPiece, int>(
+      {{"foo", 1}, {"bar", 2}, {"foo", 3}})));
+}
+
 }  // namespace base

base/containers/fixed_flat_set_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/ranges/algorithm.h"
 #include "base/strings/string_piece.h"
+#include "base/test/gtest_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -27,4 +28,10 @@ TEST(FixedFlatSetTest, MakeFixedFlatSet_UnsortedInput) {
   EXPECT_THAT(kSet, ::testing::ElementsAre("bar", "baz", "foo"));
 }
 
+// Verifies that passing repeated keys to MakeFixedFlatSet results in a CHECK
+// failure.
+TEST(FixedFlatSetTest, RepeatedKeys) {
+  EXPECT_CHECK_DEATH(MakeFixedFlatSet<int>({1, 2, 3, 1}));
+}
+
 }  // namespace base