STL `<spanstream>`: The `span` constructed by `basic_ispanstream`'s range constructor may be ill-formed

This range constructor is specified in [ispanstream.cons] as:

template<class ROS> explicit basic_ispanstream(ROS&& s)

-3- Constraints: ROS models ranges::borrowed_range. !convertible_to<ROS, std::span<charT>> && convertible_to<ROS, std::span<charT const>> is true.

-4- Effects: Let sp be std::span<const charT>(std::forward<ROS>(s)). Equivalent to: basic_ispanstream(std::span<charT>(const_cast<charT*>(sp.data()), sp.size()))

Here, only std::span<const charT>(std::forward<ROS>(s)) is required, but MSVC always constructs span via std::span<const charT>(ranges:: data(s)), ranges::size(s)), which is not always well-formed (for example, when s is not a sized_range or contiguous_range):

#include <spanstream>

struct R {
  int* begin();
  std::unreachable_sentinel_t end();
  operator std::span<const int>() const;
};

int main() {
  R r{};
  std::basic_ispanstream<int> is{r}; // error
  is.span(r); // error
}

https://godbolt.org/z/Trv8d8qd3

The above example seems contrived, but libstdc++ accepts it as the standard guarantees it. (Whether this is worthy of LWG may be arguable).

Aug 06 '24 02:08 hewillk

Seems replacing this:

template <_RANGES borrowed_range _ReadOnlyRange>
        requires (
            !convertible_to<_ReadOnlyRange, _STD span<_Elem>> && convertible_to<_ReadOnlyRange, _STD span<const _Elem>>)
    void span(_ReadOnlyRange&& _Range) noexcept {
        this->span(
            _STD span<_Elem>{const_cast<_Elem*>(_RANGES data(_Range)), static_cast<size_t>(_RANGES size(_Range))});
    }
    
template <_RANGES borrowed_range _ReadOnlyRange>
        requires (
            !convertible_to<_ReadOnlyRange, _STD span<_Elem>> && convertible_to<_ReadOnlyRange, _STD span<const _Elem>>)
    explicit basic_ispanstream(_ReadOnlyRange&& _Range)
        : basic_ispanstream(
            _STD span<_Elem>{const_cast<_Elem*>(_RANGES data(_Range)), static_cast<size_t>(_RANGES size(_Range))}) {}

With this solves the compile error but I am not sure if this is right:

template <_RANGES borrowed_range _ReadOnlyRange>
        requires (
            !convertible_to<_ReadOnlyRange, _STD span<_Elem>> && convertible_to<_ReadOnlyRange, _STD span<const _Elem>>)
    void span(_ReadOnlyRange&& _Range) noexcept {
        _STD span<const _Elem> __sp(_STD forward<_ReadOnlyRange>(_Range));
        _Buf.span({const_cast<_Elem*>(__sp.data()), __sp.size()});
    }
    
template <_RANGES borrowed_range _ReadOnlyRange>
        requires (
            !convertible_to<_ReadOnlyRange, _STD span<_Elem>> && convertible_to<_ReadOnlyRange, _STD span<const _Elem>>)
    explicit basic_ispanstream(_ReadOnlyRange&& _Range)
        : _Mybase(_STD addressof(_Buf)), _Buf(_STD ios_base::in) {
            _STD span<const _Elem> __sp(_STD forward<_ReadOnlyRange>(_Range));
            _Buf.span({const_cast<_Elem*>(__sp.data()), __sp.size()});
        }

Or better write like this:

template <_RANGES borrowed_range _ReadOnlyRange>
       requires (
           !convertible_to<_ReadOnlyRange, _STD span<_Elem>> && convertible_to<_ReadOnlyRange, _STD span<const _Elem>>)
   explicit basic_ispanstream(_ReadOnlyRange&& _Range)
       : _Mybase(_STD addressof(_Buf)), _Buf(_STD ios_base::in) {
           span(_STD forward<_ReadOnlyRange>(_Range));
       }

Aug 06 '24 06:08 ferhatgec

Perhaps we should file an LWG issue. The intent seems to be that s should be exactly the range referenced by the stored span, while the conversion function can return an unrelated range.

Aug 12 '24 05:08 frederick-vs-ja

That is almost certainly the design intent, but AFAICS that design intent doesn't matter. The wording clearly indicates that we convert the argument into a span and use the result to initialize the basic_ispanstream, so it's on the user's head if they invalidate the span and then use the basic_ispanstream.

The only issue I see with the wording is that it doesn't require the conversion to be equality-preserving. Since sp is just an alias for the expression std::span<const charT>(std::forward<ROS>(s)), there's no guarantee that std::span<const charT>(std::forward<ROS>(s)).data() and std::span<const charT>(std::forward<ROS>(s)).size() in the "equivalent to" wording are from the same span. In other words, sp.data() + [0, sp.size()) may not be a valid range.

The wording needs something silly like "Let sp be an lvalue denoting a std::span<const charT> object initialized with std::forward<ROS>(s)." to guarantee that sp in basic_ispanstream(std::span<charT>(const_cast<charT*>(sp.data()), sp.size())) always refers to the same span.

Aug 12 '24 23:08 CaseyCarter