[libcxx-commits] [libcxx] [libc++] Introduce one-sided binary search for set_intersection so that it can be completed in sub-linear time for a large class of inputs. (PR #75230)

Iuri Chaer via libcxx-commits libcxx-commits at lists.llvm.org
Sun Mar 24 08:58:47 PDT 2024


================
@@ -85,6 +86,56 @@ struct _IterOps<_ClassicAlgPolicy> {
     std::advance(__iter, __count);
   }
 
+  // advance with sentinel, a la std::ranges::advance
+  // it's unclear whether _Iter has a difference_type and whether that's signed, so we play it safe:
----------------
ichaer wrote:

I think maybe I understand where you're coming from, are you concerned about this not being a faithful version of `std::advance()`, and the similarity confusing others into making mistakes? When I wrote it like that, I was thinking that having a less strict fallback for the function when operating in `_ClassicAlgPolicy` would be a net positive, because it allows us to preserve pre-existing behaviour when using it, but I think I understand your point of view. Personally, I would probably leave it as it is (it's targeted at libc++ implementors and the whole thing will be inlined anyway), but if you disagree, I have two alternatives I'd like to suggest:
1. Rename this to something like `advance_with_custom_difference_type()` so that the intent is explicit, and add a comment explaining that its goal is to avoid introducing or enforcing requirements in legacy code. That way we avoid breaking existing user code without introducing a potential source of confusion.
2. Enforce the `LegacyInputIterator` requirement. If we go down this path, I would like to add a `static_assert()` at the entry point for `std::set_intersection()`, to help users identify the source of the new failure.

What do you think?

https://github.com/llvm/llvm-project/pull/75230


More information about the libcxx-commits mailing list