Unverified Commit 02540b2f authored by Louis Dionne's avatar Louis Dionne Committed by GitHub
Browse files

[libc++] Make sure ranges algorithms and views handle boolean-testable correctly (#69378) 

Before this patch, we would fail to implicitly convert the result of
predicates to bool, which means we'd potentially perform a copy or move
construction of the boolean-testable, which isn't allowed. The same
holds true for comparing iterators against sentinels, which is allowed
to return a boolean-testable type.

We already had tests aiming to ensure correct handling of these types,
but they failed to provide appropriate coverage in several cases due to
guaranteed RVO. This patch fixes the tests, adds tests for missing
algorithms and views, and fixes the actual problems in the code.

Fixes #69074
parent d34a10a4

libcxx/include/__algorithm/make_projected.h

+1 −1
Original line number Diff line number Diff line
@@ -96,7 +96,7 @@ decltype(auto) __make_projected_comp(_Comp& __comp, _Proj1& __proj1, _Proj2& __p 
    return __comp;



  } else {

    return [&](auto&& __lhs, auto&& __rhs) {

    return [&](auto&& __lhs, auto&& __rhs) -> bool {

      return std::invoke(__comp,

                        std::invoke(__proj1, std::forward<decltype(__lhs)>(__lhs)),

                        std::invoke(__proj2, std::forward<decltype(__rhs)>(__rhs)));

libcxx/include/__algorithm/ranges_find_if_not.h

+2 −2
Original line number Diff line number Diff line
@@ -39,14 +39,14 @@ struct __fn { 
            indirect_unary_predicate<projected<_Ip, _Proj>> _Pred>

  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Ip

  operator()(_Ip __first, _Sp __last, _Pred __pred, _Proj __proj = {}) const {

    auto __pred2 = [&](auto&& __e) { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };

    auto __pred2 = [&](auto&& __e) -> bool { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };

    return ranges::__find_if_impl(std::move(__first), std::move(__last), __pred2, __proj);

  }



  template <input_range _Rp, class _Proj = identity, indirect_unary_predicate<projected<iterator_t<_Rp>, _Proj>> _Pred>

  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Rp>

  operator()(_Rp&& __r, _Pred __pred, _Proj __proj = {}) const {

    auto __pred2 = [&](auto&& __e) { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };

    auto __pred2 = [&](auto&& __e) -> bool { return !std::invoke(__pred, std::forward<decltype(__e)>(__e)); };

    return ranges::__find_if_impl(ranges::begin(__r), ranges::end(__r), __pred2, __proj);

  }

};

libcxx/include/__algorithm/ranges_max.h

+4 −2
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ struct __fn { 
  operator()(initializer_list<_Tp> __il, _Comp __comp = {}, _Proj __proj = {}) const {

    _LIBCPP_ASSERT_UNCATEGORIZED(__il.begin() != __il.end(), "initializer_list must contain at least one element");



    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };

    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };

    return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp_lhs_rhs_swapped, __proj);

  }
@@ -72,7 +72,9 @@ struct __fn { 
    _LIBCPP_ASSERT_UNCATEGORIZED(__first != __last, "range must contain at least one element");



    if constexpr (forward_range<_Rp> && !__is_cheap_to_copy<range_value_t<_Rp>>) {

      auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };

      auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool {

        return std::invoke(__comp, __rhs, __lhs);

      };

      return *ranges::__min_element_impl(std::move(__first), std::move(__last), __comp_lhs_rhs_swapped, __proj);

    } else {

      range_value_t<_Rp> __result = *__first;

libcxx/include/__algorithm/ranges_max_element.h

+2 −2
Original line number Diff line number Diff line
@@ -37,7 +37,7 @@ struct __fn { 
            indirect_strict_weak_order<projected<_Ip, _Proj>> _Comp = ranges::less>

  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Ip

  operator()(_Ip __first, _Sp __last, _Comp __comp = {}, _Proj __proj = {}) const {

    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };

    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };

    return ranges::__min_element_impl(__first, __last, __comp_lhs_rhs_swapped, __proj);

  }
@@ -46,7 +46,7 @@ struct __fn { 
            indirect_strict_weak_order<projected<iterator_t<_Rp>, _Proj>> _Comp = ranges::less>

  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_iterator_t<_Rp>

  operator()(_Rp&& __r, _Comp __comp = {}, _Proj __proj = {}) const {

    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };

    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) -> bool { return std::invoke(__comp, __rhs, __lhs); };

    return ranges::__min_element_impl(ranges::begin(__r), ranges::end(__r), __comp_lhs_rhs_swapped, __proj);

  }

};

libcxx/include/__algorithm/ranges_remove.h

+2 −2
Original line number Diff line number Diff line
@@ -36,7 +36,7 @@ struct __fn { 
    requires indirect_binary_predicate<ranges::equal_to, projected<_Iter, _Proj>, const _Type*>

  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr subrange<_Iter>

  operator()(_Iter __first, _Sent __last, const _Type& __value, _Proj __proj = {}) const {

    auto __pred = [&](auto&& __other) { return __value == __other; };

    auto __pred = [&](auto&& __other) -> bool { return __value == __other; };

    return ranges::__remove_if_impl(std::move(__first), std::move(__last), __pred, __proj);

  }
@@ -45,7 +45,7 @@ struct __fn { 
             indirect_binary_predicate<ranges::equal_to, projected<iterator_t<_Range>, _Proj>, const _Type*>

  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr borrowed_subrange_t<_Range>

  operator()(_Range&& __range, const _Type& __value, _Proj __proj = {}) const {

    auto __pred = [&](auto&& __other) { return __value == __other; };

    auto __pred = [&](auto&& __other) -> bool { return __value == __other; };

    return ranges::__remove_if_impl(ranges::begin(__range), ranges::end(__range), __pred, __proj);

  }

};