From: Gabriel Corona Date: Fri, 15 Apr 2016 11:10:56 +0000 (+0200) Subject: Rewrite/simplify the C++ flag declaration X-Git-Tag: v3_13~78^2 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/9d0bdd2e04d23fbcc1db6e7e525cf79979c4643b?hp=9d446af3d297ad838f88db08fc5706f386362ddc Rewrite/simplify the C++ flag declaration C++-style flags are all registered as xbt_cfgelm_string. I'd be in favor of removing the other types in order to simplify the code. --- diff --git a/include/xbt/config.h b/include/xbt/config.h index 4061014295..a5ab70f931 100644 --- a/include/xbt/config.h +++ b/include/xbt/config.h @@ -132,8 +132,6 @@ struct xbt_boolean_couple { /** \brief Callback types. They get the name of the modified entry, and the position of the changed value */ typedef void (*xbt_cfg_cb_t) (const char * name); -typedef void (*xbt_cfg_cb_ext_t)(const char * name, void* cb_data); -typedef void (*xbt_cfg_cb_free_t)(void* cb_data); XBT_PUBLIC(xbt_cfg_t) xbt_cfg_new(void); XBT_PUBLIC(void) xbt_cfg_free(xbt_cfg_t * cfg); @@ -155,10 +153,6 @@ XBT_PUBLIC(void) xbt_cfg_register_boolean(const char *name, const char*default_v XBT_PUBLIC(void) xbt_cfg_register_alias(const char *newname, const char *oldname); XBT_PUBLIC(void) xbt_cfg_register_str(xbt_cfg_t * cfg, const char *entry); -XBT_PUBLIC(void) xbt_cfg_register_ext( - const char *name, const char *desc, e_xbt_cfgelm_type_t type, - xbt_cfg_cb_ext_t cb, void* data, xbt_cfg_cb_free_t data_free); - XBT_PUBLIC(void) xbt_cfg_aliases(void); XBT_PUBLIC(void) xbt_cfg_help(void); XBT_PUBLIC(e_xbt_cfgelm_type_t) xbt_cfg_get_type(xbt_cfg_t cfg, const char *name); diff --git a/include/xbt/config.hpp b/include/xbt/config.hpp index f87f525c38..af350a5307 100644 --- a/include/xbt/config.hpp +++ b/include/xbt/config.hpp @@ -7,9 +7,13 @@ #ifndef _XBT_CONFIG_HPP_ #define _XBT_CONFIG_HPP_ +#include + #include +#include #include #include +#include #include #include @@ -17,92 +21,70 @@ namespace simgrid { namespace config { -/** Get the base type of a e_xbt_cfgelm_type_t - * - * * `type` is the type used in the config framework to store - * the values of this type; - * - * * `get()` is used to get such a value from the configuration. - */ -template -struct base_type; -template<> struct base_type { - typedef bool type; - static inline bool get(const char* name) +bool parseBool(const char* value); +double parseDouble(const char* value); +long int parseLong(const char* value); + +template struct parse_option { + static inline T parse(const char* value) { - return xbt_cfg_get_boolean(name) ? true : false; + return T(value); } }; -template<> struct base_type { - typedef int type; - static inline int get(const char* name) + +template<> struct parse_option { + static inline std::string parse(const char* value) { - return xbt_cfg_get_int(name); + return std::string(value); } }; -template<> struct base_type { - typedef double type; - static inline double get(const char* name) + +template<> +struct parse_option { + static inline double parse(const char* value) { - return xbt_cfg_get_double(name); + return parseDouble(value); } }; -template<> struct base_type { - typedef std::string type; - static inline std::string get(const char* name) + +template<> +struct parse_option { + static inline double parse(const char* value) { - char* value = xbt_cfg_get_string(name); - return value != nullptr ? value : ""; + return parseLong(value); + } +}; + +template<> +struct parse_option { + static inline bool parse(const char* value) + { + return parseBool(value); } }; -/** Associate the e_xbt_cfgelm_type_t to use for a given type */ -template -struct cfgelm_type : public std::integral_constant::value ? xbt_cfgelm_boolean : - std::is_integral::value ? xbt_cfgelm_int : - std::is_floating_point::value ? xbt_cfgelm_double : - xbt_cfgelm_string> -{}; - -/** Get a value of a given type from the configuration */ -template inline -T get(const char* name) -{ - return T(base_type::value>::get(name)); -} template inline -T get(std::string const& name) +T parse(const char* value) { - return T(base_type::value>::get(name.c_str())); + return parse_option::parse(value); } -inline void setDefault(const char* name, int value) -{ - xbt_cfg_setdefault_int(name, value); -} -inline void setDefault(const char* name, double value) -{ - xbt_cfg_setdefault_double(name, value); -} -inline void setDefault(const char* name, const char* value) +template inline +std::string to_string(T&& value) { - xbt_cfg_setdefault_string(name, value); + return std::to_string(std::forward(value)); } -inline void setDefault(const char* name, std::string const& value) +inline std::string const& to_string(std::string& value) { - xbt_cfg_setdefault_string(name, value.c_str()); + return value; } -inline void setDefault(const char* name, bool value) +inline std::string const& to_string(std::string const& value) { - xbt_cfg_setdefault_boolean(name, value ? "yes" : "no"); + return value; } - -/** Set the default value of a given configuration option */ -template inline -void setDefault(const char* name, T value) +inline std::string to_string(std::string&& value) { - setDefault(name, base_type::value>::type(value)); + return std::move(value); } // Register: @@ -111,12 +93,10 @@ void setDefault(const char* name, T value) * * @param name name of the option * @param description Description of the option - * @param type config storage type for the option - * @param callback called with the option name (expected to use `simgrid::config::get`) + * @param callback called with the option value */ -XBT_PUBLIC(void) registerConfig(const char* name, const char* description, - e_xbt_cfgelm_type_t type, - std::function callback); +XBT_PUBLIC(void) declareFlag(const char* name, const char* description, + std::function callback); /** Bind a variable to configuration flag * @@ -125,30 +105,91 @@ XBT_PUBLIC(void) registerConfig(const char* name, const char* description, * @param description Option description */ template -void declareFlag(T& value, const char* name, const char* description) +void bindFlag(T& value, const char* name, const char* description) { - registerConfig(name, description, cfgelm_type::value, - [&value](const char* name) { - value = simgrid::config::get(name); - }); - setDefault(name, value); + using namespace std; + declareFlag(name, description, [&value](const char* val) { + value = simgrid::config::parse(val); + }); + xbt_cfg_setdefault_string(name, simgrid::config::to_string(value).c_str()); } /** Bind a variable to configuration flag * - * @param value Bound variable - * @param name Flag name - * @param description Option description - * @f Callback (called with the option name) providing the value + * 

