<istream> defines basic_istream and basic_iostream. The basic_iostream brings all the dependencies to <ostream>.
While implementing <ranges>'s std::views::istream, we need the declaration of basic_istream and all its member >> and free >>,
because the concept checks the validity of >>. To avoid adding too many dependencies to <ranges>, it would be good idea to split
basic_istream and basic_iostream. Also, we only need the declarations of >> to pass the concept check, we don't need the extra
dependencies coming from the implementations, namely <locale> and its indirect dependencies (__locale isn't enough). Luckily, the basic_istream's member definition is out of line, so there is not much work to do
other than split the files. Also there is not much SFINAE going so keeping declaration and definition separate won't cause too much code duplication
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
The approach we generally take is to create a libcxx/include/__fwd/foo.h header. Would that work here?
| libcxx/include/__istream/basic_iostream.h | ||
|---|---|---|
| 272 | No newline! Please add one! | |
Do you mean by creating a header that only contains the forward declaration like
template <class _Chart> class basic_istream;
But the std::ranges::istream_view class itself has a constraint that >> needs to be valid
template <class _Val, class _CharT, class _Traits>
concept __stream_extractable = requires(basic_istream<_CharT, _Traits>& __is, _Val& __t) { __is >> __t; };
template <movable _Val, class _CharT, class _Traits = char_traits<_CharT>>
requires default_initializable<_Val> && __stream_extractable<_Val, _CharT, _Traits>
class basic_istream_view;That means the declaration of basic_istream_view already requires all the member >> and free >> from the basic_istream. So only a forward declaration of basic_istream isn't enough.
And this is the main reason I made the basic_istream.h header contains all the >> declarations.
I think this can be pretty much abandoned and the forward declaration of basic_istream folded into the istream_view patch itself.
| libcxx/include/__istream/basic_istream.h | ||
|---|---|---|
| 1 | In light of the survey of other implementations we just did during live review (https://godbolt.org/z/G6EbqvY57), I suggest we:
The result will be that users can't use istream_view without including <istream> themselves, however that's what other implementations do. And the alternative would be to pull in large parts of iostream and the localization library, and that's unacceptable. | |
No newline! Please add one!