Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix conversion and sign-conversion warnings. #71

Merged
merged 1 commit into from
Apr 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/gsl/gsl-lite.h
Expand Up @@ -1359,7 +1359,7 @@ class span

gsl_api gsl_constexpr14 index_type size() const gsl_noexcept
{
return last_ - first_;
return narrow_cast<index_type>( last_ - first_ );
}

gsl_api gsl_constexpr14 index_type length() const gsl_noexcept
Expand Down Expand Up @@ -2125,7 +2125,7 @@ gsl_api static span<T> ensure_sentinel( T * seq, SizeType max = std::numeric_lim

Expects( *cur == Sentinel );

return span<T>( seq, cur - seq );
return span<T>( seq, narrow_cast< typename span<T>::index_type >( cur - seq ) );
}
} // namespace detail

Expand Down
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Expand Up @@ -59,7 +59,7 @@ elseif( "${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU" OR
target_compile_options( gsl-lite-cpp14.t PUBLIC -std=c++14 )
endif()

add_compile_options( -Wall -Wno-missing-braces -fno-elide-constructors )
set ( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wno-missing-braces -fno-elide-constructors -Wconversion -Wsign-conversion -Wno-string-conversion" )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason to change add_compile_options to set ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest, I don't fully understand the difference, but when using add_compile_options no warnings were thrown and make VERBOSE=1 did not show the warning settings on the command line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked it up, this is the issue with add_compile_options:

Adds options to the compiler command line for targets in the current directory and below that are added after this command is invoked.

From this stack overflow answer
Isn't CMake wonderfully intuitive?

So it seems the CMakeLists.txt needs to be a bit rearranged in general to fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, thanks!


elseif( ${CMAKE_CXX_COMPILER_ID} MATCHES "Intel" )
# as is
Expand Down
22 changes: 11 additions & 11 deletions test/at.t.cpp
Expand Up @@ -70,9 +70,9 @@ CASE( "at(): Terminates access to non-existing gsl::span elements" )

CASE( "at(): Allows to access existing C-array elements" )
{
int a[] = { 1, 2, 3, 4 };
size_t a[] = { 1, 2, 3, 4 };

for ( int i = 0; i < 4; ++i )
for ( size_t i = 0; i < 4; ++i )
{
EXPECT( at(a, i) == i + 1 );
}
Expand All @@ -81,9 +81,9 @@ CASE( "at(): Allows to access existing C-array elements" )
CASE( "at(): Allows to access existing std::array elements (C++11)" )
{
#if gsl_HAVE_ARRAY
std::array<int, 4> a = {{ 1, 2, 3, 4 }};
std::array<size_t, 4> a = {{ 1, 2, 3, 4 }};

for ( int i = 0; i < 4; ++i )
for ( size_t i = 0; i < 4; ++i )
{
EXPECT( at(a, i) == i + 1 );
}
Expand All @@ -94,9 +94,9 @@ CASE( "at(): Allows to access existing std::array elements (C++11)" )

CASE( "at(): Allows to access existing std::vector elements" )
{
std::vector<int> a; // = { 1, 2, 3, 4 };
std::vector<size_t> a; // = { 1, 2, 3, 4 };

for ( int i = 0; i < 4; ++i )
for ( size_t i = 0; i < 4; ++i )
{
a.push_back( i + 1 );
EXPECT( at(a, i) == i + 1 );
Expand All @@ -108,9 +108,9 @@ CASE( "at(): Allows to access std::initializer_list elements (C++11)" )
// Note: GCC 4.6.3 has std::initializer_list but selects at(Cont & cont,...) overload.

#if gsl_HAVE_INITIALIZER_LIST && ( !gsl_COMPILER_GCC_VERSION || gsl_COMPILER_GCC_VERSION >= 473 )
std::initializer_list<int> a = { 1, 2, 3, 4 };
std::initializer_list<size_t> a = { 1, 2, 3, 4 };

for ( int i = 0; i < 4; ++i )
for ( size_t i = 0; i < 4; ++i )
{
EXPECT( at(a, i) == i + 1 );
}
Expand All @@ -121,10 +121,10 @@ CASE( "at(): Allows to access std::initializer_list elements (C++11)" )

CASE( "at(): Allows to access gsl::span elements" )
{
int arr[] = { 1, 2, 3, 4 };
span<int> a( arr );
size_t arr[] = { 1, 2, 3, 4 };
span<size_t> a( arr );

for ( int i = 0; i < 4; ++i )
for ( size_t i = 0; i < 4; ++i )
{
EXPECT( at(a, i) == i + 1 );
}
Expand Down
4 changes: 2 additions & 2 deletions test/lest_cpp03.hpp
Expand Up @@ -1023,7 +1023,7 @@ inline void shuffle( tests & specification, options option )
#if lest_CPP11_OR_GREATER
std::shuffle( specification.begin(), specification.end(), std::mt19937( option.seed ) );
#else
lest::srand( option.seed );
lest::srand( static_cast<unsigned int>( option.seed ) );

rng generator;
std::random_shuffle( specification.begin(), specification.end(), generator );
Expand All @@ -1032,7 +1032,7 @@ inline void shuffle( tests & specification, options option )

inline int stoi( text num )
{
return lest::strtol( num.c_str(), NULL, 10 );
return static_cast<int>( lest::strtol( num.c_str(), NULL, 10 ) );
}

inline bool is_number( text arg )
Expand Down
34 changes: 18 additions & 16 deletions test/span.t.cpp
Expand Up @@ -124,9 +124,9 @@ CASE( "span<>: Terminates creation of a sub span outside the span" )
CASE( "span<>: Terminates access outside the span" )
{
struct F {
static void blow_ix(int i) { int arr[] = { 1, 2, 3, }; span<int> v( arr ); (void) v[i]; }
static void blow_iv(int i) { int arr[] = { 1, 2, 3, }; span<int> v( arr ); (void) v(i); }
static void blow_at(int i) { int arr[] = { 1, 2, 3, }; span<int> v( arr ); (void) v.at(i); }
static void blow_ix(size_t i) { int arr[] = { 1, 2, 3, }; span<int> v( arr ); (void) v[i]; }
static void blow_iv(size_t i) { int arr[] = { 1, 2, 3, }; span<int> v( arr ); (void) v(i); }
static void blow_at(size_t i) { int arr[] = { 1, 2, 3, }; span<int> v( arr ); (void) v.at(i); }
};

EXPECT_NO_THROW( F::blow_ix(2) );
Expand Down Expand Up @@ -463,20 +463,20 @@ CASE( "span<>: Allows to construct from a non-empty gsl::unique_ptr (array, C++1
{
#if gsl_HAVE_UNIQUE_PTR
# if gsl_HAVE_MAKE_UNIQUE
gsl::unique_ptr<int[]> arr = make_unique<int[]>( 4 );
gsl::unique_ptr<size_t[]> arr = make_unique<size_t[]>( 4 );
#else
gsl::unique_ptr<int[]> arr = unique_ptr<int[]>( new int[4] );
gsl::unique_ptr<size_t[]> arr = unique_ptr<size_t[]>( new size_t[4] );
#endif

for ( int i = 0; i < 4; i++ )
for ( size_t i = 0; i < 4; i++ )
arr[i] = i + 1;

span<int> s( arr, 4 );
span<size_t> s( arr, 4 );

EXPECT( s.length() == index_type( 4 ) );
EXPECT( s.data() == arr.get() );
EXPECT( s[0] == 1 );
EXPECT( s[1] == 2 );
EXPECT( s[0] == 1u );
EXPECT( s[1] == 2u );
#else
EXPECT( !!"gsl::unique_ptr is not available" );
#endif
Expand Down Expand Up @@ -674,7 +674,8 @@ CASE( "span<>: Allows reverse iteration" )

for ( span<int>::reverse_iterator pos = v.rbegin(); pos != v.rend(); ++pos )
{
EXPECT( *pos == arr[ v.size() - 1 - std::distance(v.rbegin(), pos)] );
size_t dist = narrow<size_t>( std::distance(v.rbegin(), pos) );
EXPECT( *pos == arr[ v.size() - 1 - dist ] );
}
}

Expand All @@ -685,7 +686,8 @@ CASE( "span<>: Allows const reverse iteration" )

for ( span<int>::const_reverse_iterator pos = v.crbegin(); pos != v.crend(); ++pos )
{
EXPECT( *pos == arr[ v.size() - 1 - std::distance(v.crbegin(), pos)] );
size_t dist = narrow<size_t>( std::distance(v.crbegin(), pos) );
EXPECT( *pos == arr[ v.size() - 1 - dist ] );
}
}

Expand Down Expand Up @@ -1346,19 +1348,19 @@ CASE( "make_span(): Allows building from a non-empty gsl::unique_ptr (array, C++
{
# if gsl_HAVE_UNIQUE_PTR
# if gsl_HAVE_MAKE_SHARED
auto arr = std::make_unique<int[]>(4);
auto arr = std::make_unique<size_t[]>(4);
#else
auto arr = std::unique_ptr<int[]>( new int[4] );
auto arr = std::unique_ptr<size_t[]>( new size_t[4] );
#endif
for ( int i = 0; i < 4; i++ )
for ( size_t i = 0; i < 4; i++ )
arr[i] = i + 1;

auto s = make_span( arr, 4 );

EXPECT( s.length() == index_type( 4 ) );
EXPECT( s.data() == arr.get() );
EXPECT( s[0] == 1 );
EXPECT( s[1] == 2 );
EXPECT( s[0] == 1u );
EXPECT( s[1] == 2u );
#else
EXPECT( !!"gsl::unique_ptr<> is not available (no C++11)" );
#endif
Expand Down
12 changes: 8 additions & 4 deletions test/string_span.t.cpp
Expand Up @@ -740,7 +740,8 @@ CASE( "string_span: Allows forward iteration" )

for ( string_span::iterator pos = a.begin(); pos != a.end(); ++pos )
{
EXPECT( *pos == a[ std::distance( a.begin(), pos )] );
index_type i = narrow<index_type>( std::distance( a.begin(), pos ) );
EXPECT( *pos == a[i] );
}
}

Expand All @@ -751,7 +752,8 @@ CASE( "string_span: Allows const forward iteration" )

for ( string_span::const_iterator pos = a.begin(); pos != a.end(); ++pos )
{
EXPECT( *pos == a[ std::distance( a.cbegin(), pos )] );
index_type i = narrow<index_type>( std::distance( a.cbegin(), pos ) );
EXPECT( *pos == a[i] );
}
}

Expand All @@ -762,7 +764,8 @@ CASE( "string_span: Allows reverse iteration" )

for ( string_span::reverse_iterator pos = a.rbegin(); pos != a.rend(); ++pos )
{
EXPECT( *pos == a[ a.size() - 1 - std::distance( a.rbegin(), pos )] );
index_type dist = narrow<index_type>( std::distance( a.rbegin(), pos ) );
EXPECT( *pos == a[ a.size() - 1 - dist ] );
}
}

Expand All @@ -773,7 +776,8 @@ CASE( "string_span: Allows const reverse iteration" )

for ( string_span::const_reverse_iterator pos = a.crbegin(); pos != a.crend(); ++pos )
{
EXPECT( *pos == a[ a.size() - 1 - std::distance( a.crbegin(), pos )] );
index_type dist = narrow<index_type>( std::distance( a.crbegin(), pos ) );
EXPECT( *pos == a[ a.size() - 1 - dist ] );
}
}

Expand Down