From 14b34a239fa23e1a28519ab87f450c0a440d4f85 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 18 Mar 2020 10:52:48 +0200 Subject: Cleanup variable type/visibility/overridability logic --- libbuild2/parser.cxx | 94 +++++++++++++++++++++++++++++--------------------- libbuild2/variable.cxx | 34 +++++++++--------- libbuild2/variable.hxx | 26 +++++++++----- 3 files changed, 89 insertions(+), 65 deletions(-) diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index e219e07..91f2d5c 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -2732,13 +2732,18 @@ namespace build2 const variable& parser:: parse_variable_name (names&& ns, const location& l) { + // Parse and enter a variable name for assignment (as opposed to lookup). + // The list should contain a single, simple name. // if (ns.size () != 1 || !ns[0].simple () || ns[0].empty ()) fail (l) << "expected variable name instead of " << ns; - return scope_->var_pool ().insert (move (ns[0].value), - true /* overridable */); + // Note that the overridability can still be restricted (e.g., by a module + // that enters this variable or by a pattern). + // + return scope_->var_pool ().insert ( + move (ns[0].value), true /* overridable */); } void parser:: @@ -2902,7 +2907,10 @@ namespace build2 return; const location& l (a.loc); + const value_type* type (nullptr); + optional vis; + optional ovr; auto print = [storage = names ()] (diag_record& dr, const value& v) mutable { @@ -2943,17 +2951,23 @@ namespace build2 } } - if (type != nullptr) + if (type != nullptr && var.type != nullptr) { - if (var.type == nullptr) - { - const bool o (true); // Allow overrides. - ctx.var_pool.update (const_cast (var), type, nullptr, &o); - } - else if (var.type != type) + if (var.type == type) + type = nullptr; + else fail (l) << "changing variable " << var << " type from " << var.type->name << " to " << type->name; } + + //@@ TODO: the same for vis and ovr. + + if (type || vis || ovr) + ctx.var_pool.update (const_cast (var), + type, + vis ? &*vis : nullptr, + ovr ? &*ovr : nullptr); + } void parser:: @@ -5720,46 +5734,46 @@ namespace build2 // Lookup. // - // @@ Why insert instead of find(), like in []-lookup? Also change to - // const string& name. - // - const variable& var (scope_->var_pool ().insert (move (name), true)); - - if (p != nullptr) + if (const variable* pvar = scope_->var_pool ().find (name)) { - // The lookup depth is a bit of a hack but should be harmless since - // unused. - // - pair r (p->vars[var], 1); + auto& var (*pvar); + + if (p != nullptr) + { + // The lookup depth is a bit of a hack but should be harmless since + // unused. + // + pair r (p->vars[var], 1); - if (!r.first.defined ()) - r = t->lookup_original (var); + if (!r.first.defined ()) + r = t->lookup_original (var); - return var.overrides == nullptr - ? r.first - : t->base_scope ().lookup_override (var, move (r), true).first; - } + return var.overrides == nullptr + ? r.first + : t->base_scope ().lookup_override (var, move (r), true).first; + } - if (t != nullptr) - { - if (var.visibility > variable_visibility::target) + if (t != nullptr) { - fail (loc) << "variable " << var << " has " << var.visibility - << " visibility but is expanded in target context"; - } + if (var.visibility > variable_visibility::target) + { + fail (loc) << "variable " << var << " has " << var.visibility + << " visibility but is expanded in target context"; + } - return (*t)[var]; - } + return (*t)[var]; + } - if (s != nullptr) - { - if (var.visibility > variable_visibility::scope) + if (s != nullptr) { - fail (loc) << "variable " << var << " has " << var.visibility - << " visibility but is expanded in scope context"; - } + if (var.visibility > variable_visibility::scope) + { + fail (loc) << "variable " << var << " has " << var.visibility + << " visibility but is expanded in scope context"; + } - return (*s)[var]; + return (*s)[var]; + } } return lookup (); diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx index f1c5515..48fcf40 100644 --- a/libbuild2/variable.cxx +++ b/libbuild2/variable.cxx @@ -1143,7 +1143,7 @@ namespace build2 // Check overridability (all overrides, if any, should already have // been entered; see context ctor for details). // - if (var.overrides != nullptr && (o == nullptr || !*o)) + if (o != nullptr && var.overrides != nullptr && !*o) fail << "variable " << var.name << " cannot be overridden"; bool ut (t != nullptr && var.type != t); @@ -1285,25 +1285,23 @@ namespace build2 var.aliases = &var; else // Note: overridden variable will always exist. { - if (pt != nullptr || pv != nullptr || po != nullptr) + // This is tricky: if the pattern does not require a match, then we + // should re-merge it with values that came from the variable. + // + bool vo; + if (pa != nullptr && !pa->match) { - // This is tricky: if the pattern does not require a match, then we - // should re-merge it with values that came from the variable. - // - bool vo; - if (pa != nullptr && !pa->match) - { - pt = t != nullptr ? t : var.type; - pv = v != nullptr ? v : &var.visibility; - po = o != nullptr ? o : &(vo = (var.overrides != nullptr)); - - merge_pattern (*pa, pt, pv, po); - } + pt = t != nullptr ? t : var.type; + pv = v != nullptr ? v : &var.visibility; + po = o != nullptr ? o : &(vo = true); - update (var, pt, pv, po); // Not changing the key. + merge_pattern (*pa, pt, pv, po); } - else if (var.overrides != nullptr) - fail << "variable " << var << " cannot be overridden"; + + if (po == nullptr) // NULL overridable falls back to false. + po = &(vo = false); + + update (var, pt, pv, po); // Not changing the key. } return var; @@ -1320,6 +1318,8 @@ namespace build2 nullptr /* override */, false /* pattern */)); + assert (a.overrides == nullptr); + if (a.aliases == &a) // Not aliased yet. { a.aliases = var.aliases; diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx index b293215..9b9880a 100644 --- a/libbuild2/variable.hxx +++ b/libbuild2/variable.hxx @@ -1065,9 +1065,12 @@ namespace build2 const variable* find (const string& name) const; - // Find existing or insert new (untyped, non-overridable, normal - // visibility; but may be overridden by a pattern). + // Find existing or insert new variable. // + // Unless specified explicitly, the variable is untyped, non-overridable, + // and with normal visibility but these may be overridden by a pattern. + // Note also that a pattern may restrict (but not relax) overridability. + const variable& insert (string name) { @@ -1083,6 +1086,9 @@ namespace build2 return insert (move (name), nullptr, &v, nullptr); } + // Note that overridability can still be restricted (but not relaxed) by + // another call to insert or via patterns (see below). + // const variable& insert (string name, bool overridable) { @@ -1099,14 +1105,15 @@ namespace build2 const variable& insert (string name) { - return insert (move (name), &value_traits::value_type); + return insert ( + move (name), &value_traits::value_type, nullptr, nullptr); } template const variable& insert (string name, variable_visibility v) { - return insert (move (name), &value_traits::value_type, &v); + return insert (move (name), &value_traits::value_type, &v, nullptr); } template @@ -1213,18 +1220,21 @@ namespace build2 friend class scope; private: + // Note that in insert() NULL overridable is interpreted as false unless + // overridden by a pattern while in update() NULL overridable is ignored. + // LIBBUILD2_SYMEXPORT variable& insert (string name, const value_type*, - const variable_visibility* = nullptr, - const bool* overridable = nullptr, + const variable_visibility*, + const bool* overridable, bool pattern = true); LIBBUILD2_SYMEXPORT void update (variable&, const value_type*, - const variable_visibility* = nullptr, - const bool* = nullptr) const; + const variable_visibility*, + const bool*) const; // Variable map. // -- cgit v1.1