From 9f71deeeb0f8e6fe2c29f209fc96f466fc2831b6 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 16 Mar 2020 08:06:15 +0200 Subject: Rework config::{omitted,required,optional}() into unified config_lookup() --- libbuild2/config/operation.cxx | 10 +- libbuild2/config/utility.cxx | 17 +-- libbuild2/config/utility.hxx | 227 +++++++++++++++++++++++++++-------------- libbuild2/config/utility.ixx | 62 +++++++++++ libbuild2/config/utility.txx | 39 +++++-- 5 files changed, 251 insertions(+), 104 deletions(-) create mode 100644 libbuild2/config/utility.ixx (limited to 'libbuild2/config') diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx index 07110e0..58f6aae 100644 --- a/libbuild2/config/operation.cxx +++ b/libbuild2/config/operation.cxx @@ -1219,13 +1219,13 @@ namespace build2 // configurations). // create_project (d, - dir_path (), /* amalgamation */ + dir_path (), /* amalgamation */ bmod, - "", /* root_pre */ + "", /* root_pre */ rmod, - "", /* root_post */ - "config", /* config */ - true, /* buildfile */ + "", /* root_post */ + string ("config"), /* config_module */ + true, /* buildfile */ "the create meta-operation"); save_config (ctx, d); diff --git a/libbuild2/config/utility.cxx b/libbuild2/config/utility.cxx index b9fc513..8ad150e 100644 --- a/libbuild2/config/utility.cxx +++ b/libbuild2/config/utility.cxx @@ -17,9 +17,9 @@ namespace build2 namespace config { pair - omitted (scope& rs, const variable& var) + lookup_config_impl (scope& rs, const variable& var) { - // This is a stripped-down version of the required()'s twisted logic. + // This is a stripped-down version of the default value case. pair org (rs.find_original (var)); @@ -50,19 +50,8 @@ namespace build2 return pair (l, n); } - lookup - optional (scope& rs, const variable& var) - { - save_variable (rs, var); - - auto l (rs[var]); - return l.defined () - ? l - : lookup (rs.assign (var), var, rs); // NULL. - } - bool - specified (scope& rs, const string& n) + specified_config (scope& rs, const string& n) { // Search all outer scopes for any value in this namespace. // diff --git a/libbuild2/config/utility.hxx b/libbuild2/config/utility.hxx index 493d296..bf4728f 100644 --- a/libbuild2/config/utility.hxx +++ b/libbuild2/config/utility.hxx @@ -77,116 +77,184 @@ namespace build2 config_save_module (rs, module, prio); } - // Set, if necessary, a required config.* variable. + // Lookup a config.* variable value and, if the value is defined, mark it + // as saved. + // + // The second version in addition sets the new_value argument to true if + // the value is "new" (but not to false; so it can be used to accumulate + // the result from multiple calls). A value is considered new if it was + // set to the default value (inherited or not, including overrides). We + // also treat command line overrides (inherited or not) as new. In this + // case new means either the default value was inherited or it was + // overridden. This flag is usually used to test that the new value is + // valid, print the configuration report, etc. + // + // Unlike the rest of the lookup_config() versions, this one leaves the + // unspecified value as undefined rather than setting it to a default + // value. This can be useful when we don't have a default value or if we + // want the mentioning of the variable to be omitted from persistent + // storage (e.g., a config file) if the default value is used. + // + // @@ Should we pass flags and interpret save_null_omitted to treat null + // as undefined? Sounds logical. + // + lookup + lookup_config (scope& rs, const variable&); + + lookup + lookup_config (bool& new_value, scope& rs, const variable&); + + // Note that the variable is expected to have already been entered. + // + inline lookup + lookup_config (scope& rs, const string& var) + { + return lookup_config (rs, rs.ctx.var_pool[var]); + } + + inline lookup + lookup_config (bool& new_value, scope& rs, const string& var) + { + return lookup_config (new_value, rs, rs.ctx.var_pool[var]); + } + + // Lookup a config.* variable value and, if the value is undefined, set it + // to the default. Always mark it as saved. + // + // If the default value is nullptr, then the unspecified value is set to + // NULL which can be used to distinguish between the "not yet configured", + // "configured as unspecified", and "configures as empty" cases which can + // have different semantics if the value is merged into a non-config.* + // variable. This default value is traditionally used for "optional" + // values such as command line options. + // + // The value is returned as lookup (even though it is always defined + // though potentially as NULL) in order to pass along its location (could + // be used to detect inheritance, etc). + // + // The second version in addition sets the new_value argument as described + // above. Note, however, that if the save_default_commented flag is + // specified, then the default value is never considered "new" since for + // such variables absence of a value means it is the default value. // // If override is true and the variable doesn't come from this root scope // or from the command line (i.e., it is inherited from the amalgamation), // then its value is "overridden" to the default value on this root scope. - // See save_variable() for more information on save_flags. - // - // Return the reference to the value as well as the indication of whether - // the value is "new", that is, it was set to the default value (inherited - // or not, including overrides). We also treat command line overrides - // (inherited or not) as new. This flag is usually used to test that the - // new value is valid, print report, etc. We return the value as lookup - // (always defined) to pass along its location (could be used to detect - // inheritance, etc). - // - // Note also that if save_flags has save_default_commented, then a default - // value is never considered "new" since for such variables absence of a - // value means the default value. // // @@ Should save_null_omitted be interpreted to treat null as undefined? // Sounds logical. // template - pair - required (scope& rs, - const variable&, - T&& default_value, - bool override = false, - uint64_t save_flags = 0); + lookup + lookup_config (scope& rs, + const variable&, + T&& default_value, + uint64_t save_flags = 0, + bool override = false); - // Note that the variable is expected to have already been entered. - // template - inline pair - required (scope& rs, - const string& var, - T&& default_value, - bool override = false, - uint64_t save_flags = 0) + lookup + lookup_config (bool& new_value, + scope& rs, + const variable&, + T&& default_value, + uint64_t save_flags = 0, + bool override = false); + + inline lookup + lookup_config (scope& rs, + const variable& var, + const char* default_value, + uint64_t save_flags = 0, + bool override = false) { - return required (rs, - rs.ctx.var_pool[var], - std::forward (default_value), // VC14 - override, - save_flags); + return lookup_config ( + rs, var, string (default_value), save_flags, override); } - inline pair - required (scope& rs, - const string& var, - const char* default_value, - bool override = false, - uint64_t save_flags = 0) + inline lookup + lookup_config (bool& new_value, + scope& rs, + const variable& var, + const char* default_value, + uint64_t save_flags = 0, + bool override = false) { - return required (rs, var, string (default_value), override, save_flags); + return lookup_config ( + new_value, rs, var, string (default_value), save_flags, override); } - // As above, but leave the unspecified value as undefined rather than - // setting it to the default value. - // - // This can be useful when we don't have a default value but may figure - // out some fallback. See config.bin.target for an example. - // - LIBBUILD2_SYMEXPORT pair - omitted (scope& rs, const variable&); - // Note that the variable is expected to have already been entered. // - inline pair - omitted (scope& rs, const string& var) + template + inline lookup + lookup_config (scope& rs, + const string& var, + T&& default_value, + uint64_t save_flags = 0, + bool override = false) { - return omitted (rs, rs.ctx.var_pool[var]); + return lookup_config (rs, + rs.ctx.var_pool[var], + std::forward (default_value), // VC14 + save_flags, + override); } - // Set, if necessary, an optional config.* variable. In particular, an - // unspecified variable is set to NULL which is used to distinguish - // between the "configured as unspecified" and "not yet configured" cases. - // - // Return the value (as always defined lookup), which can be NULL. - // - // @@ Rename since clashes with the optional class template. - // - // @@ Does it make sense to return the new indicator here as well, - // for consistency/generality. - // - LIBBUILD2_SYMEXPORT lookup - optional (scope& rs, const variable&); + template + inline lookup + lookup_config (bool& new_value, + scope& rs, + const string& var, + T&& default_value, + uint64_t save_flags = 0, + bool override = false) + { + return lookup_config (new_value, + rs, + rs.ctx.var_pool[var], + std::forward (default_value), // VC14 + save_flags, + override); + } + + inline lookup + lookup_config (scope& rs, + const string& var, + const char* default_value, + uint64_t save_flags = 0, + bool override = false) + { + return lookup_config ( + rs, var, string (default_value), save_flags, override); + } - // Note that the variable is expected to have already been registered. - // inline lookup - optional (scope& rs, const string& var) + lookup_config (bool& new_value, + scope& rs, + const string& var, + const char* default_value, + uint64_t save_flags = 0, + bool override = false) { - return optional (rs, rs.ctx.var_pool[var]); + return lookup_config ( + new_value, rs, var, string (default_value), save_flags, override); } - // Check whether there are any variables specified from the config + // Check whether there are any variables specified from the config. // namespace. The idea is that we can check if there are any, say, - // config.install.* values. If there are none, then we can assume - // this functionality is not (yet) used and omit writing a whole - // bunch of NULL config.install.* values to the config.build file. - // We call it omitted/delayed configuration. + // config.install.* values. If there are none, then we can assume this + // functionality is not (yet) used and omit writing a whole bunch of NULL + // config.install.* values to the config.build file. We call this + // omitted/delayed configuration. // - // Note that this function detects and ignores the special - // config.*.configured variable which may be used by a module to - // "remember" that it is unconfigured (e.g., in order to avoid re- - // running the tests, etc). + // Note that this function detects and ignores special config.* variables + // (such as config.*.configured) which may be used by a module to remember + // that it is unconfigured (e.g., in order to avoid re-running the tests, + // etc; see below). // LIBBUILD2_SYMEXPORT bool - specified (scope& rs, const string& var); + specified_config (scope& rs, const string& var); // Check if there is a false config.*.configured value. This mechanism can // be used to "remember" that the module is left unconfigured in order to @@ -204,6 +272,7 @@ namespace build2 } } +#include #include #endif // LIBBUILD2_CONFIG_UTILITY_HXX diff --git a/libbuild2/config/utility.ixx b/libbuild2/config/utility.ixx new file mode 100644 index 0000000..79d5470 --- /dev/null +++ b/libbuild2/config/utility.ixx @@ -0,0 +1,62 @@ +// file : libbuild2/config/utility.ixx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +namespace build2 +{ + namespace config + { + LIBBUILD2_SYMEXPORT pair + lookup_config_impl (scope&, const variable&); + + template + pair + lookup_config_impl (scope&, const variable&, T&&, uint64_t, bool); + + inline lookup + lookup_config (scope& rs, const variable& var) + { + return lookup_config_impl (rs, var).first; + } + + inline lookup + lookup_config (bool& new_value, scope& rs, const variable& var) + { + auto r (lookup_config_impl (rs, var)); + new_value = new_value || r.second; + return r.first; + } + + template + inline lookup + lookup_config (scope& rs, + const variable& var, + T&& def_val, + uint64_t sflags, + bool def_ovr) + { + return lookup_config_impl (rs, + var, + std::forward (def_val), // VC14 + sflags, + def_ovr).first; + } + + template + inline lookup + lookup_config (bool& new_value, + scope& rs, + const variable& var, + T&& def_val, + uint64_t sflags, + bool def_ovr) + { + auto r (lookup_config_impl (rs, + var, + std::forward (def_val), // VC14 + sflags, + def_ovr)); + new_value = new_value || r.second; + return r.first; + } + } +} diff --git a/libbuild2/config/utility.txx b/libbuild2/config/utility.txx index f52df8d..ae40ba7 100644 --- a/libbuild2/config/utility.txx +++ b/libbuild2/config/utility.txx @@ -7,13 +7,14 @@ namespace build2 { template pair - required (scope& rs, - const variable& var, - T&& def_val, - bool def_ovr, - uint64_t sflags) + lookup_config_impl (scope& rs, + const variable& var, + T&& def_val, + uint64_t sflags, + bool def_ovr) { - // Note: see also omitted() if changing anything here. + // Note: see also the other lookup_config() implementation if changing + // anything here. save_variable (rs, var, sflags); @@ -28,6 +29,32 @@ namespace build2 // are going to do is first ignore overrides and perform the normal // logic on the original. Then we apply the overrides on the result. // + // Note that this is not exactly the "lookup and set to default if + // undefined" semantics in case there is no original but there is an + // override. In this case we will set original to default and then apply + // the override, which could be append or non-recursive (as mentioned + // above). It does, however, feel like taking into account the default + // in such cases is the correct semantics since append is meant as an + // addition to something existing and non-recursive override is only + // meant to override at the level it was specified. Though it won't be + // surprising at all if we end up with some counter-intuitive behavior + // here. + // + // Actually, the above analysis is not the full picture: if we have one + // of those overrides (append, non-recursive) in the outer project, then + // the lookup_config() call at that level will set the corresponding + // variable on that scope and we will see it as "original-defined" from + // our scope. Of course if there is no call to lookup_config() for this + // variable in the outer scope, then we won't see anything but then our + // behavior in this case seems correct: since that value is not part of + // the configuration (and won't be saved), then we should stick to our + // default. In other words, we should only inherit the value if it is + // actually recognized as a configuration value by the outer project. + // + // So, to summarize the current understanding, while our semantics is + // not exactly "lookup and set to default if undefined" in some obscure + // corner cases, it seem to be the correct/preferred one. + // if (!l.defined () || (def_ovr && !l.belongs (rs))) { value& v (rs.assign (var) = std::forward (def_val)); // VC14 -- cgit v1.1