From e20a351013745e8d6c3a0a99bd40c172ed0ae8be Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 14 Mar 2019 17:20:02 +0200 Subject: Add support for multiple variable overrides Now we can do: $ b config.cxx.coptions=-O3 config.cxx.coptions=-O0 Or even: $ b config.cxx.coptions=-O3 config.cxx.coptions+=-g --- build2/b.cxx | 29 +++---------- build2/cc/module.cxx | 4 +- build2/config/operation.cxx | 18 +++----- build2/config/utility.cxx | 2 +- build2/config/utility.txx | 2 +- build2/context.cxx | 60 ++++++++++---------------- build2/dump.cxx | 5 +-- build2/parser.cxx | 2 +- build2/scope.cxx | 28 ++++++------- build2/scope.hxx | 2 +- build2/target.hxx | 4 +- build2/test/script/script.cxx | 2 +- build2/variable.cxx | 20 ++++----- build2/variable.hxx | 84 +++++++++++++++++++++++++++++-------- old-tests/variable/override/test.sh | 6 +-- tests/variable/override/testscript | 30 +++++++++++++ 16 files changed, 168 insertions(+), 130 deletions(-) diff --git a/build2/b.cxx b/build2/b.cxx index 789bbc2..882b469 100644 --- a/build2/b.cxx +++ b/build2/b.cxx @@ -1296,11 +1296,10 @@ main (int argc, char* argv[]) // Enter project-wide (as opposed to global) variable overrides. // // The mildly tricky part here is to distinguish the situation where - // we are bootstrapping the same project multiple times (which is - // ok) vs overriding the same variable multiple times (which is not - // ok). The first override that we set cannot possibly end up in the - // second sitution so if it is already set, then it can only be the - // first case. + // we are bootstrapping the same project multiple times. The first + // override that we set cannot already exist (because the override + // variable names are unique) so if it is already set, then it can + // only mean this project is already bootstrapped. // // This is further complicated by the project vs amalgamation logic // (we may have already done the amalgamation but not the project). @@ -1309,7 +1308,6 @@ main (int argc, char* argv[]) { auto& sm (scope_map::instance); - bool first_a (true); for (const variable_override& o: var_ovs) { if (o.ovr.visibility != variable_visibility::normal) @@ -1325,20 +1323,12 @@ main (int argc, char* argv[]) auto p (s.vars.insert (o.ovr)); if (!p.second) - { - if (first_a) - break; - - fail << "multiple " << (o.dir ? "scope" : "amalgamation") - << " overrides of variable " << o.var.name; - } + break; value& v (p.first); v = o.val; - first_a = false; } - bool first_p (true); for (const variable_override& o: var_ovs) { // Ours is either project (%foo) or scope (/foo). @@ -1353,17 +1343,10 @@ main (int argc, char* argv[]) auto p (s.vars.insert (o.ovr)); if (!p.second) - { - if (first_p) - break; - - fail << "multiple " << (o.dir ? "scope" : "project") - << " overrides of variable " << o.var.name; - } + break; value& v (p.first); v = o.val; - first_p = false; } } diff --git a/build2/cc/module.cxx b/build2/cc/module.cxx index 78e2e0f..824eee7 100644 --- a/build2/cc/module.cxx +++ b/build2/cc/module.cxx @@ -191,7 +191,7 @@ namespace build2 rs.assign (x_pattern) = ci.pattern; - if (!x_stdlib.aliases (c_stdlib)) + if (!x_stdlib.alias (c_stdlib)) rs.assign (x_stdlib) = ci.x_stdlib; new_ = p.second; @@ -390,7 +390,7 @@ namespace build2 dr << "\n runtime " << ci.runtime << "\n stdlib " << ci.x_stdlib; - if (!x_stdlib.aliases (c_stdlib)) + if (!x_stdlib.alias (c_stdlib)) dr << "\n c stdlib " << ci.c_stdlib; } diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx index 724b92d..7d6fc74 100644 --- a/build2/config/operation.cxx +++ b/build2/config/operation.cxx @@ -131,7 +131,7 @@ namespace build2 const variable& var (sv.var); pair org (root.find_original (var)); - pair ovr (var.override == nullptr + pair ovr (var.overrides == nullptr ? org : root.find_override (var, org)); const lookup& l (ovr.first); @@ -859,19 +859,11 @@ namespace build2 { const variable& var (p.first->first); - // Annoyingly, this is one of the __override/__prefix/__suffix - // values. So we strip the last component. + // Annoyingly, this can be (always is?) one of the overrides + // (__override, __prefix, etc). // - size_t n (var.name.size ()); - - if (var.name.compare (n - 11, 11, ".__override") == 0) - n -= 11; - else if (var.name.compare (n - 9, 9, ".__prefix") == 0) - n -= 9; - else if (var.name.compare (n - 9, 9, ".__suffix") == 0) - n -= 9; - - m.save_variable (*vp.find (string (var.name, 0, n))); + size_t n (var.override ()); + m.save_variable (n != 0 ? *vp.find (string (var.name, 0, n)) : var); } // Now project-specific. For now we just save all of them and let diff --git a/build2/config/utility.cxx b/build2/config/utility.cxx index cc89d2f..e15c689 100644 --- a/build2/config/utility.cxx +++ b/build2/config/utility.cxx @@ -33,7 +33,7 @@ namespace build2 if (l.defined () && l->extra) n = true; - if (var.override != nullptr) + if (var.overrides != nullptr) { pair ovr (r.find_override (var, move (org))); diff --git a/build2/config/utility.txx b/build2/config/utility.txx index 4f3429f..b8e0acc 100644 --- a/build2/config/utility.txx +++ b/build2/config/utility.txx @@ -47,7 +47,7 @@ namespace build2 else if (l->extra) n = (save_flags & save_commented) == 0; // Absence means default. - if (var.override != nullptr) + if (var.overrides != nullptr) { pair ovr (root.find_override (var, move (org))); diff --git a/build2/context.cxx b/build2/context.cxx index 633f3ff..e1aaeee 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -557,8 +557,10 @@ namespace build2 // marked as such first. Then, as we enter variables, we can verify that // the override is alowed. // - for (const string& s: cmd_vars) + for (size_t i (0); i != cmd_vars.size (); ++i) { + const string& s (cmd_vars[i]); + istringstream is (s); is.exceptions (istringstream::failbit | istringstream::badbit); @@ -678,48 +680,39 @@ namespace build2 if (c == '!' && dir) fail << "scope-qualified global override of variable " << n; - variable_visibility v (c == '/' ? variable_visibility::scope : - c == '%' ? variable_visibility::project : - variable_visibility::normal); - variable& var (const_cast ( vp.insert (n, true /* overridable */))); - const char* k (tt == token_type::assign ? ".__override" : - tt == token_type::append ? ".__suffix" : ".__prefix"); - // We might already have a variable for this kind of override. - // - const variable* o (var.override.get ()); - for (; o != nullptr; o = o->override.get ()) + const variable* o; { - if (o->visibility == v && o->name.rfind (k) != string::npos) - break; - } + variable_visibility v (c == '/' ? variable_visibility::scope : + c == '%' ? variable_visibility::project : + variable_visibility::normal); + + const char* k (tt == token_type::assign ? "__override" : + tt == token_type::append ? "__suffix" : "__prefix"); - // Add it if not found. - // - if (o == nullptr) - { unique_ptr p ( new variable { - n + k, - nullptr /* alias */, - nullptr /* type */, - nullptr /* override */, + n + '.' + to_string (i + 1) + '.' + k, + nullptr /* aliases */, + nullptr /* type */, + nullptr /* overrides */, v}); // Back link. // - p->alias = p.get (); - if (var.override != nullptr) - swap (p->alias, const_cast (var.override.get ())->alias); + p->aliases = p.get (); + if (var.overrides != nullptr) + swap (p->aliases, + const_cast (var.overrides.get ())->aliases); // Forward link. // - p->override = move (var.override); - var.override = move (p); + p->overrides = move (var.overrides); + var.overrides = move (p); - o = var.override.get (); + o = var.overrides.get (); } // Currently we expand project overrides in the global scope to keep @@ -745,16 +738,9 @@ namespace build2 if (c == '!' || (dir && dir->absolute ())) { scope& s (c == '!' ? gs : sm.insert (*dir)->second); - auto p (s.vars.insert (*o)); - if (!p.second) - { - if (c == '!') - fail << "multiple global overrides of variable " << n; - else - fail << "multiple overrides of variable " << n - << " in scope " << *dir; - } + auto p (s.vars.insert (*o)); + assert (p.second); // Variable name is unique. value& v (p.first); v = move (r.first); diff --git a/build2/dump.cxx b/build2/dump.cxx index 8f58488..d9a1855 100644 --- a/build2/dump.cxx +++ b/build2/dump.cxx @@ -96,10 +96,7 @@ namespace build2 // if (k != variable_kind::prerequisite) { - if (var.override != nullptr && - var.name.rfind (".__override") == string::npos && - var.name.rfind (".__suffix") == string::npos && - var.name.rfind (".__prefix") == string::npos) + if (var.overrides != nullptr && !var.override ()) { lookup org (v, var, vm); diff --git a/build2/parser.cxx b/build2/parser.cxx index 3acff37..997a1ac 100644 --- a/build2/parser.cxx +++ b/build2/parser.cxx @@ -5108,7 +5108,7 @@ namespace build2 if (!r.first.defined ()) r = t->find_original (var); - return var.override == nullptr + return var.overrides == nullptr ? r.first : t->base_scope ().find_override (var, move (r), true).first; } diff --git a/build2/scope.cxx b/build2/scope.cxx index 4dd475d..ec2776e 100644 --- a/build2/scope.cxx +++ b/build2/scope.cxx @@ -191,7 +191,7 @@ namespace build2 // no overrides apply, then we return the original value and not its copy // in the cache (this is used to detect if the value was overriden). // - assert (var.override != nullptr); + assert (var.overrides != nullptr); const lookup& orig (original.first); size_t orig_depth (original.second); @@ -269,13 +269,13 @@ namespace build2 return true; }; - // Return the override value if present in scope s and (optionally) ends - // with the specified suffix. + // Return the override value if present in scope s and (optionally) of + // the specified kind (__override, __prefix, etc). // auto find = [&s, &var] (const variable* o, - const char* sf = nullptr) -> lookup + const char* k = nullptr) -> lookup { - if (sf != nullptr && o->name.rfind (sf) == string::npos) + if (k != nullptr && !o->override (k)) return lookup (); // Note: using the original as storage variable. @@ -317,9 +317,9 @@ namespace build2 inner_proj = s->root_scope (); } - for (const variable* o (var.override.get ()); + for (const variable* o (var.overrides.get ()); o != nullptr; - o = o->override.get ()) + o = o->overrides.get ()) { if (inner_vars != nullptr && !applies (o, inner_vars, inner_proj)) continue; @@ -400,9 +400,9 @@ namespace build2 // Note that the override list is in the reverse order of appearance and // so we will naturally see the most recent override first. // - for (const variable* o (var.override.get ()); + for (const variable* o (var.overrides.get ()); o != nullptr; - o = o->override.get ()) + o = o->overrides.get ()) { // If we haven't yet found anything, then any override will still be // "visible" even if it doesn't apply. @@ -410,7 +410,7 @@ namespace build2 if (stem.defined () && !applies (o, stem.vars, stem_proj)) continue; - auto l (find (o, ".__override")); + auto l (find (o, "__override")); if (l.defined ()) { @@ -500,9 +500,9 @@ namespace build2 // bool skip (stem_ovr != nullptr && stem_depth == ovr_depth); - for (const variable* o (var.override->alias); // Last override. + for (const variable* o (var.overrides->aliases); // Last override. o != nullptr; - o = (o->alias != var.override->alias ? o->alias : nullptr)) + o = (o->aliases != var.overrides->aliases ? o->aliases : nullptr)) { if (skip) { @@ -524,8 +524,8 @@ namespace build2 // variable itself is typed. We also pass the original variable for // diagnostics. // - auto lp (find (o, ".__prefix")); - auto ls (find (o, ".__suffix")); + auto lp (find (o, "__prefix")); + auto ls (find (o, "__suffix")); if (cl) { diff --git a/build2/scope.hxx b/build2/scope.hxx index 6e48de3..cfb3d24 100644 --- a/build2/scope.hxx +++ b/build2/scope.hxx @@ -125,7 +125,7 @@ namespace build2 const string* tn = nullptr) const { auto p (find_original (var, tt, tn)); - return var.override == nullptr ? p : find_override (var, move (p)); + return var.overrides == nullptr ? p : find_override (var, move (p)); } // Implementation details (used by scope target lookup). The start_depth diff --git a/build2/target.hxx b/build2/target.hxx index c86a10b..fafb689 100644 --- a/build2/target.hxx +++ b/build2/target.hxx @@ -399,7 +399,7 @@ namespace build2 find (const variable& var) const { auto p (find_original (var)); - return var.override == nullptr + return var.overrides == nullptr ? p : base_scope ().find_override (var, move (p), true); } @@ -534,7 +534,7 @@ namespace build2 find (const variable& var) const { auto p (find_original (var)); - return var.override == nullptr + return var.overrides == nullptr ? p : target_->base_scope ().find_override (var, move (p), true, true); } diff --git a/build2/test/script/script.cxx b/build2/test/script/script.cxx index 94d6d8b..865c798 100644 --- a/build2/test/script/script.cxx +++ b/build2/test/script/script.cxx @@ -653,7 +653,7 @@ namespace build2 if (p.first) { - if (var.override != nullptr) + if (var.overrides != nullptr) p = s.target_scope.find_override (var, move (p), true); return p.first; diff --git a/build2/variable.cxx b/build2/variable.cxx index 264928f..173efe5 100644 --- a/build2/variable.cxx +++ b/build2/variable.cxx @@ -1142,7 +1142,7 @@ namespace build2 // Check overridability (all overrides, if any, should already have // been entered (see context.cxx:reset()). // - if (var.override != nullptr && (o == nullptr || !*o)) + if (var.overrides != nullptr && (o == nullptr || !*o)) fail << "variable " << var.name << " cannot be overridden"; bool ut (t != nullptr && var.type != t); @@ -1150,7 +1150,7 @@ namespace build2 // Variable should not be updated post-aliasing. // - assert (var.alias == &var || (!ut && !uv)); + assert (var.aliases == &var || (!ut && !uv)); // Update type? // @@ -1277,12 +1277,12 @@ namespace build2 variable& r (p.first->second); if (p.second) - r.alias = &r; + r.aliases = &r; else // Note: overridden variable will always exist. { if (t != nullptr || v != nullptr || o != nullptr) update (r, t, v, o); // Not changing the key. - else if (r.override != nullptr) + else if (r.overrides != nullptr) fail << "variable " << r.name << " cannot be overridden"; } @@ -1292,7 +1292,7 @@ namespace build2 const variable& variable_pool:: insert_alias (const variable& var, string n) { - assert (var.alias != nullptr && var.override == nullptr); + assert (var.aliases != nullptr && var.overrides == nullptr); variable& a (insert (move (n), var.type, @@ -1300,13 +1300,13 @@ namespace build2 nullptr /* override */, false /* pattern */)); - if (a.alias == &a) // Not aliased yet. + if (a.aliases == &a) // Not aliased yet. { - a.alias = var.alias; - const_cast (var).alias = &a; + a.aliases = var.aliases; + const_cast (var).aliases = &a; } else - assert (a.aliases (var)); // Make sure it is already an alias of var. + assert (a.alias (var)); // Make sure it is already an alias of var. return a; } @@ -1406,7 +1406,7 @@ namespace build2 break; } - v = v->alias; + v = v->aliases; } while (v != &var && v != nullptr); diff --git a/build2/variable.hxx b/build2/variable.hxx index 2bdfe30..5e6ad2f 100644 --- a/build2/variable.hxx +++ b/build2/variable.hxx @@ -145,25 +145,39 @@ namespace build2 // The two variables are considered the same if they have the same name. // // Variables can be aliases of each other in which case they form a circular - // linked list (alias for variable without any aliases points to the - // variable itself). + // linked list (the aliases pointer for variable without any aliases points + // to the variable itself). // // If the variable is overridden on the command line, then override is the // linked list of the special override variables. Their names are derived - // from the main variable name as .{__override,__prefix,__suffix} and - // they are not entered into the var_pool. The override variables only vary - // in their names and visibility. Their alias pointer is re-purposed to make - // the list doubly-linked with the first override's alias pointing to the - // last element (or itself). - // - // Note that the override list is in the reverse order of the overrides - // appearing on the command line, which is important when deciding whether - // and in what order they apply (see find_override() for details). + // from the main variable name as ..{__override,__prefix,__suffix} + // and they are not entered into the var_pool. The override variables only + // vary in their names and visibility. Their aliases pointer is re-purposed + // to make the list doubly-linked with the first override's aliases pointer + // pointing to the last element (or itself). // // Note also that we don't propagate the variable type to override variables // and we keep override values as untyped names. They get "typed" when they // are applied. // + // The overrides list is in the reverse order of the overrides appearing on + // the command line, which is important when deciding whether and in what + // order they apply (see find_override() for details). + // + // The part in the override variable name is its position on the command + // line, which effectively means we will have as many variable names as + // there are overrides. This strange arrangement is here to support multiple + // overrides. For example: + // + // b config.cc.coptions=-O2 config.cc.coptions+=-g config.cc.coptions+=-Wall + // + // We cannot yet apply them to form a single value since this requires + // knowing their type. And there is no way to store multiple values of the + // same variable in any given variable_map. As a result, the best option + // appears to be to store them as multiple variables. While not very + // efficient, this shouldn't be a big deal since we don't expect to have + // many overrides. + // // We use the "modify original, override on query" model. Because of that, a // modified value does not necessarily represent the actual value so care // must be taken to re-query after (direct) modification. And because of @@ -179,20 +193,56 @@ namespace build2 struct variable { string name; - const variable* alias; // Circular linked list. - const value_type* type; // If NULL, then not (yet) typed. - unique_ptr override; + const variable* aliases; // Circular linked list. + const value_type* type; // If NULL, then not (yet) typed. + unique_ptr overrides; variable_visibility visibility; // Return true if this variable is an alias of the specified variable. // bool - aliases (const variable& var) const + alias (const variable& var) const { - const variable* v (alias); - for (; v != &var && v != this; v = v->alias) ; + const variable* v (aliases); + for (; v != &var && v != this; v = v->aliases) ; return v == &var; } + + // Return the length of the original variable if this is an override, + // optionally of the specified kind (__override, __prefix, etc), and 0 + // otherwise (so this function can be used as a predicate). + // + // @@ It would be nicer to return the original variable but there is no + // natural place to store such a "back" pointer. The overrides pointer + // in the last element could work but it is owning. So let's not + // complicate things for now seeing that there are only a few places + // where we need this. + // + size_t + override (const char* k = nullptr) const + { + size_t p (name.rfind ('.')); + if (p != string::npos) + { + auto cmp = [this, p] (const char* k) + { + return name.compare (p + 1, string::npos, k) == 0; + }; + + if (k != nullptr + ? (cmp (k)) + : (cmp ("__override") || cmp ("__prefix") || cmp ("__suffix"))) + { + // Skip .. + // + p = name.rfind ('.', p - 1); + assert (p != string::npos && p != 0); + return p; + } + } + + return 0; + } }; inline bool diff --git a/old-tests/variable/override/test.sh b/old-tests/variable/override/test.sh index baef1ca..94ed61f 100755 --- a/old-tests/variable/override/test.sh +++ b/old-tests/variable/override/test.sh @@ -55,9 +55,9 @@ function test () fail foo=bar[] # error: unexpected [ in variable assignment 'foo=bar[]' fail foo=[string]bar # error: typed override of variable foo -fail "!foo=bar" "!foo=BAR" # error: multiple global overrides of variable foo -fail "foo=bar" "foo=BAR" # error: multiple project overrides of variable foo -fail "%foo=bar" "%foo=BAR" # error: multiple project overrides of variable foo +#fail "!foo=bar" "!foo=BAR" # error: multiple global overrides of variable foo +#fail "foo=bar" "foo=BAR" # error: multiple project overrides of variable foo +#fail "%foo=bar" "%foo=BAR" # error: multiple project overrides of variable foo test --buildfile simple foo=bar ./ ./ <<< "bar" # Multiple bootstraps of the same project. diff --git a/tests/variable/override/testscript b/tests/variable/override/testscript index 28225c1..f8a930b 100644 --- a/tests/variable/override/testscript +++ b/tests/variable/override/testscript @@ -105,3 +105,33 @@ 1 2 EOO } + +: multiple +: +{ + : assign + : + $* x=0 !y=0 x=1 !y=1 <>EOO + print $x + print $y + EOI + 1 + 1 + EOO + + : append + : + $* x=0 x+=1 x+=2 <>EOO + print $x + EOI + 0 1 2 + EOO + + : prepend + : + $* x=2 x=+1 x=+0 <>EOO + print $x + EOI + 0 1 2 + EOO +} -- cgit v1.1