From d174d2f65ba34906160e867776a0a5b258ede247 Mon Sep 17 00:00:00 2001 From: Peter Kasting Date: Sun, 29 Jan 2023 06:32:00 +0000 Subject: [PATCH] Convert base::clamp to std::clamp under the hood. Bug: 1373621 Change-Id: I1dde975f08c195f50b92a7adc0b497823096ff27 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3946347 Commit-Queue: Peter Kasting Reviewed-by: Nico Weber Cr-Commit-Position: refs/heads/main@{#1098352} --- base/cxx17_backports.h | 22 ++++---------------- base/cxx17_backports_unittest.cc | 35 ++++++++++++++++++++------------ ui/gfx/skia_color_space_util.cc | 1 + 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/base/cxx17_backports.h b/base/cxx17_backports.h index c37e4fdd477a4..bbcce2789d113 100644 --- a/base/cxx17_backports.h +++ b/base/cxx17_backports.h @@ -5,27 +5,13 @@ #ifndef BASE_CXX17_BACKPORTS_H_ #define BASE_CXX17_BACKPORTS_H_ -#include - -#include "base/check.h" +#include namespace base { -// C++14 implementation of C++17's std::clamp(): -// https://en.cppreference.com/w/cpp/algorithm/clamp -// Please note that the C++ spec makes it undefined behavior to call std::clamp -// with a value of `lo` that compares greater than the value of `hi`. This -// implementation uses a CHECK to enforce this as a hard restriction. -template -constexpr const T& clamp(const T& v, const T& lo, const T& hi, Compare comp) { - CHECK(!comp(hi, lo)); - return comp(v, lo) ? lo : comp(hi, v) ? hi : v; -} - -template -constexpr const T& clamp(const T& v, const T& lo, const T& hi) { - return base::clamp(v, lo, hi, std::less{}); -} +// TODO(crbug.com/1373621): Rewrite all uses of base::clamp as std::clamp and +// remove this file. +using std::clamp; } // namespace base diff --git a/base/cxx17_backports_unittest.cc b/base/cxx17_backports_unittest.cc index 5e6401a4c7424..c0c73219c45a7 100644 --- a/base/cxx17_backports_unittest.cc +++ b/base/cxx17_backports_unittest.cc @@ -4,15 +4,8 @@ #include "base/cxx17_backports.h" -#include -#include #include -#include -#include -#include -#include "base/test/gtest_util.h" -#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace base { @@ -38,7 +31,7 @@ bool operator==(const AnotherType& lhs, const AnotherType& rhs) { return lhs.some_other_int == rhs.some_other_int; } -TEST(Cxx17BackportTest, Clamp) { +TEST(ClampTest, Behavior) { EXPECT_EQ(0, base::clamp(-5, 0, 10)); EXPECT_EQ(0, base::clamp(0, 0, 10)); EXPECT_EQ(3, base::clamp(3, 0, 10)); @@ -89,12 +82,28 @@ TEST(Cxx17BackportTest, Clamp) { EXPECT_EQ(another_type_10, base::clamp(another_type_15, another_type_0, another_type_10, compare_another_type)); +} + +TEST(ClampTest, Death) { + EXPECT_DEATH_IF_SUPPORTED(std::ignore = base::clamp(3, 10, 0), ""); + EXPECT_DEATH_IF_SUPPORTED(std::ignore = base::clamp(3.0, 10.0, 0.0), ""); + + OneType one_type_0{0}; + OneType one_type_3{3}; + OneType one_type_10{10}; + AnotherType another_type_0{0}; + AnotherType another_type_3{3}; + AnotherType another_type_10{10}; + auto compare_another_type = [](const auto& lhs, const auto& rhs) { + return lhs.some_other_int < rhs.some_other_int; + }; - EXPECT_CHECK_DEATH(base::clamp(3, 10, 0)); - EXPECT_CHECK_DEATH(base::clamp(3.0, 10.0, 0.0)); - EXPECT_CHECK_DEATH(base::clamp(one_type_3, one_type_10, one_type_0)); - EXPECT_CHECK_DEATH(base::clamp(another_type_3, another_type_10, - another_type_0, compare_another_type)); + EXPECT_DEATH_IF_SUPPORTED( + std::ignore = base::clamp(one_type_3, one_type_10, one_type_0), ""); + EXPECT_DEATH_IF_SUPPORTED( + std::ignore = base::clamp(another_type_3, another_type_10, another_type_0, + compare_another_type), + ""); } } // namespace diff --git a/ui/gfx/skia_color_space_util.cc b/ui/gfx/skia_color_space_util.cc index 0bae7399b14d8..ab04f0a094be4 100644 --- a/ui/gfx/skia_color_space_util.cc +++ b/ui/gfx/skia_color_space_util.cc @@ -8,6 +8,7 @@ #include #include +#include "base/check.h" #include "base/cxx17_backports.h" namespace gfx {