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/utility.txx | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) (limited to 'libbuild2/config/utility.txx') 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