From e2994ed485c941c669787ce38876e2478dba28d0 Mon Sep 17 00:00:00 2001 From: Gereon Kremer Date: Thu, 2 Sep 2021 14:34:40 -0700 Subject: [PATCH] Refactor options handlers (#7080) This PR does a major refactoring to the default option handlers (i.e. the piece of code that converts strings to the respective option types). It gets rid of the current multi-layer template specialization in favor of a simple overloaded template function. --- src/options/mkoptions.py | 2 +- src/options/options_public.h | 2 - src/options/options_public_template.cpp | 302 +++++++++++------------- 3 files changed, 135 insertions(+), 171 deletions(-) diff --git a/src/options/mkoptions.py b/src/options/mkoptions.py index d2881cc94..1adc2094d 100644 --- a/src/options/mkoptions.py +++ b/src/options/mkoptions.py @@ -195,7 +195,7 @@ def get_handler(option): return 'opts.handler().{}("{}", name, optionarg)'.format(option.handler, optname) elif option.mode: return 'stringTo{}(optionarg)'.format(option.type) - return 'handleOption<{}>("{}", name, optionarg)'.format(option.type, optname) + return 'handlers::handleOption<{}>("{}", name, optionarg)'.format(option.type, optname) def get_predicates(option): diff --git a/src/options/options_public.h b/src/options/options_public.h index afd777761..a5a223517 100644 --- a/src/options/options_public.h +++ b/src/options/options_public.h @@ -34,8 +34,6 @@ namespace cvc5::options { -bool getUfHo(const Options& opts) CVC5_EXPORT; - /** * Retrieve an option value by name (as given in key) from the Options object * opts as a string. diff --git a/src/options/options_public_template.cpp b/src/options/options_public_template.cpp index 9c44dacba..c21fdee97 100644 --- a/src/options/options_public_template.cpp +++ b/src/options/options_public_template.cpp @@ -30,200 +30,166 @@ ${headers_handler}$ #include namespace cvc5::options { - -bool getUfHo(const Options& opts) { return opts.uf.ufHo; } - -/** Set a given Options* as "current" just for a particular scope. */ -class OptionsGuard { - Options** d_field; - Options* d_old; -public: - OptionsGuard(Options** field, Options* opts) : - d_field(field), - d_old(*field) { - *field = opts; - } - ~OptionsGuard() { - *d_field = d_old; + // Contains the default option handlers (i.e. parsers) + namespace handlers { + + /** + * Utility function for handling numeric options. Takes care of checking for + * unsignedness, parsing and handling parsing exceptions. Expects `conv` to be + * a conversion function like `std::stod`, accepting a `std::string` and a + * `size_t*`. The argument `type` is only used to generate proper error + * messages and should be the string representation of `T`. If `T` is + * unsigned, checks that `optionarg` contains no minus. Then `conv` is called + * and the error conditions are handled: `conv` may throw an exception or + * `pos` may be smaller than the size of `optionarg`, indicating that not the + * entirety of `optionarg` was parsed. + */ + template + T parseNumber(const std::string& flag, + const std::string& optionarg, + FF&& conv, + const std::string& type) + { + if (!std::numeric_limits::is_signed + && (optionarg.find('-') != std::string::npos)) + { + std::stringstream ss; + ss << "Argument '" << optionarg << "' for " << type << " option " << flag + << " is negative"; + throw OptionException(ss.str()); + } + size_t pos = 0; + T res; + try + { + res = conv(optionarg, &pos); + } + catch (const std::exception& e) + { + std::stringstream ss; + ss << "Argument '" << optionarg << "' for " << type << " option " << flag + << " did not parse as " << type; + throw OptionException(ss.str()); + } + if (pos < optionarg.size()) + { + std::stringstream ss; + ss << "Argument '" << optionarg << "' for " << type << " option " << flag + << " did parse only partially as " << type << ", leaving '" + << optionarg.substr(pos) << "'"; + throw OptionException(ss.str()); + } + return res; } -};/* class OptionsGuard */ - -/** - * This is a default handler for options of built-in C++ type. This - * template is really just a helper for the handleOption() template, - * below. Variants of this template handle numeric and non-numeric, - * integral and non-integral, signed and unsigned C++ types. - * handleOption() makes sure to instantiate the right one. - * - * This implements default behavior when e.g. an option is - * unsigned but the user specifies a negative argument; etc. - */ -template -struct OptionHandler { - static T handle(const std::string& option, const std::string& flag, const std::string& optionarg); -};/* struct OptionHandler<> */ - -/** Variant for integral C++ types */ -template -struct OptionHandler { - static bool stringToInt(T& t, const std::string& str) { - std::istringstream ss(str); - ss >> t; - char tmp; - return !(ss.fail() || ss.get(tmp)); + /** Default handler that triggers a compiler error */ + template + T handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) + { + T::unsupported_handleOption_specialization; + return *static_cast(nullptr); } - static bool containsMinus(const std::string& str) { - return str.find('-') != std::string::npos; + /** Handle a string option by returning it as is. */ + template <> + std::string handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) + { + return optionarg; } - - static T handle(const std::string& option, const std::string& flag, const std::string& optionarg) { - try { - T i; - bool success = stringToInt(i, optionarg); - - if(!success){ - throw OptionException(flag + ": failed to parse "+ optionarg + - " as an integer of the appropriate type."); - } - - // Depending in the platform unsigned numbers with '-' signs may parse. - // Reject these by looking for any minus if it is not signed. - if( (! std::numeric_limits::is_signed) && containsMinus(optionarg) ) { - // unsigned type but user gave negative argument - throw OptionException(flag + " requires a nonnegative argument"); - } else if(i < std::numeric_limits::min()) { - // negative overflow for type - std::stringstream ss; - ss << flag << " requires an argument >= " - << std::numeric_limits::min(); - throw OptionException(ss.str()); - } else if(i > std::numeric_limits::max()) { - // positive overflow for type - std::stringstream ss; - ss << flag << " requires an argument <= " - << std::numeric_limits::max(); - throw OptionException(ss.str()); - } - - return i; - - // if(std::numeric_limits::is_signed) { - // return T(i.getLong()); - // } else { - // return T(i.getUnsignedLong()); - // } - } catch(std::invalid_argument&) { - // user gave something other than an integer - throw OptionException(flag + " requires an integer argument"); + /** Handle a bool option, recognizing "true" or "false". */ + template <> + bool handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) + { + if (optionarg == "true") + { + return true; } - } -};/* struct OptionHandler */ - -/** Variant for numeric but non-integral C++ types */ -template -struct OptionHandler { - static T handle(const std::string& option, const std::string& flag, const std::string& optionarg) { - std::stringstream inss(optionarg); - long double r; - inss >> r; - if(! inss.eof()) { - // we didn't consume the whole string (junk at end) - throw OptionException(flag + " requires a numeric argument"); + if (optionarg == "false") + { + return false; } + throw OptionException("Argument '" + optionarg + "' for bool option " + flag + + " is not a bool constant"); + } - if(! std::numeric_limits::is_signed && r < 0.0) { - // unsigned type but user gave negative value - throw OptionException(flag + " requires a nonnegative argument"); - } else if(r < -std::numeric_limits::max()) { - // negative overflow for type - std::stringstream ss; - ss << flag << " requires an argument >= " - << -std::numeric_limits::max(); - throw OptionException(ss.str()); - } else if(r > std::numeric_limits::max()) { - // positive overflow for type - std::stringstream ss; - ss << flag << " requires an argument <= " - << std::numeric_limits::max(); - throw OptionException(ss.str()); - } + /** Handle a double option, using `parseNumber` with `std::stod`. */ + template <> + double handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) + { + return parseNumber( + flag, + optionarg, + [](const auto& s, auto p) { return std::stod(s, p); }, + "double"); + } - return T(r); + /** Handle a int64_t option, using `parseNumber` with `std::stoll`. */ + template <> + int64_t handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) + { + return parseNumber( + flag, + optionarg, + [](const auto& s, auto p) { return std::stoll(s, p); }, + "int64_t"); } -};/* struct OptionHandler */ -/** Variant for non-numeric C++ types */ -template -struct OptionHandler { - static T handle(const std::string& option, const std::string& flag, const std::string& optionarg) { - T::unsupported_handleOption_call___please_write_me; - // The above line causes a compiler error if this version of the template - // is ever instantiated (meaning that a specialization is missing). So - // don't worry about the segfault in the next line, the "return" is only - // there to keep the compiler from giving additional, distracting errors - // and warnings. - return *(T*)0; + /** Handle a uint64_t option, using `parseNumber` with `std::stoull`. */ + template <> + uint64_t handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) + { + return parseNumber( + flag, + optionarg, + [](const auto& s, auto p) { return std::stoull(s, p); }, + "uint64_t"); } -};/* struct OptionHandler */ -/** Specialization for ManagedErr */ -template <> -struct OptionHandler -{ - static ManagedErr handle(const std::string& option, - const std::string& flag, - const std::string& optionarg) + /** Handle a ManagedIn option. */ + template <> + ManagedIn handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) { - ManagedErr res; + ManagedIn res; res.open(optionarg); return res; } -}; -/** Specialization for ManagedIn */ -template <> -struct OptionHandler -{ - static ManagedIn handle(const std::string& option, - const std::string& flag, - const std::string& optionarg) + + /** Handle a ManagedErr option. */ + template <> + ManagedErr handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) { - ManagedIn res; + ManagedErr res; res.open(optionarg); return res; } -}; -/** Specialization for ManagedOut */ -template <> -struct OptionHandler -{ - static ManagedOut handle(const std::string& option, - const std::string& flag, - const std::string& optionarg) + + /** Handle a ManagedOut option. */ + template <> + ManagedOut handleOption(const std::string& option, + const std::string& flag, + const std::string& optionarg) { ManagedOut res; res.open(optionarg); return res; } -}; - -/** Handle an option of type T in the default way. */ -template -T handleOption(const std::string& option, const std::string& flag, const std::string& optionarg) { - return OptionHandler::is_specialized, std::numeric_limits::is_integer>::handle(option, flag, optionarg); -} - -/** Handle an option of type std::string in the default way. */ -template <> -std::string handleOption(const std::string& option, const std::string& flag, const std::string& optionarg) { - return optionarg; -} -template <> -bool handleOption(const std::string& option, const std::string& flag, const std::string& optionarg) { - Assert(optionarg == "true" || optionarg == "false"); - return optionarg == "true"; -} + } std::string get(const Options& options, const std::string& name) { -- 2.30.2