aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2020-03-19 09:27:22 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2020-03-19 09:27:22 +0200
commitcbcb0b03501ce346ca3778624dcf908e851e6e2e (patch)
treeec7cf219ea6772ed5ea03de35eefd5e781d7906a
parent14b34a239fa23e1a28519ab87f450c0a440d4f85 (diff)
Tweak lookup_config() semantics some more
-rw-r--r--libbuild2/config/utility.cxx28
-rw-r--r--libbuild2/config/utility.hxx14
-rw-r--r--libbuild2/scope.cxx16
-rw-r--r--libbuild2/scope.hxx24
-rw-r--r--libbuild2/variable.hxx12
-rw-r--r--libbuild2/variable.txx14
6 files changed, 82 insertions, 26 deletions
diff --git a/libbuild2/config/utility.cxx b/libbuild2/config/utility.cxx
index cf0f73b..b3f94be 100644
--- a/libbuild2/config/utility.cxx
+++ b/libbuild2/config/utility.cxx
@@ -33,9 +33,31 @@ namespace build2
if (var.overrides != nullptr)
{
- pair<lookup, size_t> ovr (rs.lookup_override (var, move (org)));
-
- if (l != ovr.first) // Overriden?
+ // This is tricky: if we didn't find the original, pretend we have set
+ // the default value for the purpose of override lookup in order to
+ // have consistent semantics with the default value case (see notes in
+ // that implementation for background).
+ //
+ // In particular, this makes sure we can first do the lookup without
+ // the default value and then, if there is no value, call the version
+ // with the default value and end up with the same result if we called
+ // the default value version straight away.
+ //
+ // Note that we need to detect both when the default value is not
+ // overridden as well as when the override is based on it (e.g., via
+ // append; think config.cxx+=-m32).
+ //
+ // @@ Maybe a callback that computes the default value on demand is a
+ // better way?
+ //
+ variable_map::value_data v; // NULL value, but must be with version.
+ if (!l.defined ())
+ org = make_pair (lookup (v, var, rs), 1); // As default value case.
+
+ scope::override_info li (rs.lookup_override_info (var, move (org)));
+ pair<lookup, size_t>& ovr (li.lookup);
+
+ if (l.defined () ? l != ovr.first : !li.original) // Overriden?
{
// Override is always treated as new.
//
diff --git a/libbuild2/config/utility.hxx b/libbuild2/config/utility.hxx
index bf4728f..cbfe588 100644
--- a/libbuild2/config/utility.hxx
+++ b/libbuild2/config/utility.hxx
@@ -91,10 +91,16 @@ namespace build2
//
// 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
+ // value. This can be useful when we don't have a default value or in case
+ // we want the mentioning of the variable to be omitted from persistent
// storage (e.g., a config file) if the default value is used.
//
+ // Note also that we can first do the lookup without the default value and
+ // then, if there is no value, call the version with the default value and
+ // end up with the same result if we called the default value version
+ // straight away. This is useful when computing the default value is
+ // expensive.
+ //
// @@ Should we pass flags and interpret save_null_omitted to treat null
// as undefined? Sounds logical.
//
@@ -135,7 +141,9 @@ namespace build2
// 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.
+ // such variables absence of a value means it is the default value. This
+ // flag is normally used for dynamically adjusting (e.g., hinted) default
+ // values.
//
// 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),
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index 51704fb..d18c4eb 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -175,11 +175,11 @@ namespace build2
return make_pair (lookup_type (), size_t (~0));
}
- pair<lookup, size_t> scope::
- lookup_override (const variable& var,
- pair<lookup_type, size_t> original,
- bool target,
- bool rule) const
+ auto scope::
+ lookup_override_info (const variable& var,
+ const pair<lookup_type, size_t> original,
+ bool target,
+ bool rule) const -> override_info
{
assert (!rule || target); // Rule-specific is target-specific.
@@ -346,7 +346,7 @@ namespace build2
}
if (!apply)
- return original;
+ return override_info {original, orig.defined ()};
assert (inner_vars != nullptr);
@@ -567,7 +567,9 @@ namespace build2
// Use the location of the innermost value that contributed as the
// location of the result.
//
- return make_pair (lookup_type (&cv, &var, vars), depth);
+ return override_info {
+ make_pair (lookup_type (&cv, &var, vars), depth),
+ orig.defined () && stem == orig};
}
value& scope::
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 97ec448..c49778f 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -151,10 +151,30 @@ namespace build2
size_t start_depth = 1) const;
pair<lookup_type, size_t>
- lookup_override (const variable&,
+ lookup_override (const variable& var,
pair<lookup_type, size_t> original,
bool target = false,
- bool rule = false) const;
+ bool rule = false) const
+ {
+ return lookup_override_info (var, original, target, rule).lookup;
+ }
+
+ // As above but also return an indication of whether the resulting value
+ // is/is based (e.g., via append/prepend overrides) on the original or an
+ // "outright" override. Note that it will always be false if there is no
+ // original.
+ //
+ struct override_info
+ {
+ pair<lookup_type, size_t> lookup;
+ bool original;
+ };
+
+ override_info
+ lookup_override_info (const variable&,
+ pair<lookup_type, size_t> original,
+ bool target = false,
+ bool rule = false) const;
// Return a value suitable for assignment (or append if you only want to
// append to the value from this scope). If the value does not exist in
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index 9b9880a..bf0aa00 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -1523,7 +1523,11 @@ namespace build2
// then typify the cached value.
//
pair<value&, ulock>
- insert (context&, K, const lookup& stem, size_t version, const variable&);
+ insert (context&,
+ K,
+ const lookup& stem,
+ size_t base_version,
+ const variable&);
private:
struct entry_type
@@ -1535,7 +1539,7 @@ namespace build2
//
variable_map::value_data value;
- size_t version = 0; // Version on which this value is based.
+ size_t base_version = 0; // Version on which this value is based.
// Location of the stem as well as the version on which this cache
// value is based. Used to track the location and value of the stem
@@ -1548,11 +1552,11 @@ namespace build2
//
entry_type () = default;
entry_type (variable_map::value_data val,
- size_t ver,
+ size_t bver,
const variable_map* svars,
size_t sver)
: value (move (val)),
- version (ver),
+ base_version (bver),
stem_vars (svars),
stem_version (sver) {}
};
diff --git a/libbuild2/variable.txx b/libbuild2/variable.txx
index 26f00ab..51176ae 100644
--- a/libbuild2/variable.txx
+++ b/libbuild2/variable.txx
@@ -766,7 +766,7 @@ namespace build2
insert (context& ctx,
K k,
const lookup& stem,
- size_t ver,
+ size_t bver,
const variable& var)
{
using value_data = variable_map::value_data;
@@ -788,7 +788,7 @@ namespace build2
// Cache hit.
//
if (i != m_.end () &&
- i->second.version == ver &&
+ i->second.base_version == bver &&
i->second.stem_vars == svars &&
i->second.stem_version == sver &&
(var.type == nullptr || i->second.value.type == var.type))
@@ -807,7 +807,7 @@ namespace build2
if (p.second)
p = m_.emplace (move (k),
- entry_type {value_data (nullptr), ver, svars, sver});
+ entry_type {value_data (nullptr), bver, svars, sver});
entry_type& e (p.first->second);
@@ -817,14 +817,14 @@ namespace build2
//
e.value.version++; // New value.
}
- else if (e.version != ver ||
- e.stem_vars != svars ||
+ else if (e.base_version != bver ||
+ e.stem_vars != svars ||
e.stem_version != sver)
{
// Cache invalidation.
//
- assert (e.version <= ver);
- e.version = ver;
+ assert (e.base_version <= bver);
+ e.base_version = bver;
if (e.stem_vars != svars)
e.stem_vars = svars;