depending on string pulls in heavy dependiencies

[nolange]

Hello,

I just realized you pulled in dependencies on string + shift operators. I am not really happy about that, as this will add alot of code, especially if you otherwise dont depend on this class, along with heap allocations.

I can create a pull request, replacing those dependencies with code using std::to_chars or std::sprintf working purely on the stack. This seems to be the better approach to me, would you agree and accept this?

[martinmoene]

Thanks @nolange ,

While introducing this dependency in #14 and avoid s(n)printf() I already was (somewhat) aware that it might be too heavy. It's nice to know someone will turn up to just say so ;)

I'd welcome a PR that supports C++98 and later in the guises MSVC, GNUC, Clang in some way by e.g.:

• only using sprintf()
• using snprintf(), and sprintf() where not available
• using std::to_chars(), and partially emulate it (via sprintf()?) where not available
• ...

[nolange]

I made a branch, still need a bit time and rebases for a PR. Just tell me early if I am off the road.

Design rationale is to depend as little as possible on the runtime library, so span can be used painlessly (side-effect free) as replacement for simple pointer+size struct. some code could be better organized at the cost of... more code (adding some helper functions for putting together the string), as its only used in on part I don't find this worthwhile.

I will be able to test this on Clang + gcc, and maybe one flavor of MSC at work. I believe newer compiler might complain about "unsafe" sprintf with those checks enabled.

[martinmoene]

For something that could be as short as:

throw std::out_of_range( "span::at(): index outside span" );

be it a little less informative, this seems becoming a bit out of hand.

I like:

• using type std::out_of_range directly as in std::vector

I prefer:

• not to start a name with an underscore: _throw_out_of_range_impl()
• use "standard library case"

Given this and the fact that callingthrow_out_of_range() is exceptional, my thinking at the moment is along the lines of:

#include <cstring>
#include <stdexcept>

#define span_constexpr /*constexpr*/

namespace nonstd {

typedef std::ptrdiff_t index_t;

namespace detail {

template< typename T >
inline char const * format( T x, char const * const begin, char * end )
{
    const bool neg = x < 0;

    // MSVC:  warning C4146: unary minus operator applied to unsigned type, result still unsigned
    if ( neg ) x = -x;

    *--end = '\0';    
    do
    {
        *--end = '0' + x % 10;
    }
    while ( (x /= 10) != 0 && end != begin ); 

    if ( neg && end != begin ) *--end = '-';

    return end;
}

template< size_t N, size_t M >
inline void throw_out_of_range_impl( char const (&where)[N], index_t idx, index_t size )
{
    span_constexpr const char text_index[] = ": index '";
    span_constexpr const char text_range[] = "' is out of range [0..";
    span_constexpr const char text_tail [] = ")";

    char number[ M ];
    char buffer[ M * 2 + N + sizeof text_index + sizeof text_range + sizeof text_tail ];

    std::strcpy( buffer, where );
    std::strcat( buffer, text_index );
    std::strcat( buffer, format( idx, number, number + sizeof number ) );
    std::strcat( buffer, text_range );
    std::strcat( buffer, format( size, number, number + sizeof number ) );
    std::strcat( buffer, text_tail );

    throw std::out_of_range( buffer );
}

#define nssv_num_chars(T) ( sizeof(T) >= 8 ? 20 : 11 )  // include sign

template< std::size_t N >
inline void throw_out_of_range( char const (&where)[N], index_t idx, index_t size )
{
    return throw_out_of_range_impl< N, nssv_num_chars(index_t) >( where, idx, size );
}

}  // namespace detail
}  // namespace nonstd

// Use above code -------------------------------------------

#include <iostream>
#include <limits>
#include <cmath>

template< typename T >
std::size_t num_digits()
{
    return static_cast<std::size_t>( std::log10( 1. * std::numeric_limits<T>::max() ) ) + 1;
}

using namespace nonstd;

int main()
{
    std::cout <<
        "\n          type: bytes: max required digits" <<
        "\n         short: " << sizeof(short)          << ": " << num_digits<short>() <<
        "\n           int: " << sizeof(int  )          << ": " << num_digits<int  >() <<
        "\n          long: " << sizeof(long )          << ": " << num_digits<long >() <<
        "\n     long long: " << sizeof(long long)      << ": " << num_digits<long long>() <<
        "\n   std::size_t: " << sizeof(std::size_t)    << ": " << num_digits< std::size_t>() <<
        "\nstd::ptrdiff_t: " << sizeof(std::ptrdiff_t) << ": " << num_digits< std::ptrdiff_t>() <<
        "\n";

    char buf[ nssv_num_chars(index_t) ];
    std::cout << 
        "\nformat( 42 , buf, buf + sizeof buf ): '" << detail::format( 42 , buf, buf + sizeof buf ) <<
        "\nformat( 42u, buf, buf + sizeof buf ): '" << detail::format( 42u, buf, buf + sizeof buf ) << 
        "'\n";

    try
    {
        detail::throw_out_of_range( "span::at()", 42, 7 );
    }
    catch ( std::exception const & e )
    {
        std::cout << "\n'" << e.what() << "'";
    }    

    try
    {
        detail::throw_out_of_range( "span::at()", -42, 7 );
    }
    catch ( std::exception const & e )
    {
        std::cout << "\n'" << e.what() << "'";
    }    
}

