Skip to content

Commit

Permalink
fix the tests by creating a numerical_conversion function that handle…
Browse files Browse the repository at this point in the history
…s the actual numerical conversion using the lower level function to avoid catching exceptions internally.

add a test for char options

add support for char types to the lexical cast, to allow single character types that make sense, add a integral_conversion operations to simplify the conversions from string to integers and allow discrimination in a few cases with enumerations.
  • Loading branch information
phlptp committed Mar 29, 2020
1 parent 7432ab2 commit d844c3e
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 59 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ While all options internally are the same type, there are several ways to add an
app.add_option(option_name, help_str="")

app.add_option(option_name,
variable_to_bind_to, // bool, int, float, vector, enum, or string-like, or anything with a defined conversion from a string or that takes an int 🆕, double 🆕, or string in a constructor. Also allowed are tuples 🆕, std::array 🆕 or std::pair 🆕. Also supported are complex numbers🚧, wrapper types🚧, and containers besides vector🚧 of any other supported type.
variable_to_bind_to, // bool, char(see note)🚧, int, float, vector, enum, or string-like, or anything with a defined conversion from a string or that takes an int 🆕, double 🆕, or string in a constructor. Also allowed are tuples 🆕, std::array 🆕 or std::pair 🆕. Also supported are complex numbers🚧, wrapper types🚧, and containers besides vector🚧 of any other supported type.
help_string="")

