[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