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

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision as: var-const.
var-const added a comment.

LGTM with one remaining question.



================
Comment at: libcxx/include/__algorithm/ranges_search.h:68
+      if (__size2 <= 0)
+        return {__first1, __first1};
+      if constexpr (sized_range<_Range1>) {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124079