This introduces forward declarations of string_view and span to
keep drop_view.h as miminal as possible, and to avoid the possibility
of circular dependencies since span and string_view both depend on
some parts of ranges too. They don't technically depend on drop_view
right now, but this is guarding against future headache.
Details
- Reviewers
• Quuxplusone - Group Reviewers
Restricted Project
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
I haven't really looked at the implementation yet, other than to say "ergh, this doesn't look like how I did it." ;) I'll wait for test/forward-declaration updates before looking again, in case the test updates flush out any issues with the implementation.
libcxx/include/CMakeLists.txt | ||
---|---|---|
226–227 |
On the cost side, there are several red flags associated with this part of the PR:
How about renaming these to __string/stringfwd.h and __span/spanfwd.h, and perhaps granularizing the rest of <string> and/or <span> in a preliminary PR while you're at it? (I suggest foofwd.h by analogy with iosfwd. I can see a case for just __foo/fwd.h, but that might be ambiguous when <foo> contains multiple types and only some of them are being forward-declared in the detail header.) | |
libcxx/test/std/ranges/range.adaptors/range.drop/adaptor.pass.cpp | ||
53 | Here and throughout, can you think of a good way to test that result.end() also has its correct value? | |
89 | Simpler: assert(result.empty()), since iota_view provides an .empty() method? | |
110–116 | I'd like to see more value-category tests, i.e. std::views::drop(static_cast<const View&&>(view), 2) for all four interesting cvref-qualifications, and ditto for all four cvref-qualifications on at least one of the "passthrough" types. |
Hmm, it looks like that page only tracks things that were in the original One Ranges Proposal, and drop_view was added by P1035 "Input Range Adaptors" (among other things).
Rebasing onto main -- after talking with @var-const, he'll commandeer this because I'm busy with other stuff at the moment, and we should really get this landed.
libcxx/include/__ranges/drop_view.h | ||
---|---|---|
142–143 | https://eel.is/c++draft/range.drop#overview-2.2.1 | |
151–152 | This smells like it doesn't exactly match what's supposed to happen in @var-const, you should definitely take a look at my D115122 for comparison. On glancing at it right now, I'm not sure that I do the subrange bullet points correctly, either, though! |
On the cost side, there are several red flags associated with this part of the PR:
How about renaming these to __string/stringfwd.h and __span/spanfwd.h, and perhaps granularizing the rest of <string> and/or <span> in a preliminary PR while you're at it? (I suggest foofwd.h by analogy with iosfwd. I can see a case for just __foo/fwd.h, but that might be ambiguous when <foo> contains multiple types and only some of them are being forward-declared in the detail header.)