depending on string pulls in heavy dependiencies
nolange opened this issue · 7 comments
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?
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()
, andsprintf()
where not available - using
std::to_chars()
, and partially emulate it (viasprintf()
?) where not available - ...
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.
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 instd::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 );
}
...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?
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
@nolange Assuming it will suit you, I'll advance with in above direction. .. and have an eye to your span_DO_SPRINTF.
I already applied the changes locally, but haven't tested them yet on multiple compilers