[libcxx-commits] [PATCH] D150831: [libc++] Implement ranges::ends_with

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_ends_with.h:43
+      _Sent2 __last2,
+      _Pred __pred   = {},
+      _Proj1 __proj1 = {},
----------------
This helper function doesn't need default arguments (since it's never invoked outside this file) and can take the predicate and the projections by reference (so there's no need to wrap the arguments passed to it in `std::ref` -- `std::ref` is only needed when passing those arguments to `ranges::equal`).


================
Comment at: libcxx/include/__algorithm/ranges_ends_with.h:59
+      return false;
+    std::advance(__first1, (N1 - N2));
+    auto __unwrapped1 = std::__unwrap_range(std::move(__first1), std::move(__last1));
----------------
var-const wrote:
> var-const wrote:
> > philnik wrote:
> > > philnik wrote:
> > > > 
> > > Potential optimization: If they are bidirectional and a common range we can step through back to front instead of iterating over the whole range.
> > Looks like it's still `std::advance`.
> Still seems not done?
Friendly ping on this -- this sounds like a good optimization to implement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150831