aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2020-03-18 10:52:48 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2020-03-18 10:52:48 +0200
commit14b34a239fa23e1a28519ab87f450c0a440d4f85 (patch)
tree740db344520d93279cdc4a5f686ef339600e152f
parent7d0c4c5ab6760e4487230f9eda87c352609be553 (diff)
Cleanup variable type/visibility/overridability logic
-rw-r--r--libbuild2/parser.cxx94
-rw-r--r--libbuild2/variable.cxx34
-rw-r--r--libbuild2/variable.hxx26
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<variable_visibility> vis;
+ optional<bool> 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<variable&> (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<variable&> (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<lookup, size_t> 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<lookup, size_t> 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<T>::value_type);
+ return insert (
+ move (name), &value_traits<T>::value_type, nullptr, nullptr);
}
template <typename T>
const variable&
insert (string name, variable_visibility v)
{
- return insert (move (name), &value_traits<T>::value_type, &v);
+ return insert (move (name), &value_traits<T>::value_type, &v, nullptr);
}
template <typename T>
@@ -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.
//