+ *  static int x;
+ *  simgrid::config::bindFlag(a, "x", [](const char* value) {
+ *    return simgrid::config::parse(value);
+ *  }
+ *
+ */ +// F is a parser, F : const char* -> T +template +typename std::enable_if()(std::declval()) + ) >::type +>::value, void>::type +bindFlag(T& value, const char* name, const char* description, + F callback) +{ + declareFlag(name, description, [&value,callback](const char* val) { + value = callback(val); + }); + xbt_cfg_setdefault_string(name, to_string(value).c_str()); +} + +/** Bind a variable to configuration flag + * + * 

+ *  static int x;
+ *  simgrid::config::bindFlag(a, "x", [](int x) {
+ *    if (x < x_min || x => x_max)
+ *      throw std::range_error("must be in [x_min, x_max)")
+ *  });
+ *
*/ +// F is a checker, F : T& -> () template -void declareFlag(T& value, const char* name, const char* description, F f) +typename std::enable_if()(std::declval()) ) +>::value, void>::type +bindFlag(T& value, const char* name, const char* description, + F callback) { - registerConfig(name, description, cfgelm_type::value, - [&value,f](const char* name) { - value = f(name); - }); - setDefault(name, value); + declareFlag(name, description, [&value,callback](const char* val) { + T res = parse(val); + callback(res); + value = std::move(res); + }); + xbt_cfg_setdefault_string(name, to_string(value).c_str()); +} + +/** Bind a variable to configuration flag + * + * 

+ *  static int x;
+ *  simgrid::config::bindFlag(a, "x", [](int x) { return return x > 0; });
+ *
+ */ +// F is a predicate, F : T const& -> bool +template +typename std::enable_if()(std::declval()) ) +>::value, void>::type +bindFlag(T& value, const char* name, const char* description, + F callback) +{ + declareFlag(name, description, [&value,callback](const char* val) { + T res = parse(val); + if (!callback(res)) + throw std::range_error("invalid value"); + value = std::move(res); + }); + xbt_cfg_setdefault_string(name, to_string(value).c_str()); } /** A variable bound to a CLI option @@ -160,7 +201,7 @@ void declareFlag(T& value, const char* name, const char* description, F f) * */ template -class flag { +class Flag { T value_; public: @@ -170,14 +211,20 @@ public: * @param desc Flag description * @param value Flag initial/default value */ - flag(const char* name, const char* desc, T value) : value_(value) + Flag(const char* name, const char* desc, T value) : value_(value) { - declareFlag(value_, name, desc); + simgrid::config::bindFlag(value_, name, desc); + } + + template + Flag(const char* name, const char* desc, T value, F callback) : value_(value) + { + simgrid::config::bindFlag(value_, name, desc, std::move(callback)); } // No copy: - flag(flag const&) = delete; - flag& operator=(flag const&) = delete; + Flag(Flag const&) = delete; + Flag& operator=(Flag const&) = delete; // Get the underlying value: T& get() { return value_; } @@ -186,6 +233,16 @@ public: // Implicit conversion to the underlying type: operator T&() { return value_; } operator T const&() const{ return value_; } + + // Basic interop with T: + Flag& operator=(T const& that) { value_ = that; return *this; } + Flag& operator=(T && that) { value_ = that; return *this; } + bool operator==(T const& that) const { return value_ == that; } + bool operator!=(T const& that) const { return value_ != that; } + bool operator<(T const& that) const { return value_ < that; } + bool operator>(T const& that) const { return value_ > that; } + bool operator<=(T const& that) const { return value_ <= that; } + bool operator>=(T const& that) const { return value_ >= that; } }; } diff --git a/src/xbt/config.cpp b/src/xbt/config.cpp index 60a7f5193f..7578fbdfa5 100644 --- a/src/xbt/config.cpp +++ b/src/xbt/config.cpp @@ -3,7 +3,11 @@ #include +#include +#include +#include #include +#include #include #include @@ -24,7 +28,6 @@ XBT_EXPORT_NO_IMPORT(xbt_cfg_t) simgrid_config = NULL; namespace { -// We could define some range/iterator for the num values. static inline void increment(e_xbt_cfgelm_type_t& type) { @@ -44,13 +47,11 @@ typedef struct s_xbt_cfgelm_t { e_xbt_cfgelm_type_t type; bool isdefault = true; - /* Callbacks */ + /* Callback */ xbt_cfg_cb_t cb_set = nullptr; - /* Advanced callbacks */ - xbt_cfg_cb_ext_t cb_set_ext = nullptr; - void* cb_set_data = nullptr; - xbt_cfg_cb_free_t cb_set_free = nullptr; + /* Advanced callback (for xbt_cfgelm_string only) */ + std::function callback; /* actual content (could be an union or something) */ xbt_dynar_t content = nullptr; @@ -58,10 +59,6 @@ typedef struct s_xbt_cfgelm_t { ~s_xbt_cfgelm_t() { XBT_DEBUG("Frees cfgelm %p", this); - if (!this) - return; - if (this->cb_set_free) - this->cb_set_free(this->cb_set_data); if (this->type != xbt_cfgelm_alias) xbt_dynar_free(&(this->content)); } @@ -81,7 +78,8 @@ const struct xbt_boolean_couple xbt_cfgelm_boolean_values[] = { /* Internal stuff used in cache to free a variable */ static void xbt_cfgelm_free(void *data) { - delete (xbt_cfgelm_t) data; + if (data) + delete (xbt_cfgelm_t) data; } /* Retrieve the variable we'll modify */ @@ -185,7 +183,7 @@ void xbt_cfg_dump(const char *name, const char *indent, xbt_cfg_t cfg) static void xbt_cfg_register( xbt_cfg_t * cfg, const char *name, const char *desc, e_xbt_cfgelm_type_t type, xbt_cfg_cb_t cb_set, - xbt_cfg_cb_ext_t cb_set_ext, void* cb_set_data, xbt_cfg_cb_free_t cb_set_free) + std::function callback = std::function()) { if (*cfg == NULL) *cfg = xbt_cfg_new(); @@ -200,11 +198,10 @@ static void xbt_cfg_register( XBT_DEBUG("Register cfg elm %s (%s) (%s (=%d) @%p in set %p)", name, desc, xbt_cfgelm_type_name[type], (int)type, res, *cfg); res->type = type; - res->desc = desc; + if (desc) + res->desc = desc; res->cb_set = cb_set; - res->cb_set_ext = cb_set_ext; - res->cb_set_data = cb_set_data; - res->cb_set_free = cb_set_free; + res->callback = std::move(callback); switch (type) { case xbt_cfgelm_int: @@ -228,37 +225,22 @@ static void xbt_cfg_register( } void xbt_cfg_register_double(const char *name, double default_value,xbt_cfg_cb_t cb_set, const char *desc){ - xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_double,cb_set, NULL, NULL, NULL); + xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_double,cb_set); xbt_cfg_setdefault_double(name, default_value); } void xbt_cfg_register_int(const char *name, int default_value,xbt_cfg_cb_t cb_set, const char *desc) { - xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_int,cb_set, NULL, NULL, NULL); + xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_int,cb_set); xbt_cfg_setdefault_int(name, default_value); } void xbt_cfg_register_string(const char *name, const char *default_value, xbt_cfg_cb_t cb_set, const char *desc){ - xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_string,cb_set, NULL, NULL, NULL); + xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_string,cb_set); xbt_cfg_setdefault_string(name, default_value); } void xbt_cfg_register_boolean(const char *name, const char*default_value,xbt_cfg_cb_t cb_set, const char *desc){ - xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_boolean,cb_set, NULL, NULL, NULL); + xbt_cfg_register(&simgrid_config,name,desc,xbt_cfgelm_boolean,cb_set); xbt_cfg_setdefault_boolean(name, default_value); } -/** Register a config with an extended callback - * - * @param name Name of the flag - * @param desc Description of the flag - * @param type Type of the flag - * @param cb Extended callback - * @param data Data associated with the callback - * @param data_free Function used to free the callback data (or NULL) - */ -void xbt_cfg_register_ext(const char *name, const char *desc, e_xbt_cfgelm_type_t type, - xbt_cfg_cb_ext_t cb, void* data, xbt_cfg_cb_free_t data_free) -{ - xbt_cfg_register(&simgrid_config, name, desc, type, NULL, cb, data, data_free); -} - void xbt_cfg_register_alias(const char *newname, const char *oldname) { if (simgrid_config == NULL) @@ -308,7 +290,7 @@ void xbt_cfg_register_str(xbt_cfg_t * cfg, const char *entry) xbt_assert(type < xbt_cfgelm_type_count, "Invalid type in config element descriptor: %s; Should be one of 'string', 'int' or 'double'.", entry); - xbt_cfg_register(cfg, entrycpy, NULL, type, NULL, NULL, NULL, NULL); + xbt_cfg_register(cfg, entrycpy, NULL, type, NULL); free(entrycpy); /* strdup'ed by dict mechanism, but cannot be const */ } @@ -708,8 +690,6 @@ void xbt_cfg_set_int(const char *name, int val) if (variable->cb_set) variable->cb_set(name); - if (variable->cb_set_ext) - variable->cb_set_ext(name, variable->cb_set_data); variable->isdefault = false; } @@ -726,8 +706,6 @@ void xbt_cfg_set_double(const char *name, double val) if (variable->cb_set) variable->cb_set(name); - if (variable->cb_set_ext) - variable->cb_set_ext(name, variable->cb_set_data); variable->isdefault = false; } @@ -752,8 +730,22 @@ void xbt_cfg_set_string(const char *name, const char *val) if (variable->cb_set) variable->cb_set(name); - if (variable->cb_set_ext) - variable->cb_set_ext(name, variable->cb_set_data); + + if (variable->callback) { + try { + variable->callback(val); + } + catch(std::range_error& e) { + xbt_die("Invalid flag %s=%s: %s", val, name, e.what()); + } + catch(std::exception& e) { + xbt_die("Error for flag %s=%s: %s", val, name, e.what()); + } + catch(...) { + xbt_die("Error for flag %s=%s", val, name); + } + } + variable->isdefault = false; } @@ -782,8 +774,6 @@ void xbt_cfg_set_boolean(const char *name, const char *val) if (variable->cb_set) variable->cb_set(name); - if (variable->cb_set_ext) - variable->cb_set_ext(name, variable->cb_set_data); variable->isdefault = false; } @@ -883,33 +873,69 @@ int xbt_cfg_get_boolean(const char *name) namespace simgrid { namespace config { -static void callCallback(const char* name, void* data) -{ - (*(std::function*) data)(name); -} - -static void freeCallback(void* data) +bool parseBool(const char* value) { - delete (std::function*) data; + for (int i = 0; xbt_cfgelm_boolean_values[i].true_val != NULL; i++) { + if (std::strcmp(value, xbt_cfgelm_boolean_values[i].true_val) == 0) + return true; + if (std::strcmp(value, xbt_cfgelm_boolean_values[i].false_val) == 0) + return false; + } + throw std::range_error("not a boolean"); +} + +double parseDouble(const char* value) +{ + char* end; + errno = 0; + double res = std::strtod(value, &end); + if (errno == ERANGE) + throw std::range_error("out of range"); + else if (errno) + xbt_die("Unexpected errno"); + if (end == value || *end != '\0') + throw std::range_error("invalid double"); + else + return res; +} + +long int parseLong(const char* value) +{ + char* end; + errno = 0; + long int res = std::strtol(value, &end, 0); + if (errno) { + if (res == LONG_MIN && errno == ERANGE) + throw std::range_error("underflow"); + else if (res == LONG_MAX && errno == ERANGE) + throw std::range_error("overflow"); + xbt_die("Unexpected errno"); + } + if (end == value || *end != '\0') + throw std::range_error("invalid integer"); + else + return res; } -void registerConfig(const char* name, const char* description, - e_xbt_cfgelm_type_t type, - std::function callback) +void declareFlag(const char* name, const char* description, + std::function callback) { - std::function* code - = new std::function(std::move(callback)); - xbt_cfg_register_ext(name, description, type, - callCallback, code, freeCallback); + xbt_cfg_register(&simgrid_config, name, description, xbt_cfgelm_string, NULL, + std::move(callback)); } } } #ifdef SIMGRID_TEST + +#include + #include "xbt.h" #include "xbt/ex.h" +#include + XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(xbt_cfg); XBT_TEST_SUITE("config", "Configuration support"); @@ -964,4 +990,27 @@ XBT_TEST_UNIT("use", test_config_use, "Data retrieving tests") } xbt_cfg_free(&simgrid_config); } + +XBT_TEST_UNIT("c++flags", test_config_cxx_flags, "C++ flags") +{ + simgrid_config = make_set(); + xbt_test_add("C++ declaration of flags"); + + simgrid::config::Flag int_flag("int", "", 0); + simgrid::config::Flag string_flag("string", "", "foo"); + simgrid::config::Flag double_flag("double", "", 0.32); + simgrid::config::Flag bool_flag1("bool1", "", false); + simgrid::config::Flag bool_flag2("bool2", "", true); + + xbt_test_add("Parse values"); + xbt_cfg_set_parse("int:42 string:bar double:8.0 bool1:true bool2:false"); + xbt_test_assert(int_flag == 42, "Check int flag"); + xbt_test_assert(string_flag == "bar", "Check string flag"); + xbt_test_assert(double_flag == 8.0, "Check double flag"); + xbt_test_assert(bool_flag1, "Check bool1 flag"); + xbt_test_assert(!bool_flag2, "Check bool2 flag"); + + xbt_cfg_free(&simgrid_config); +} + #endif /* SIMGRID_TEST */