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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added a comment.
This revision now requires changes to proceed.

In D141238#4036364 <https://reviews.llvm.org/D141238#4036364>, @jloser wrote:

> @philnik mind if I land this?  CI isn't green yet due to unrelated issues.
>
> 1. https://reviews.llvm.org/harbormaster/unit/view/5710279/ is fixed with https://reviews.llvm.org/D141284
>
>
>
> 1. https://reviews.llvm.org/harbormaster/unit/view/5710280/ is a separate issue — module map problem?

Given that I've found a small issue I think it would be better to rebase and have a green CI. The second failure is also a problem with includes. I'll fix it myself.



================
Comment at: libcxx/test/support/test_iterators.h:720
+
+  constexpr std::iter_reference_t<Base> operator*() const { return *it_; }
+
----------------
@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)`?


================
Comment at: libcxx/test/support/test_iterators.h:700
 
+#  if TEST_STD_VER > 17
+
----------------
jloser wrote:
> philnik wrote:
> > 
> Just switched to using `>= 20`.  Did we decide on using `>=` always going forward?  I thought there was a discussion within the last 6 months or so, but I forget and am used to the old-school way.
Yes, we decided to use `>=`. Not sure when exactly it was, but I think was within this release cycle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141238