Skip to content

Commit

Permalink
Fix issues with get() and unsigned integral types (#76)
Browse files Browse the repository at this point in the history 
* Fixed issue with integer boundary values getting recognized improperly

* Added fix for unsigned integer boundaries not being recognized properly
  • Loading branch information
@vincentlaucsb
vincentlaucsb authored Mar 10, 2020
1 parent 1a1c6d4 commit bb78aed
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 30 deletions.
8 changes: 7 additions & 1 deletion include/internal/csv_row.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ namespace csv {

// Allow fallthrough from previous if branch
IF_CONSTEXPR(!std::is_floating_point<T>::value) {
if (internals::type_num<T>() < this->_type) {
IF_CONSTEXPR(std::is_unsigned<T>::value) {
// Quick hack to perform correct unsigned integer boundary checks
if (this->value > internals::get_uint_max<sizeof(T)>()) {
throw std::runtime_error(internals::ERROR_OVERFLOW);
}
}
else if (internals::type_num<T>() < this->_type) {
throw std::runtime_error(internals::ERROR_OVERFLOW);
}
}
Expand Down
56 changes: 51 additions & 5 deletions include/internal/data_type.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ namespace csv {

/** Given a byte size, return the largest number than can be stored in
* an integer of that size
*
* Note: Provides a platform-agnostic way of mapping names like "long int" to
* byte sizes
*/
template<size_t Bytes>
CONSTEXPR long double get_int_max() {
Expand Down Expand Up @@ -124,7 +127,38 @@ namespace csv {
HEDLEY_UNREACHABLE();
}

/** Largest number that can be stored in a 1-bit integer */
/** Given a byte size, return the largest number than can be stored in
* an unsigned integer of that size
*/
template<size_t Bytes>
CONSTEXPR long double get_uint_max() {
static_assert(Bytes == 1 || Bytes == 2 || Bytes == 4 || Bytes == 8,
"Bytes must be a power of 2 below 8.");

IF_CONSTEXPR(sizeof(unsigned char) == Bytes) {
return (long double)std::numeric_limits<unsigned char>::max();
}

IF_CONSTEXPR(sizeof(unsigned short) == Bytes) {
return (long double)std::numeric_limits<unsigned short>::max();
}

IF_CONSTEXPR(sizeof(unsigned int) == Bytes) {
return (long double)std::numeric_limits<unsigned int>::max();
}

IF_CONSTEXPR(sizeof(unsigned long int) == Bytes) {
return (long double)std::numeric_limits<unsigned long int>::max();
}

IF_CONSTEXPR(sizeof(unsigned long long int) == Bytes) {
return (long double)std::numeric_limits<unsigned long long int>::max();
}

HEDLEY_UNREACHABLE();
}

/** Largest number that can be stored in a 8-bit integer */
CONSTEXPR_VALUE long double CSV_INT8_MAX = get_int_max<1>();

/** Largest number that can be stored in a 16-bit integer */
Expand All @@ -136,6 +170,18 @@ namespace csv {
/** Largest number that can be stored in a 64-bit integer */
CONSTEXPR_VALUE long double CSV_INT64_MAX = get_int_max<8>();

/** Largest number that can be stored in a 8-bit ungisned integer */
CONSTEXPR_VALUE long double CSV_UINT8_MAX = get_uint_max<1>();

/** Largest number that can be stored in a 16-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT16_MAX = get_uint_max<2>();

/** Largest number that can be stored in a 32-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT32_MAX = get_uint_max<4>();

/** Largest number that can be stored in a 64-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT64_MAX = get_uint_max<8>();

/** Given a pointer to the start of what is start of
* the exponential part of a number written (possibly) in scientific notation
* parse the exponent
Expand Down Expand Up @@ -164,13 +210,13 @@ namespace csv {
// We can assume number is always non-negative
assert(number >= 0);

if (number < internals::CSV_INT8_MAX)
if (number <= internals::CSV_INT8_MAX)
return CSV_INT8;
else if (number < internals::CSV_INT16_MAX)
else if (number <= internals::CSV_INT16_MAX)
return CSV_INT16;
else if (number < internals::CSV_INT32_MAX)
else if (number <= internals::CSV_INT32_MAX)
return CSV_INT32;
else if (number < internals::CSV_INT64_MAX)
else if (number <= internals::CSV_INT64_MAX)
return CSV_INT64;
else // Conversion to long long will cause an overflow
return CSV_DOUBLE;
Expand Down
64 changes: 58 additions & 6 deletions single_include/csv.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3332,6 +3332,9 @@ namespace csv {

/** Given a byte size, return the largest number than can be stored in
* an integer of that size
*
* Note: Provides a platform-agnostic way of mapping names like "long int" to
* byte sizes
*/
template<size_t Bytes>
CONSTEXPR long double get_int_max() {
Expand Down Expand Up @@ -3361,7 +3364,38 @@ namespace csv {
HEDLEY_UNREACHABLE();
}

/** Largest number that can be stored in a 1-bit integer */
/** Given a byte size, return the largest number than can be stored in
* an unsigned integer of that size
*/
template<size_t Bytes>
CONSTEXPR long double get_uint_max() {
static_assert(Bytes == 1 || Bytes == 2 || Bytes == 4 || Bytes == 8,
"Bytes must be a power of 2 below 8.");

IF_CONSTEXPR(sizeof(unsigned char) == Bytes) {
return (long double)std::numeric_limits<unsigned char>::max();
}

IF_CONSTEXPR(sizeof(unsigned short) == Bytes) {
return (long double)std::numeric_limits<unsigned short>::max();
}

IF_CONSTEXPR(sizeof(unsigned int) == Bytes) {
return (long double)std::numeric_limits<unsigned int>::max();
}

IF_CONSTEXPR(sizeof(unsigned long int) == Bytes) {
return (long double)std::numeric_limits<unsigned long int>::max();
}

IF_CONSTEXPR(sizeof(unsigned long long int) == Bytes) {
return (long double)std::numeric_limits<unsigned long long int>::max();
}

HEDLEY_UNREACHABLE();
}

/** Largest number that can be stored in a 8-bit integer */
CONSTEXPR_VALUE long double CSV_INT8_MAX = get_int_max<1>();

/** Largest number that can be stored in a 16-bit integer */
Expand All @@ -3373,6 +3407,18 @@ namespace csv {
/** Largest number that can be stored in a 64-bit integer */
CONSTEXPR_VALUE long double CSV_INT64_MAX = get_int_max<8>();

/** Largest number that can be stored in a 8-bit ungisned integer */
CONSTEXPR_VALUE long double CSV_UINT8_MAX = get_uint_max<1>();

/** Largest number that can be stored in a 16-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT16_MAX = get_uint_max<2>();

/** Largest number that can be stored in a 32-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT32_MAX = get_uint_max<4>();

/** Largest number that can be stored in a 64-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT64_MAX = get_uint_max<8>();

/** Given a pointer to the start of what is start of
* the exponential part of a number written (possibly) in scientific notation
* parse the exponent
Expand Down Expand Up @@ -3401,13 +3447,13 @@ namespace csv {
// We can assume number is always non-negative
assert(number >= 0);

if (number < internals::CSV_INT8_MAX)
if (number <= internals::CSV_INT8_MAX)
return CSV_INT8;
else if (number < internals::CSV_INT16_MAX)
else if (number <= internals::CSV_INT16_MAX)
return CSV_INT16;
else if (number < internals::CSV_INT32_MAX)
else if (number <= internals::CSV_INT32_MAX)
return CSV_INT32;
else if (number < internals::CSV_INT64_MAX)
else if (number <= internals::CSV_INT64_MAX)
return CSV_INT64;
else // Conversion to long long will cause an overflow
return CSV_DOUBLE;
Expand Down Expand Up @@ -3818,7 +3864,13 @@ namespace csv {

// Allow fallthrough from previous if branch
IF_CONSTEXPR(!std::is_floating_point<T>::value) {
if (internals::type_num<T>() < this->_type) {
IF_CONSTEXPR(std::is_unsigned<T>::value) {
// Quick hack to perform correct unsigned integer boundary checks
if (this->value > internals::get_uint_max<sizeof(T)>()) {
throw std::runtime_error(internals::ERROR_OVERFLOW);
}
}
else if (internals::type_num<T>() < this->_type) {
throw std::runtime_error(internals::ERROR_OVERFLOW);
}
}
Expand Down
64 changes: 58 additions & 6 deletions single_include_test/csv.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3332,6 +3332,9 @@ namespace csv {

/** Given a byte size, return the largest number than can be stored in
* an integer of that size
*
* Note: Provides a platform-agnostic way of mapping names like "long int" to
* byte sizes
*/
template<size_t Bytes>
CONSTEXPR long double get_int_max() {
Expand Down Expand Up @@ -3361,7 +3364,38 @@ namespace csv {
HEDLEY_UNREACHABLE();
}

/** Largest number that can be stored in a 1-bit integer */
/** Given a byte size, return the largest number than can be stored in
* an unsigned integer of that size
*/
template<size_t Bytes>
CONSTEXPR long double get_uint_max() {
static_assert(Bytes == 1 || Bytes == 2 || Bytes == 4 || Bytes == 8,
"Bytes must be a power of 2 below 8.");

IF_CONSTEXPR(sizeof(unsigned char) == Bytes) {
return (long double)std::numeric_limits<unsigned char>::max();
}

IF_CONSTEXPR(sizeof(unsigned short) == Bytes) {
return (long double)std::numeric_limits<unsigned short>::max();
}

IF_CONSTEXPR(sizeof(unsigned int) == Bytes) {
return (long double)std::numeric_limits<unsigned int>::max();
}

IF_CONSTEXPR(sizeof(unsigned long int) == Bytes) {
return (long double)std::numeric_limits<unsigned long int>::max();
}

IF_CONSTEXPR(sizeof(unsigned long long int) == Bytes) {
return (long double)std::numeric_limits<unsigned long long int>::max();
}

HEDLEY_UNREACHABLE();
}

/** Largest number that can be stored in a 8-bit integer */
CONSTEXPR_VALUE long double CSV_INT8_MAX = get_int_max<1>();

/** Largest number that can be stored in a 16-bit integer */
Expand All @@ -3373,6 +3407,18 @@ namespace csv {
/** Largest number that can be stored in a 64-bit integer */
CONSTEXPR_VALUE long double CSV_INT64_MAX = get_int_max<8>();

/** Largest number that can be stored in a 8-bit ungisned integer */
CONSTEXPR_VALUE long double CSV_UINT8_MAX = get_uint_max<1>();

/** Largest number that can be stored in a 16-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT16_MAX = get_uint_max<2>();

/** Largest number that can be stored in a 32-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT32_MAX = get_uint_max<4>();

/** Largest number that can be stored in a 64-bit unsigned integer */
CONSTEXPR_VALUE long double CSV_UINT64_MAX = get_uint_max<8>();

/** Given a pointer to the start of what is start of
* the exponential part of a number written (possibly) in scientific notation
* parse the exponent
Expand Down Expand Up @@ -3401,13 +3447,13 @@ namespace csv {
// We can assume number is always non-negative
assert(number >= 0);

if (number < internals::CSV_INT8_MAX)
if (number <= internals::CSV_INT8_MAX)
return CSV_INT8;
else if (number < internals::CSV_INT16_MAX)
else if (number <= internals::CSV_INT16_MAX)
return CSV_INT16;
else if (number < internals::CSV_INT32_MAX)
else if (number <= internals::CSV_INT32_MAX)
return CSV_INT32;
else if (number < internals::CSV_INT64_MAX)
else if (number <= internals::CSV_INT64_MAX)
return CSV_INT64;
else // Conversion to long long will cause an overflow
return CSV_DOUBLE;
Expand Down Expand Up @@ -3818,7 +3864,13 @@ namespace csv {

// Allow fallthrough from previous if branch
IF_CONSTEXPR(!std::is_floating_point<T>::value) {
if (internals::type_num<T>() < this->_type) {
IF_CONSTEXPR(std::is_unsigned<T>::value) {
// Quick hack to perform correct unsigned integer boundary checks
if (this->value > internals::get_uint_max<sizeof(T)>()) {
throw std::runtime_error(internals::ERROR_OVERFLOW);
}
}
else if (internals::type_num<T>() < this->_type) {
throw std::runtime_error(internals::ERROR_OVERFLOW);
}
}
Expand Down
14 changes: 13 additions & 1 deletion tests/test_csv_field.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,20 @@ TEST_CASE("CSVField get<>() - Integral Value", "[test_csv_field_get_int]") {
REQUIRE(ex_caught);
}

TEST_CASE("CSVField get<>() - Integer Boundary Value", "[test_csv_field_get_boundary]") {
// Note: Tests may fail if compiler defines typenames differently than
// Microsoft/GCC/clang
REQUIRE(CSVField("127").get<signed char>() == 127);
REQUIRE(CSVField("32767").get<short>() == 32767);
REQUIRE(CSVField("2147483647").get<int>() == 2147483647);

REQUIRE(CSVField("255").get<unsigned char>() == 255);
REQUIRE(CSVField("65535").get<unsigned short>() == 65535);
REQUIRE(CSVField("4294967295").get<unsigned>() == 4294967295);
}

// Test converting a small integer to unsigned and signed integer types
TEMPLATE_TEST_CASE("CSVField get<>() - Integral Value to Int", "[test_csv_field_get_int]",
TEMPLATE_TEST_CASE("CSVField get<>() - Integral Value to Int", "[test_csv_field_convert_int]",
unsigned char, unsigned short, unsigned int, unsigned long long,
char, short, int, long long int) {
CSVField savage("21");
Expand Down
40 changes: 29 additions & 11 deletions tests/test_data_type.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,40 @@ TEST_CASE( "Recognize Floats Properly", "[dtype_float]" ) {
REQUIRE(is_equal(out, 2.71828L));
}

TEST_CASE("Integer Overflow", "[int_overflow]") {
TEST_CASE("Integer Size Recognition", "[int_sizes]") {
std::string s;
long double out;

s = std::to_string((long long)csv::internals::CSV_INT16_MAX + 1);
REQUIRE(data_type(s, &out) == CSV_INT32);
REQUIRE(out == (long long)CSV_INT16_MAX + 1);
SECTION("Boundary Values") {
s = std::to_string((long long)csv::internals::CSV_INT8_MAX);
REQUIRE(data_type(s, &out) == CSV_INT8);
REQUIRE(out == (long long)CSV_INT8_MAX);

s = std::to_string((long long)csv::internals::CSV_INT32_MAX + 1);
REQUIRE(data_type(s, &out) == CSV_INT64);
REQUIRE(out == (long long)CSV_INT32_MAX + 1);
s = std::to_string((long long)csv::internals::CSV_INT16_MAX);
REQUIRE(data_type(s, &out) == CSV_INT16);
REQUIRE(out == (long long)CSV_INT16_MAX);

// Case: Integer too large to fit in int64 --> store in long double
s = std::to_string((long long)csv::internals::CSV_INT64_MAX);
s.append("1");
REQUIRE(data_type(s, &out) == CSV_DOUBLE);
s = std::to_string((long long)csv::internals::CSV_INT32_MAX);
REQUIRE(data_type(s, &out) == CSV_INT32);
REQUIRE(out == (long long)CSV_INT32_MAX);

// Note: data_type() doesn't have enough precision for CSV_INT64
}

SECTION("Integer Overflow") {
s = std::to_string((long long)csv::internals::CSV_INT16_MAX + 1);
REQUIRE(data_type(s, &out) == CSV_INT32);
REQUIRE(out == (long long)CSV_INT16_MAX + 1);

s = std::to_string((long long)csv::internals::CSV_INT32_MAX + 1);
REQUIRE(data_type(s, &out) == CSV_INT64);
REQUIRE(out == (long long)CSV_INT32_MAX + 1);

// Case: Integer too large to fit in int64 --> store in long double
s = std::to_string((long long)csv::internals::CSV_INT64_MAX);
s.append("1");
REQUIRE(data_type(s, &out) == CSV_DOUBLE);
}
}

TEST_CASE( "Recognize Sub-Unit Double Values", "[regression_double]" ) {
Expand Down

0 comments on commit bb78aed

Please sign in to comment.