[libcxx-commits] [PATCH] D101922: [libcxx][iterator] adds `std::ranges::advance`

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/__iterator/primitives.h:99
+  template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
+  [[nodiscard]] constexpr iter_difference_t<_Ip> operator()(_Ip& __i, iter_difference_t<_Ip> __n, _Sp __bound) const {
+    _LIBCPP_ASSERT(__n >= 0 || (bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>),
----------------
cjdb wrote:
> zoecarver wrote:
> > cjdb wrote:
> > > zoecarver wrote:
> > > > This is a value we likely actually do want to discard.
> > > Please elaborate?
> > If we did the thing I wish we did in C++ and functions were nodiscard by default, there would be a `[[discardable_result]]` attribute instead. 
> > 
> > If that were the case, this function would be one of the ones that's marked with `[[discardable_result]]`, because there are valid reasons to use this function and discard the result. That's probably the most common use, in fact. 
> > 
> > In other words, this function is one of the ones where we don't actually need the nodiscard attribute. 
> > In other words, this function is one of the ones where we don't actually need the nodiscard attribute.
> 
> What I meant was "please elaborate on why you feel this function's result should be considered discardable".
Because it has side effects: advancing the iterator. It's possible, and even likely, that users would only care about the side effects. One example of this is in the implementation of `ranges::next` another is in my `drop_view` patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101922