Skip to content

[libc++][ranges] Avoid using distance in ranges::contains_subrange #87155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

Either std::distance or ranges::distance are inefficient for non-sized ranges. Also, calculation the range in int type is seriously problematic.

This patch avoids using distance and calculation of the length of non-sized ranges.

Fixes #86833.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner March 30, 2024 13:52
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 30, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 30, 2024

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

Either std::distance or ranges::distance are inefficient for non-sized ranges. Also, calculation the range in int type is seriously problematic.

This patch avoids using distance and calculation of the length of non-sized ranges.

Fixes #86833.


Full diff: https://github.com/llvm/llvm-project/pull/87155.diff

2 Files Affected:

  • (modified) libcxx/include/__algorithm/ranges_contains_subrange.h (+4-6)
  • (modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains_subrange.pass.cpp (+4)
diff --git a/libcxx/include/__algorithm/ranges_contains_subrange.h b/libcxx/include/__algorithm/ranges_contains_subrange.h
index 4cd03cbb537060..e4b1c78a4f282f 100644
--- a/libcxx/include/__algorithm/ranges_contains_subrange.h
+++ b/libcxx/include/__algorithm/ranges_contains_subrange.h
@@ -15,7 +15,6 @@
 #include <__functional/ranges_operations.h>
 #include <__functional/reference_wrapper.h>
 #include <__iterator/concepts.h>
-#include <__iterator/distance.h>
 #include <__iterator/indirectly_comparable.h>
 #include <__iterator/projected.h>
 #include <__ranges/access.h>
@@ -70,14 +69,13 @@ struct __fn {
     requires indirectly_comparable<iterator_t<_Range1>, iterator_t<_Range2>, _Pred, _Proj1, _Proj2>
   _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr bool static
   operator()(_Range1&& __range1, _Range2&& __range2, _Pred __pred = {}, _Proj1 __proj1 = {}, _Proj2 __proj2 = {}) {
-    auto __n2 = 0;
     if constexpr (sized_range<_Range2>) {
-      __n2 = ranges::size(__range2);
+      if (ranges::size(__range2) == 0)
+        return true;
     } else {
-      __n2 = std::distance(cbegin(__range2), cend(__range2));
+      if (ranges::begin(__range2) == ranges::end(__range2))
+        return true;
     }
-    if (__n2 == 0)
-      return true;
 
     auto __ret = ranges::search(__range1, __range2, __pred, std::ref(__proj1), std::ref(__proj2));
     return __ret.empty() == false;
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains_subrange.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains_subrange.pass.cpp
index d48ee9e4e7e02e..761691c2afdcb9 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains_subrange.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains_subrange.pass.cpp
@@ -309,6 +309,10 @@ constexpr bool test() {
     });
   });
 
+  assert(std::ranges::contains_subrange(
+      std::views::iota(0, 5), std::views::iota(0, 5) | std::views::filter([](int) { return true; })));
+  assert(!std::ranges::contains_subrange(std::views::iota(0ULL, 42ULL), std::views::iota(0ULL, 1ULL << 32)));
+
   return true;
 }
 

@frederick-vs-ja frederick-vs-ja force-pushed the contains_subrange_empty branch from 0ff37d9 to 88ef5c8 Compare March 30, 2024 14:09
@frederick-vs-ja frederick-vs-ja changed the title [libc++][ranges] Avoid using std::distance in ranges::contains_subrange [libc++][ranges] Avoid using distance in ranges::contains_subrange Mar 30, 2024
…range`

Either `std::distance` or `ranges::distance` are inefficient for
non-sized ranges. Also, calculation the range in `int` type is seriously
problematic.

This patch avoids using `distance` and calculation of the length of
non-sized ranges.
@frederick-vs-ja frederick-vs-ja force-pushed the contains_subrange_empty branch from 88ef5c8 to 629a4fa Compare March 30, 2024 14:34
@var-const var-const self-assigned this Apr 1, 2024
@var-const var-const added the ranges Issues related to `<ranges>` label Apr 1, 2024
Copy link
Member

@var-const var-const left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

@var-const var-const merged commit 04dbf7a into llvm:main Apr 3, 2024
@frederick-vs-ja frederick-vs-ja deleted the contains_subrange_empty branch April 3, 2024 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. ranges Issues related to `<ranges>`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<algorithm>: contains_subrange should not use std::distance
3 participants