| From 9363886a0a37668976a8765e5d85cf99f72d49f9 Mon Sep 17 00:00:00 2001 |
| From: Andrew Moylan <amoylan@chromium.org> |
| Date: Tue, 9 Jan 2024 23:08:40 +1100 |
| Subject: [PATCH] Revert "Make base::MakeFixedFlatMap consteval" |
| |
| This reverts commit 29d88809787f9a2396be6ab2063bce79ad8dc6ab which is |
| crrev.com/c/5123237. |
| |
| This patch also adds an impl of MakeFixedFlatMapNonConsteval which was |
| introduced in crrev.com/c/5123237. |
| |
| To clean up this backwards compatibility patch |
| 1. All uses of MakeFixedFlatMap that fail to compile without this patch |
| should switch to using MakeFixedFlatMapNonConsteval. A bug should be |
| filed against the code-owning team to find out why that use of |
| MakeFixedFlatMap doesn't have consteval args and update them. |
| 2. This patch can then be removed. |
| 3. Code-owning teams should migrate to MakeFixedFlatMap per the bugs |
| filed in step 1. |
| |
| Change-Id: If769011e50437113db0aa1490ef26a3378bf7718 |
| --- |
| base/containers/fixed_flat_map.h | 57 ++++++++++++++-------- |
| base/containers/fixed_flat_map_unittest.cc | 3 +- |
| 2 files changed, 37 insertions(+), 23 deletions(-) |
| |
| diff --git a/base/containers/fixed_flat_map.h b/base/containers/fixed_flat_map.h |
| index df048f06f1..7f8848dbba 100644 |
| --- a/base/containers/fixed_flat_map.h |
| +++ b/base/containers/fixed_flat_map.h |
| @@ -90,46 +90,61 @@ using fixed_flat_map = base:: |
| flat_map<Key, Mapped, Compare, std::array<std::pair<const Key, Mapped>, N>>; |
| |
| // Utility function to simplify constructing a fixed_flat_map from a fixed list |
| -// of keys and values. Requires that the passed in `data` contains unique keys. |
| -// |
| -// Large inputs will run into compiler limits, e.g. "constexpr evaluation hit |
| -// maximum step limit". In that case, use `MakeFixedFlatMap(sorted_unique)`. |
| +// of keys and values. Requires that the passed in `data` contains unique keys |
| +// and be sorted by key. See `MakeFixedFlatMap` for a variant that sorts the |
| +// input automatically. |
| // |
| // Example usage: |
| // constexpr auto kMap = base::MakeFixedFlatMap<std::string_view, int>( |
| -// {{"foo", 1}, {"bar", 2}, {"baz", 3}}); |
| +// base::sorted_unique, {{"bar", 2}, {"baz", 3}, {"foo", 1}}); |
| template <class Key, class Mapped, size_t N, class Compare = std::less<>> |
| -consteval fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap( |
| +constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap( |
| + sorted_unique_t, |
| std::pair<Key, Mapped> (&&data)[N], |
| const Compare& comp = Compare()) { |
| using FixedFlatMap = fixed_flat_map<Key, Mapped, N, Compare>; |
| typename FixedFlatMap::value_compare value_comp{comp}; |
| - if (!internal::is_sorted_and_unique(data, value_comp)) { |
| - std::sort(data, data + N, value_comp); |
| - // If this CHECK fails, a compiler error will occur because CHECK failure |
| - // is not consteval. Either the provided data wasn't unique, or the |
| - // provided comparison can't establish a strict ordering on it. |
| - CHECK(internal::is_sorted_and_unique(data, 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 FixedFlatMap( |
| sorted_unique, internal::ToArray<typename FixedFlatMap::value_type>(data), |
| comp); |
| } |
| |
| -// Performs sorting at runtime. Do not introduce new calls to this. |
| -// Use MakeFixedFlatMap() or FixedFlatMap() instead. |
| -// https://crbug.com/1513684 |
| +// Utility function to simplify constructing a fixed_flat_map from a fixed list |
| +// of keys and values. Requires that the passed in `data` contains unique keys. |
| +// |
| +// Large inputs will run into compiler limits, e.g. "constexpr evaluation hit |
| +// maximum step limit". In that case, use `MakeFixedFlatMap(sorted_unique)`. |
| +// |
| +// Example usage: |
| +// constexpr auto kMap = base::MakeFixedFlatMap<std::string_view, int>( |
| +// {{"foo", 1}, {"bar", 2}, {"baz", 3}}); |
| template <class Key, class Mapped, size_t N, class Compare = std::less<>> |
| -constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMapNonConsteval( |
| +constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap( |
| std::pair<Key, Mapped> (&&data)[N], |
| const Compare& comp = Compare()) { |
| using FixedFlatMap = fixed_flat_map<Key, Mapped, N, Compare>; |
| typename FixedFlatMap::value_compare value_comp{comp}; |
| std::sort(data, data + N, value_comp); |
| - CHECK(internal::is_sorted_and_unique(data, value_comp)); |
| - return FixedFlatMap( |
| - sorted_unique, internal::ToArray<typename FixedFlatMap::value_type>(data), |
| - comp); |
| + return MakeFixedFlatMap(sorted_unique, std::move(data), comp); |
| +} |
| + |
| +// In a future version of libchrome, MakeFixedFlatMap will become consteval. |
| +// To prepare for this, |
| +// In the short term: Existing uses of MakeFixedFlatMap whose arguments are not |
| +// consteval should migrate to this function (which will be |
| +// present in the future version of libchrome and which will |
| +// remain non-consteval). |
| +// In the long term: After MakeFixedFlatMap has become consteval, this function |
| +// should be marked deprecated and all uses should be updated |
| +// to have consteval args and use MakeFixedFlatMap. |
| +template <class Key, class Mapped, size_t N, class Compare = std::less<>> |
| +constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMapNonConsteval( |
| + std::pair<Key, Mapped> (&&data)[N], |
| + const Compare& comp = Compare()) { |
| + return MakeFixedFlatMap(std::move(data), comp); |
| } |
| |
| } // namespace base |
| diff --git a/base/containers/fixed_flat_map_unittest.cc b/base/containers/fixed_flat_map_unittest.cc |
| index 5c74faa19e..012625b088 100644 |
| --- a/base/containers/fixed_flat_map_unittest.cc |
| +++ b/base/containers/fixed_flat_map_unittest.cc |
| @@ -83,8 +83,7 @@ TEST(FixedFlatMapTest, UnsortableValues) { |
| TEST(FixedFlatMapTest, RepeatedKeys) { |
| // Note: The extra pair of parens is needed to escape the nested commas in the |
| // type list. |
| - // Need to use NonConsteval since CHECK(false) is not constexpr. |
| - EXPECT_CHECK_DEATH((MakeFixedFlatMapNonConsteval<StringPiece, int>( |
| + EXPECT_CHECK_DEATH((MakeFixedFlatMap<StringPiece, int>( |
| {{"foo", 1}, {"bar", 2}, {"foo", 3}}))); |
| } |
| |
| -- |
| 2.43.0.472.g3155946c3a-goog |
| |