app.add_option_function<type>(option_name,
Expand All @@ -204,6 +204,8 @@ app.add_option_function<type>(option_name,

app.add_complex(... // Special case: support for complex numbers ⚠️. Complex numbers are now fully supported in the add_option so this function is redundant.

// char as an option type is supported before 2.0 but in 2.0 it defaulted to allowing single non numerical characters in addition to the numeric values.

// 🆕 There is a template overload which takes two template parameters the first is the type of object to assign the value to, the second is the conversion type. The conversion type should have a known way to convert from a string, such as any of the types that work in the non-template version. If XC is a std::pair and T is some non pair type. Then a two argument constructor for T is called to assign the value. For tuples or other multi element types, XC must be a single type or a tuple like object of the same size as the assignment type
app.add_option<typename T, typename XC>(option_name,
T &output, // output must be assignable or constructible from a value of type XC
Expand Down
1 change: 1 addition & 0 deletions book/chapters/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ You can use any C++ int-like type, not just `int`. CLI11 understands the followi
|-------------|-------|
| number like | Integers, floats, bools, or any type that can be constructed from an integer or floating point number |
| string-like | std\::string, or anything that can be constructed from or assigned a std\::string |
| char | For a single char, single string values are accepted, otherwise longer strings are treated as integral values and a conversion is attempted |
| complex-number | std::complex or any type which has a real(), and imag() operations available, will allow 1 or 2 string definitions like "1+2j" or two arguments "1","2" |
| enumeration | any enum or enum class type is supported through conversion from the underlying type(typically int, though it can be specified otherwise) |
| container-like | a container(like vector) of any available types including other containers |
Expand Down
110 changes: 66 additions & 44 deletions include/CLI/TypeTools.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ struct expected_count<T, typename std::enable_if<!is_mutable_container<T>::value

// Enumeration of the different supported categorizations of objects
enum class object_category : int {
char_value = 1,
integral_value = 2,
unsigned_integral = 4,
enumeration = 6,
Expand All @@ -525,27 +526,36 @@ enum class object_category : int {

};

/// Set of overloads to classify an object according to type

/// some type that is not otherwise recognized
template <typename T, typename Enable = void> struct classify_object {
static constexpr object_category value{object_category::other};
};

/// Set of overloads to classify an object according to type
/// Signed integers
template <typename T>
struct classify_object<T,
typename std::enable_if<std::is_integral<T>::value && std::is_signed<T>::value &&
!is_bool<T>::value && !std::is_enum<T>::value>::type> {
struct classify_object<
T,
typename std::enable_if<std::is_integral<T>::value && !std::is_same<T, char>::value && std::is_signed<T>::value &&
!is_bool<T>::value && !std::is_enum<T>::value>::type> {
static constexpr object_category value{object_category::integral_value};
};

/// Unsigned integers
template <typename T>
struct classify_object<
T,
typename std::enable_if<std::is_integral<T>::value && std::is_unsigned<T>::value && !is_bool<T>::value>::type> {
struct classify_object<T,
typename std::enable_if<std::is_integral<T>::value && std::is_unsigned<T>::value &&
!std::is_same<T, char>::value && !is_bool<T>::value>::type> {
static constexpr object_category value{object_category::unsigned_integral};
};

/// single character values
template <typename T>
struct classify_object<T, typename std::enable_if<std::is_same<T, char>::value && !std::is_enum<T>::value>::type> {
static constexpr object_category value{object_category::char_value};
};

/// Boolean values
template <typename T> struct classify_object<T, typename std::enable_if<is_bool<T>::value>::type> {
static constexpr object_category value{object_category::boolean_value};
Expand Down Expand Up @@ -657,6 +667,12 @@ template <typename T> struct classify_object<T, typename std::enable_if<is_mutab
/// http://stackoverflow.com/questions/1055452/c-get-name-of-type-in-template
/// But this is cleaner and works better in this case

template <typename T,
enable_if_t<classify_object<T>::value == object_category::char_value, detail::enabler> = detail::dummy>
constexpr const char *type_name() {
return "CHAR";
}

template <typename T,
enable_if_t<classify_object<T>::value == object_category::integral_value ||
classify_object<T>::value == object_category::integer_constructible,
Expand Down Expand Up @@ -767,6 +783,30 @@ inline std::string type_name() {

// Lexical cast

/// Convert to an unsigned integral
template <typename T, enable_if_t<std::is_unsigned<T>::value, detail::enabler> = detail::dummy>
bool integral_conversion(const std::string &input, T &output) noexcept {
if(input.empty()) {
return false;
}
char *val = nullptr;
std::uint64_t output_ll = std::strtoull(input.c_str(), &val, 0);
output = static_cast<T>(output_ll);
return val == (input.c_str() + input.size()) && static_cast<std::uint64_t>(output) == output_ll;
}

/// Convert to a signed integral
template <typename T, enable_if_t<std::is_signed<T>::value, detail::enabler> = detail::dummy>
bool integral_conversion(const std::string &input, T &output) noexcept {
if(input.empty()) {
return false;
}
char *val = nullptr;
std::int64_t output_ll = std::strtoll(input.c_str(), &val, 0);
output = static_cast<T>(output_ll);
return val == (input.c_str() + input.size()) && static_cast<std::int64_t>(output) == output_ll;
}

/// Convert a flag into an integer value typically binary flags
inline std::int64_t to_flag_value(std::string val) {
static const std::string trueString("true");
Expand Down Expand Up @@ -810,39 +850,24 @@ inline std::int64_t to_flag_value(std::string val) {
return ret;
}

/// Signed integers
/// Integer conversion
template <typename T,
enable_if_t<classify_object<T>::value == object_category::integral_value, detail::enabler> = detail::dummy>
enable_if_t<classify_object<T>::value == object_category::integral_value ||
classify_object<T>::value == object_category::unsigned_integral,
detail::enabler> = detail::dummy>
bool lexical_cast(const std::string &input, T &output) {
try {
std::size_t n = 0;
std::int64_t output_ll = std::stoll(input, &n, 0);
output = static_cast<T>(output_ll);
return n == input.size() && static_cast<std::int64_t>(output) == output_ll;
} catch(const std::invalid_argument &) {
return false;
} catch(const std::out_of_range &) {
return false;
}
return integral_conversion(input, output);
}

/// Unsigned integers
/// char values
template <typename T,
enable_if_t<classify_object<T>::value == object_category::unsigned_integral, detail::enabler> = detail::dummy>
enable_if_t<classify_object<T>::value == object_category::char_value, detail::enabler> = detail::dummy>
bool lexical_cast(const std::string &input, T &output) {
if(!input.empty() && input.front() == '-')
return false; // std::stoull happily converts negative values to junk without any errors.

try {
std::size_t n = 0;
std::uint64_t output_ll = std::stoull(input, &n, 0);
output = static_cast<T>(output_ll);
return n == input.size() && static_cast<std::uint64_t>(output) == output_ll;
} catch(const std::invalid_argument &) {
return false;
} catch(const std::out_of_range &) {
return false;
if(input.size() == 1) {
output = static_cast<T>(input[0]);
return true;
}
return integral_conversion(input, output);
}

/// Boolean values
Expand All @@ -867,15 +892,13 @@ bool lexical_cast(const std::string &input, T &output) {
template <typename T,
enable_if_t<classify_object<T>::value == object_category::floating_point, detail::enabler> = detail::dummy>
bool lexical_cast(const std::string &input, T &output) {
try {
std::size_t n = 0;
output = static_cast<T>(std::stold(input, &n));
return n == input.size();
} catch(const std::invalid_argument &) {
return false;
} catch(const std::out_of_range &) {
if(input.empty()) {
return false;
}
char *val = nullptr;
auto output_ld = std::strtold(input.c_str(), &val);
output = static_cast<T>(output_ld);
return val == (input.c_str() + input.size());
}

/// complex
Expand Down Expand Up @@ -932,8 +955,7 @@ template <typename T,
enable_if_t<classify_object<T>::value == object_category::enumeration, detail::enabler> = detail::dummy>
bool lexical_cast(const std::string &input, T &output) {
typename std::underlying_type<T>::type val;
bool retval = detail::lexical_cast(input, val);
if(!retval) {
if(!integral_conversion(input, val)) {
return false;
}
output = static_cast<T>(val);
Expand All @@ -958,7 +980,7 @@ template <
enable_if_t<classify_object<T>::value == object_category::number_constructible, detail::enabler> = detail::dummy>
bool lexical_cast(const std::string &input, T &output) {
int val;
if(lexical_cast(input, val)) {
if(integral_conversion(input, val)) {
output = T(val);
return true;
} else {
Expand All @@ -977,7 +999,7 @@ template <
enable_if_t<classify_object<T>::value == object_category::integer_constructible, detail::enabler> = detail::dummy>
bool lexical_cast(const std::string &input, T &output) {
int val;
if(lexical_cast(input, val)) {
if(integral_conversion(input, val)) {
output = T(val);
return true;
}
Expand Down
31 changes: 19 additions & 12 deletions include/CLI/Validators.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -961,14 +961,11 @@ class AsNumberWithUnit : public Validator {
if(opts & CASE_INSENSITIVE) {
unit = detail::to_lower(unit);
}

bool converted = detail::lexical_cast(input, num);
if(!converted) {
throw ValidationError(std::string("Value ") + input + " could not be converted to " +
detail::type_name<Number>());
}

if(unit.empty()) {
if(!detail::lexical_cast(input, num)) {
throw ValidationError(std::string("Value ") + input + " could not be converted to " +
detail::type_name<Number>());
}
// No need to modify input if no unit passed
return {};
}
Expand All @@ -982,12 +979,22 @@ class AsNumberWithUnit : public Validator {
detail::generate_map(mapping, true));
}

// perform safe multiplication
bool ok = detail::checked_multiply(num, it->second);
if(!ok) {
throw ValidationError(detail::to_string(num) + " multiplied by " + unit +
" factor would cause number overflow. Use smaller value.");
if(!input.empty()) {
bool converted = detail::lexical_cast(input, num);
if(!converted) {
throw ValidationError(std::string("Value ") + input + " could not be converted to " +
detail::type_name<Number>());
}
// perform safe multiplication
bool ok = detail::checked_multiply(num, it->second);
if(!ok) {
throw ValidationError(detail::to_string(num) + " multiplied by " + unit +
" factor would cause number overflow. Use smaller value.");
}
} else {
num = static_cast<Number>(it->second);
}

input = detail::to_string(num);

return {};
Expand Down
14 changes: 13 additions & 1 deletion tests/HelpersTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,9 @@ TEST(Types, TypeName) {
std::string float_name = CLI::detail::type_name<double>();
EXPECT_EQ("FLOAT", float_name);

std::string char_name = CLI::detail::type_name<char>();
EXPECT_EQ("CHAR", char_name);

std::string vector_name = CLI::detail::type_name<std::vector<int>>();
EXPECT_EQ("INT", vector_name);

Expand Down Expand Up @@ -1025,6 +1028,11 @@ TEST(Types, LexicalCastInt) {

std::string extra_input = "912i";
EXPECT_FALSE(CLI::detail::lexical_cast(extra_input, y));

std::string empty_input{};
EXPECT_FALSE(CLI::detail::lexical_cast(empty_input, x_signed));
EXPECT_FALSE(CLI::detail::lexical_cast(empty_input, x_unsigned));
EXPECT_FALSE(CLI::detail::lexical_cast(empty_input, y_signed));
}

TEST(Types, LexicalCastDouble) {
Expand All @@ -1037,10 +1045,14 @@ TEST(Types, LexicalCastDouble) {
EXPECT_FALSE(CLI::detail::lexical_cast(bad_input, x));

std::string overflow_input = "1" + std::to_string(LDBL_MAX);
EXPECT_FALSE(CLI::detail::lexical_cast(overflow_input, x));
EXPECT_TRUE(CLI::detail::lexical_cast(overflow_input, x));
EXPECT_FALSE(std::isfinite(x));

std::string extra_input = "9.12i";
EXPECT_FALSE(CLI::detail::lexical_cast(extra_input, x));

std::string empty_input{};
EXPECT_FALSE(CLI::detail::lexical_cast(empty_input, x));
}

TEST(Types, LexicalCastBool) {
Expand Down
25 changes: 25 additions & 0 deletions tests/OptionTypeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,31 @@ TEST_F(TApp, BoolOption) {
EXPECT_FALSE(bflag);
}

TEST_F(TApp, CharOption) {
char c1{'t'};
app.add_option("-c", c1);

args = {"-c", "g"};
run();
EXPECT_EQ(c1, 'g');

args = {"-c", "1"};
run();
EXPECT_EQ(c1, '1');

args = {"-c", "77"};
run();
EXPECT_EQ(c1, 77);

// convert hex for digit
args = {"-c", "0x44"};
run();
EXPECT_EQ(c1, 0x44);

args = {"-c", "751615654161688126132138844896646748852"};
EXPECT_THROW(run(), CLI::ConversionError);
}

TEST_F(TApp, vectorDefaults) {
std::vector<int> vals{4, 5};
auto opt = app.add_option("--long", vals, "", true);
Expand Down
1 change: 1 addition & 0 deletions tests/OptionalTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ TEST_F(TApp, BoostOptionalEnumTest) {

enum class eval : char { val0 = 0, val1 = 1, val2 = 2, val3 = 3, val4 = 4 };
boost::optional<eval> opt, opt2;

auto optptr = app.add_option<decltype(opt), eval>("-v,--val", opt);
app.add_option_no_stream("-e,--eval", opt2);
optptr->capture_default_str();
Expand Down
3 changes: 2 additions & 1 deletion tests/TransformTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,8 @@ TEST_F(TApp, NumberWithUnitBadInput) {
args = {"-n", "13 c"};
EXPECT_THROW(run(), CLI::ValidationError);
args = {"-n", "a"};
EXPECT_THROW(run(), CLI::ValidationError);
// Assume 1.0 unit
EXPECT_NO_THROW(run());
args = {"-n", "12.0a"};
EXPECT_THROW(run(), CLI::ValidationError);
args = {"-n", "a5"};
Expand Down

0 comments on commit d844c3e

Please sign in to comment.