Skip to content

Commit

Permalink
Convert base::clamp to std::clamp under the hood.
Browse files Browse the repository at this point in the history
Bug: 1373621
Change-Id: I1dde975f08c195f50b92a7adc0b497823096ff27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3946347
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098352}
  • Loading branch information
pkasting authored and Chromium LUCI CQ committed Jan 29, 2023
1 parent 6963bb6 commit d174d2f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 31 deletions.
22 changes: 4 additions & 18 deletions base/cxx17_backports.h
Expand Up @@ -5,27 +5,13 @@
#ifndef BASE_CXX17_BACKPORTS_H_
#define BASE_CXX17_BACKPORTS_H_

#include <functional>

#include "base/check.h"
#include <algorithm>

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 <typename T, typename Compare>
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 <typename T>
constexpr const T& clamp(const T& v, const T& lo, const T& hi) {
return base::clamp(v, lo, hi, std::less<T>{});
}
// TODO(crbug.com/1373621): Rewrite all uses of base::clamp as std::clamp and
// remove this file.
using std::clamp;

} // namespace base

Expand Down
35 changes: 22 additions & 13 deletions base/cxx17_backports_unittest.cc
Expand Up @@ -4,15 +4,8 @@

#include "base/cxx17_backports.h"

#include <array>
#include <memory>
#include <tuple>
#include <type_traits>
#include <utility>
#include <vector>

#include "base/test/gtest_util.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace base {
Expand All @@ -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));
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions ui/gfx/skia_color_space_util.cc
Expand Up @@ -8,6 +8,7 @@
#include <cmath>
#include <vector>

#include "base/check.h"
#include "base/cxx17_backports.h"

namespace gfx {
Expand Down

0 comments on commit d174d2f

Please sign in to comment.