[libc++][NFC] Rename basic_istream_view::__iterator to __basic_istream_view_iterator

[philnik]

Summary

This makes it a lot easier to specialize traits types for it, like __segmented_iterator_traits.

[Mordante]

LGTM modulo one minor issue.

libcxx/include/__ranges/istream_view.h

41

Same for the definition.

[philnik]

libcxx/include/__ranges/istream_view.h

41

What does this actually do and how can we test it? We use the macro quite a bit, but not in any consistent way AFAICT.

[ldionne]

libcxx/include/__ranges/istream_view.h

41

Looking at the documentation and the definition of the macro, it applies the __type_visibility__(default) attribute to the type. This gives the vtable and the RTTI of the type (if any) default visibility.

This type doesn't have a vtable. The only scenario I can see this mattering in would be if someone throws a __basic_istream_view_iterator from a dylib and tries to catch it from an executable that uses that dylib. If the RTTI doesn't have default visibility, the RTTI generated in the executable and the RTTI generated in the dylib won't be coalesced by the dynamic linker, so we'll end up with two copies of the RTTI in the process. This means that e.g. the type_info::name() address would differ depending on whether you're inside the dylib or the executable, etc. Since some ABIs (the Itanium ABI) assume that the RTTI is unique within the process, this could lead to a situation where you are e.g. incapable of catching the object that was thrown from the dylib, since from the runtime's perspective they would be different types. See the Unique implementation of RTTI inside libcxx/include/typeinfo.

Caveat: This is kinda complicated and I'm just trying to work out the implications of this macro right now. TBH I've always found our visibility macros to be overcomplicated and have had on my mind to refactor them. For now I'd suggest that it's acceptable to not use this macro on this type.

libcxx/include/__ranges/istream_view.h

 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 namespace ranges {
 
 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>
+  requires default_initializable<_Val> && __stream_extractable<_Val, _CharT, _Traits>
+class __basic_istream_view_iterator;
+
 template <movable _Val, class _CharT, class _Traits = char_traits<_CharT>>
   requires default_initializable<_Val> && __stream_extractable<_Val, _CharT, _Traits>
 class basic_istream_view : public view_interface<basic_istream_view<_Val, _CharT, _Traits>> {
+  using __iterator = __basic_istream_view_iterator<_Val, _CharT, _Traits>;
+
+  template <movable _ValueType, class _CharType, class _TraitsType>
+    requires default_initializable<_ValueType> && __stream_extractable<_ValueType, _CharType, _TraitsType>
+  friend class __basic_istream_view_iterator;
+
 public:
   _LIBCPP_HIDE_FROM_ABI constexpr explicit basic_istream_view(basic_istream<_CharT, _Traits>& __stream)
       : __stream_(std::addressof(__stream)) {}
 
   _LIBCPP_HIDE_FROM_ABI constexpr auto begin() {
     *__stream_ >> __value_;
     return __iterator{*this};
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr default_sentinel_t end() const noexcept { return default_sentinel; }
 
 private:
-  class __iterator;
-
   basic_istream<_CharT, _Traits>* __stream_;
   _LIBCPP_NO_UNIQUE_ADDRESS _Val __value_ = _Val();
 };
 
 template <movable _Val, class _CharT, class _Traits>
   requires default_initializable<_Val> && __stream_extractable<_Val, _CharT, _Traits>
-class basic_istream_view<_Val, _CharT, _Traits>::__iterator {
+class __basic_istream_view_iterator {
 public:
   using iterator_concept = input_iterator_tag;
   using difference_type  = ptrdiff_t;
   using value_type       = _Val;
 
-  _LIBCPP_HIDE_FROM_ABI constexpr explicit __iterator(basic_istream_view& __parent) noexcept
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit __basic_istream_view_iterator(
+      basic_istream_view<_Val, _CharT, _Traits>& __parent) noexcept
       : __parent_(std::addressof(__parent)) {}
 
-  __iterator(const __iterator&)                  = delete;
-  _LIBCPP_HIDE_FROM_ABI __iterator(__iterator&&) = default;
+  __basic_istream_view_iterator(const __basic_istream_view_iterator&)                  = delete;
+  _LIBCPP_HIDE_FROM_ABI __basic_istream_view_iterator(__basic_istream_view_iterator&&) = default;
 
-  __iterator& operator=(const __iterator&)                  = delete;
-  _LIBCPP_HIDE_FROM_ABI __iterator& operator=(__iterator&&) = default;
+  __basic_istream_view_iterator& operator=(const __basic_istream_view_iterator&)                  = delete;
+  _LIBCPP_HIDE_FROM_ABI __basic_istream_view_iterator& operator=(__basic_istream_view_iterator&&) = default;
 
-  _LIBCPP_HIDE_FROM_ABI __iterator& operator++() {
+  _LIBCPP_HIDE_FROM_ABI __basic_istream_view_iterator& operator++() {
     *__parent_->__stream_ >> __parent_->__value_;
     return *this;
   }
 
   _LIBCPP_HIDE_FROM_ABI void operator++(int) { ++*this; }
 
   _LIBCPP_HIDE_FROM_ABI _Val& operator*() const { return __parent_->__value_; }
 
-  _LIBCPP_HIDE_FROM_ABI friend bool operator==(const __iterator& __x, default_sentinel_t) {
+  _LIBCPP_HIDE_FROM_ABI friend bool operator==(const __basic_istream_view_iterator& __x, default_sentinel_t) {
     return !*__x.__get_parent_stream();
   }
 
 private:
-  basic_istream_view* __parent_;
+  basic_istream_view<_Val, _CharT, _Traits>* __parent_;
 
   _LIBCPP_HIDE_FROM_ABI constexpr basic_istream<_CharT, _Traits>* __get_parent_stream() const {
     return __parent_->__stream_;
   }
 };
 
 template <class _Val>
 using istream_view = basic_istream_view<_Val, char>;
 
-#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+#  ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
 template <class _Val>
 using wistream_view = basic_istream_view<_Val, wchar_t>;
-#endif
+#  endif
 
 namespace views {
 namespace __istream {
 
 // clang-format off
 template <class _Tp>
 struct __fn {
   template <class _Up, class _UnCVRef = remove_cvref_t<_Up>>
_LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Up&& __u) const
     }
 };
 // clang-format on
 
 } // namespace __istream
 
 inline namespace __cpo {
 template <class _Tp>
-  inline constexpr auto istream = __istream::__fn<_Tp>{};
 } // namespace __cpo
 } // namespace views
 
 } // namespace ranges
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_STD_VER >= 20
 
 #endif // _LIBCPP___RANGES_ISTREAM_VIEW_H