[libcxx-commits] [PATCH] D141238: [libc++][test] Move `common_input_iterator` to `test_iterators.h`

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/test/support/test_iterators.h:720
+
+  constexpr std::iter_reference_t<Base> operator*() const { return *it_; }
+
----------------
jloser wrote:
> philnik wrote:
> > @huixie90 This is different in the two versions we have. Is `std::iter_reference_t<Base>` correct here, or should we just use `decltype(auto)`?
> Thanks for calling this out explicitly.  I noticed this and stuck with `std::iter_reference_t<Base>` rather than `decltype(auto)` that @CaseyCarter fixed in https://reviews.llvm.org/D141216.  I think using `iter_reference_t` is fine, but I'll let others chime in here.
Actually, based on the rationale in https://reviews.llvm.org/D141216, it seems the problem is when the template type parameter `Base` is const-qualified, we'd actually need to use `std::iter_const_reference_t` and not `std::iter_reference_t`.  Given that, it's easier to just always use `decltype(auto)` and let the deduction happen for us.  

But I'm not sure we have a call site or test mandating that behavior, but we should change it to use `decltype(auto)` AFAICT now. Is that right, @CaseyCarter @huixie90?  In any case, I just changed it to use `decltype(auto`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141238