[libc++] Fix complexity guarantee in ranges::clamp by ldionne · Pull Request #68413 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-libcxx

Changes

This patch prevents us from calling the projection more than 3 times in std::clamp, as required by the Standard.

Fixes #64717

2 Files Affected:

(modified) libcxx/include/__algorithm/ranges_clamp.h (+3-2)
(modified) libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp (+29-20)

diff --git a/libcxx/include/__algorithm/ranges_clamp.h b/libcxx/include/__algorithm/ranges_clamp.h index 9613f7f37720a6c..e6c86207254a19f 100644 --- a/libcxx/include/__algorithm/ranges_clamp.h +++ b/libcxx/include/__algorithm/ranges_clamp.h @@ -37,9 +37,10 @@ struct __fn { _LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(comp, std::invoke(proj, high), std::invoke(proj, low))), "Bad bounds passed to std::ranges::clamp"); - if (std::invoke(comp, std::invoke(proj, value), std::invoke(proj, low))) + auto&& projected = std::invoke(proj, value); + if (std::invoke(comp, std::forward<decltype(projected)>(projected), std::invoke(proj, low))) return low; - else if (std::invoke(comp, std::invoke(proj, high), std::invoke(proj, value))) + else if (std::invoke(comp, std::invoke(proj, high), std::forward<decltype(projected)>(projected))) return high; else return __value; diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp index 036552f75de4eb4..35ef55a91e243d4 100644 --- a/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp +++ b/libcxx/test/std/algorithms/alg.sorting/alg.clamp/ranges.clamp.pass.cpp @@ -38,6 +38,16 @@ static_assert(!HasClamp); static_assert(!HasClamp<int, NoComp>); static_assert(!HasClamp<int, std::ranges::less, CreateNoComp>); +struct EnsureValueCategoryComp { + constexpr bool operator()(const int&& x, const int&& y) const { return x < y; } + constexpr bool operator()(const int&& x, int& y) const { return x < y; } + constexpr bool operator()(int& x, const int&& y) const { return x < y; } + constexpr bool operator()(int& x, int& y) const { return x < y; } + constexpr bool operator()(std::same_as<const int&> auto&& x, std::same_as<const int&> auto&& y) const { + return x < y; + } +}; + constexpr bool test() { { // low < val < high int val = 2; @@ -71,6 +81,7 @@ constexpr bool test() { constexpr const int& lvalue_proj() const { return i; } constexpr int prvalue_proj() const { return i; } + constexpr bool operator==(S const& other) const { return i == other.i; } }; struct Comp { @@ -82,31 +93,29 @@ constexpr bool test() { auto low = S{20}; auto high = S{30}; // Check that the value category of the projection return type is preserved. - assert(&std::ranges::clamp(val, low, high, Comp{}, &S::lvalue_proj) == &low); - assert(&std::ranges::clamp(val, high, low, Comp{}, &S::prvalue_proj) == &low); + assert(std::ranges::clamp(val, low, high, Comp{}, &S::lvalue_proj) == low); + assert(std::ranges::clamp(val, high, low, Comp{}, &S::prvalue_proj) == low); } - { // Check that the implementation doesn't cause double moves (which could result from calling the projection on - // `value` once and then forwarding the result into the comparator). - struct CheckDoubleMove { - int i; - bool moved = false;

 constexpr explicit CheckDoubleMove(int set_i) : i(set_i) {}

 constexpr CheckDoubleMove(const CheckDoubleMove&) = default;

 constexpr CheckDoubleMove(CheckDoubleMove&& rhs) noexcept : i(rhs.i) {

{ // Ensure that we respect the value category of the projection when calling the comparator.
// This additional example was provided by Tim Song in https://github.com/microsoft/STL/issues/3970#issuecomment-1685120958.
struct MoveProj {

 constexpr int const&& operator()(int const& x) const { return std::move(x); }

};

static_assert(std::indirect_strict_weak_order<EnsureValueCategoryComp, std::projected<const int*, MoveProj>>);

auto moving_comp = [](CheckDoubleMove lhs, CheckDoubleMove rhs) { return lhs.i < rhs.i; };
auto prvalue_proj = [](const CheckDoubleMove& x) -> CheckDoubleMove { return x; };
assert(&std::ranges::clamp(val, low, high, moving_comp, prvalue_proj) == &val);

assert(std::ranges::clamp(0, 1, 2, EnsureValueCategoryComp{}, MoveProj{}) == 1);
}
{ // Make sure we don't call the projection more than three times per [alg.clamp], see #64717
int counter = 0;
auto projection_function = [&counter](const int value) -> int {
```
 counter++;
```
```
 return value;
```
};
assert(std::ranges::clamp(3, 2, 4, std::ranges::less{}, projection_function) == 3);
assert(counter <= 3);

}

return true;