[libcxx-commits] [PATCH] D115766: [libc++] [test] Simplify (sized_)sentinel_wrapper; use it in more places.

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Dec 15 12:31:17 PST 2021


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

This is a really nice cleanup, however I would like the drive-by refactorings to be split into a different NFC patch.



================
Comment at: libcxx/test/std/ranges/range.adaptors/range.take/sentinel/ctor.pass.cpp:22
 #include "test_macros.h"
 #include "test_iterators.h"
 #include "../types.h"
----------------
Same here, please keep for the other NFC refactoring patch.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp:37
+    ASSERT_SAME_TYPE(decltype(end.base()), std::ranges::sentinel_t<ForwardView>);
+    assert(base(end.base()) == globalBuff + 8);
+    static_assert(!HasConstQualifiedEnd<TransformView>);
----------------
Same here, please keep for a separate patch.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp:41
+    std::same_as<cpp20_input_iterator<int *>> auto it = std::move(begin).base();
+    assert(base(it) == globalBuff);
   }
----------------
This is the only change that actually pertains to this patch IMO, i.e. calling `base(it)` instead of `it.base()`. I like the rest of the refactoring you're doing, but can you please keep it for a separate patch?


================
Comment at: libcxx/test/support/test_iterators.h:851
 public:
     sentinel_wrapper() = default;
+    constexpr explicit sentinel_wrapper(const It& it) : base_(base(it)) {}
----------------



================
Comment at: libcxx/test/support/test_iterators.h:854
+    constexpr bool operator==(const It& other) const { return base_ == base(other); }
+    friend constexpr auto base(const sentinel_wrapper& s) { return It(s.base_); }
 private:
----------------



================
Comment at: libcxx/test/support/test_iterators.h:858-859
-
-    constexpr const I& base() const& { return base_; }
-    constexpr I base() && { return std::move(base_); }
-
----------------
Let's not make this change in this patch -- let's do it separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115766



More information about the libcxx-commits mailing list