- add some test cases for cbegin/cend;
- make class definitions generally follow the order in which they are used;
- add a missing include.
Details
- Reviewers
• Quuxplusone philnik - Group Reviewers
Restricted Project - Commits
- rGe1e17a648962: [libc++][ranges][NFC] Refactor tests for `ranges::{begin,end}`.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM % comments (thanks for the cleanup!) but the merge-conflict comment is significant.
libcxx/test/std/ranges/range.access/begin.pass.cpp | ||
---|---|---|
32–41 | Looks like you had a merge conflict here. Please take what's on the left-hand side. | |
194–197 | FWIW, there's no difference between BeginFunctionReturnsInt and BeginFunctionReturnsEmpty, so I'd be perfectly happy to remove one or the other (and all else being equal, I'd remove BeginFunctionReturnsEmpty because then I could perhaps also remove Empty). | |
libcxx/test/std/ranges/range.access/end.pass.cpp | ||
155 | FWIW, here the main problem is that iterator_t<EmptyEndMember> is ill-formed because Empty is not an iterator type; we never even really get around to checking whether t.end() is well-formed. So I question the usefulness of this test case. But it's harmless. | |
257 | Twice now I've looked at this PR, and twice I've been fooled by this "clever" line — "What? There's no private member here, just two friends!" Consider eliminating the inheritance relationship here, so that this code will more closely resemble BeginFunctionWithPrivateBeginMember above. |
Address feedback and rebase on main.
libcxx/test/std/ranges/range.access/begin.pass.cpp | ||
---|---|---|
32–41 | Thanks! Sorry, yeah, I accidentally overwrote the changes on main. | |
libcxx/test/std/ranges/range.access/end.pass.cpp | ||
155 | I agree. It's a preexistent test case, but I'm quite open to deleting it -- just let me know if you'd prefer to do it in this patch. |
@Quuxplusone I'll go ahead and merge based on the understanding that this patch was LGTM to you modulo the comments I've addressed. My rbegin patch depends on this one and is approved, so I'd rather not let it go stale. If you have any additional feedback, I'll be happy to address in a follow-up. Thanks!
clang-format not found in user’s local PATH; not linting file.