[libcxx-commits] [PATCH] D124079: [libc++] Implement ranges::find_end, ranges::search{, _n}

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_search.h:68
+      if (__size2 <= 0)
+        return {__first1, __first1};
+      if constexpr (sized_range<_Range1>) {
----------------
var-const wrote:
> philnik wrote:
> > var-const wrote:
> > > Hmm, I'd like to double-check this. IIUC, the relevant section is
> > > 
> > > > `{i, i + (last2 - first2)}`, where `i` is the first iterator in the range `[first1, last1 - (last2 - first2))` such that for every non-negative integer `n` less than `last2 - first2` the condition
> > > `bool(invoke(pred, invoke(proj1, *(i + n)), invoke(proj2, *(first2 + n))))` is `true`.
> > > 
> > > However, if `last2 - first2` is zero, there exists no non-negative integer `n` less than that. It seems like it would mean that no such iterator `i` can exist and thus the algorithm should go to the next clause and return `{last1, last1}`.
> > Yes, the non-negative integers less than zero is the empty set. Normally "all of an empty set" is trivially true.
> Hmm, I might be missing something (I wrote the original comment a while ago), but it seems that if `last2 - first2 == 0`, the current implementation would return `{first1, first1}`, whereas it should return `{last1, last1}` (assuming I'm correct that the "if no such iterator exists" cause applies). `first1` isn't necessarily equal to `last1` in this case, so returning `{first1, first1}` isn't equivalent to returning `{last1, last1}`, even if both are empty sets.
I think you are wrong about the condition. IIUC the condition applies for every non-negative integer, because there are no non-negative integers less than zero. It's the same way that `∀x∈∅. ⊥` is true.


================
Comment at: libcxx/include/__iterator/advance.h:201
+#if _LIBCPP_STD_VER > 17
+  ranges::advance(__iter, __to);
+#else
----------------
huixie90 wrote:
> What if this line is called on C++20 mode with the classic algorithm/classic iterator?
> 
> C++20 `sentinel_for` requires the sentinel to be `semiregular`. But C++ 17's iterator are not required to be default constructible.
> 
> what happens if you call the classic algorithm with a classic iterator that is not default constructible?
Oops, this shouldn't have been here anymore.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124079/new/

https://reviews.llvm.org/D124079