// Tried with MSVC 10, 14.1
// cl -EHsc -W4 -D_CRT_SECURE_NO_WARNINGS format.cpp && format.exe
// g++ -std=c++98 -Wall -Wextra -o format.exe format.cpp && format.exe

With std::to_chars() and a miminmal emulated version thereof, throw_out_of_range_impl() might look like:

using std::to_chars;

template< size_t N, size_t M >
inline void throw_out_of_range_impl( char const (&where)[N], index_t idx, index_t size )
{
    span_constexpr const char text_index[] = ": index '";
    span_constexpr const char text_range[] = "' is out of range [0..";
    span_constexpr const char text_tail [] = ")";

    char buffer[ M * 2 + N + sizeof text_index + sizeof text_range + sizeof text_tail ];

    char * ptr = buffer;
    memcpy( ptr, where, N ); 
    ptr += N - 1;
    memcpy( ptr, text_index, sizeof text_index ); 
    ptr += sizeof text_index - 1;
    ptr  = to_chars( ptr, buffer + sizeof buffer, idx ).ptr;
    memcpy( ptr, text_range, sizeof text_range ); 
    ptr += sizeof text_range - 1;
    ptr  = to_chars( ptr, buffer + sizeof buffer, size ).ptr;
    memcpy( ptr, text_tail, sizeof text_tail );

    throw std::out_of_range( buffer );
}

Or simply:

template< typename T >
long long to_longlong( T x ) { return static_cast<long long>( x ); }

template< size_t N, size_t M >
inline void throw_out_of_range_impl( char const (&where)[N], index_t idx, index_t size )
{
//  const char fmt[] = "%s: index '%td' is out of range [0..%td]";
    const char fmt[] = "%s: index '%lli' is out of range [0..%lli]";

    char buffer[ M * 2 + N + sizeof fmt ];

    sprintf( buffer, fmt, where, to_longlong(idx), to_longlong(size) );

    throw std::out_of_range( buffer );
}

[nolange]

...be it a little less informative, this seems becoming a bit out of hand.

Oh, I would be all in favor for dropping the formatting... I just want a header that I can use without second thoughts on PC as well as micro-controllers (pulling in string or printf can mean pulling in locale support aswell), or implement a heap manager with it and hook it into new without considering that a helper class could call it recursively.

Alternatively, libstdc++ would have its own __throw_out_of_range_fmt function (which dates back to 2004). That could be a viable alternative if __GLIBCXX__ is defined.

         if ( idx < 0 || size() <= idx ) 
          {
#if defined (__GLIBCXX__)
            std::__throw_out_of_range_fmt("%s: index '%lli' is out of range [0..%lli]", static_cast<long long>(idx), static_cast<long long>(size()));
#else
            throw std::out_of_range( span::at(): index outside span);
#endif
          }

so what do you prefer?

[martinmoene]

Ok, then I would be in favor of introducing span_CONFIG_MEMBER_AT_REPORTS_DETAIL (or something similar) that is 0 at default, like so:

#if span_CONFIG( MEMBER_AT_REPORTS_DETAIL )
inline void throw_out_of_range( index_t idx, index_t size )
{
    const char fmt[] = "span::at(): index '%lli' is out of range [0..%lli)";
    char buffer[ 2 * 20 + sizeof fmt ];
    sprintf( buffer, fmt, static_cast<long long>(idx), static_cast<long long>(size) );

    throw std::out_of_range( buffer );
}
#else 
inline void throw_out_of_range( index_t /*idx*/, index_t /*size*/ )
{
    throw std::out_of_range( "span::at(): index outside span" );
}
#endif

with span<>::at():

#if span_FEATURE( MEMBER_AT )
    span_constexpr14 reference at( index_type idx ) const
    {
#if span_CONFIG( NO_EXCEPTIONS )
        return this->operator[]( idx );
#else
        if ( idx < 0 || size() <= idx ) 
        {
            detail::throw_out_of_range( idx, size() );
        }
        return *( data() + idx );
#endif
    }
#endif

[martinmoene]

@nolange Assuming it will suit you, I'll advance with in above direction. .. and have an eye to your span_DO_SPRINTF.

[nolange]

I already applied the changes locally, but haven't tested them yet on multiple compilers