From a60da308bfcc003fd07d2b7d848ccb8d166e472a Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 11 Apr 2016 14:38:00 +0200 Subject: Redo config inheritance logic --- build2/config/operation.cxx | 81 +++++++++++++++++++++++++++++++++++++++++---- build2/config/utility | 9 ++--- build2/config/utility.txx | 21 ++++++++---- build2/diagnostics | 5 +-- build2/diagnostics.cxx | 10 +++++- build2/scope | 2 +- build2/scope.cxx | 2 +- build2/variable | 11 ++++-- build2/variable.cxx | 4 +-- 9 files changed, 118 insertions(+), 27 deletions(-) diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx index d1a01ea..fc583b5 100644 --- a/build2/config/operation.cxx +++ b/build2/config/operation.cxx @@ -102,16 +102,83 @@ namespace build2 { const variable& var (p.first); - lookup l (root[var]); - - // We only write values that are set on our root scope or are global - // overrides. Anything in-between is inherited. We might also not - // have any value at all (see unconfigured()). + pair org (root.find_original (var)); + pair ovr (var.override == nullptr + ? org + : root.find_override (var, org)); + const lookup& l (ovr.first); + + // We definitely write values that are set on our root scope or are + // global overrides. Anything in-between is presumably inherited. + // We might also not have any value at all (see unconfigured()). // - if (!l.defined () || - !(l.belongs (root) || l.belongs (*global_scope))) + if (!l.defined ()) continue; + if (!(l.belongs (root) || l.belongs (*global_scope))) + { + // This is presumably an inherited value. But it could also be + // some left-over garbage. For example, our amalgamation could + // have used a module but then dropped it while its configuration + // values are still lingering in config.build. They are probably + // still valid and we should probably continue using them but we + // definitely want to move them to our config.build since they + // will be dropped from the amalgamation's config.build. Let's + // also warn the user just in case. + // + bool found (false); + scope* r (&root); + while ((r = r->parent_scope ()->root_scope ()) != nullptr) + { + if (l.belongs (*r)) + { + if (auto* m = r->modules.lookup (module::name)) + found = m->vars.find (var) != m->vars.end (); + + break; + } + } + + if (found) // Inherited. + continue; + + location loc (&f); + + // If this value is not defined in a project's root scope, then + // something is broken. + // + if (r == nullptr) + fail (loc) << "inherited variable " << var.name << " value " + << "is not from a root scope"; + + // If none of the outer project's configurations use this value, + // then we warn and save as our own. One special case where we + // don't want to warn the user is if the variable is overriden. + // + if (org.first == ovr.first) + { + diag_record dr; + dr << warn (loc) << "saving previously inherited variable " + << var.name; + + dr << info (loc) << "because project " << r->out_path () + << " no longer uses it in its configuration"; + + if (verb >= 2) + { + dr << info (loc) << "variable value: "; + + if (*l) + { + names storage; + dr << "'" << reverse (*l, storage) << "'"; + } + else + dr << "[null]"; + } + } + } + const value& val (*l); // We will only write config.*.configured if it is false (true is diff --git a/build2/config/utility b/build2/config/utility index 451df30..8a93c09 100644 --- a/build2/config/utility +++ b/build2/config/utility @@ -21,12 +21,13 @@ namespace build2 // // 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 amalgamtion), - // then its value is "overridden" for this root scope. + // then its value is "overridden" to the default value on this root scope. // // Return the reference to the value as well as the indication of whether - // the value is "new", that is, it was either set (including override) or - // it came from the command line and was not inherited. This is usually - // used to test the new value. + // the value is "new", that is, it was set to the default value (inherited + // or not, including overrides). We also treat command line overrides + // (inherited or not) as new. This flag is usually used to test that the + // new value is valid, etc. // template pair, bool> diff --git a/build2/config/utility.txx b/build2/config/utility.txx index f49be75..30ed02f 100644 --- a/build2/config/utility.txx +++ b/build2/config/utility.txx @@ -17,8 +17,9 @@ namespace build2 save_variable (root, var); pair org (root.find_original (var)); + + bool n (false); // New flag. lookup l (org.first); - bool n (false); // The interaction with command line overrides can get tricky. For // example, the override to defaul value could make (non-recursive) @@ -28,10 +29,17 @@ namespace build2 // if (!l.defined () || (def_ovr && !l.belongs (root))) { - l = lookup ((root.assign (var) = def_val), root); - org = make_pair (l, 1); // Lookup depth is 1 since in root.vars. + value& v (root.assign (var) = def_val); + v.extra = true; // Default value flag. + n = true; + l = lookup (v, root); + org = make_pair (l, 1); // Lookup depth is 1 since it's in root.vars. } + // Treat an inherited value that was set to default as new. + // + else if (l->extra) + n = true; if (var.override != nullptr) { @@ -39,11 +47,10 @@ namespace build2 if (l != ovr.first) // Overriden? { - l = move (ovr.first); - - // Overriden and not inherited (same logic as in save_config()). + // Override is always treated as new. // - n = l.belongs (root) || l.belongs (*global_scope); + n = true; + l = move (ovr.first); } } diff --git a/build2/diagnostics b/build2/diagnostics index ffb24ff..c0ba6ec 100644 --- a/build2/diagnostics +++ b/build2/diagnostics @@ -280,10 +280,11 @@ namespace build2 class location { public: - // Note that location maintains a shallow reference to path. + // Note that location maintains a shallow reference to path. Zero lines + // or columns are not printed. // location (): file (nullptr), line (0), column (0) {} - location (const path* f, uint64_t l, uint64_t c) + location (const path* f, uint64_t l = 0, uint64_t c = 0) : file (f), line (l), column (c) {} const path* file; diff --git a/build2/diagnostics.cxx b/build2/diagnostics.cxx index d6278a8..7b60d05 100644 --- a/build2/diagnostics.cxx +++ b/build2/diagnostics.cxx @@ -106,7 +106,15 @@ namespace build2 { stream_verb (r.os_, sverb_); - r << *loc_.file << ':' << loc_.line << ':' << loc_.column << ": "; + r << *loc_.file << ':'; + + if (loc_.line != 0) + r << loc_.line << ':'; + + if (loc_.column != 0) + r << loc_.column << ':'; + + r << ' '; if (type_ != nullptr) r << type_ << ": "; diff --git a/build2/scope b/build2/scope index 09914a9..c1dcb91 100644 --- a/build2/scope +++ b/build2/scope @@ -146,7 +146,7 @@ namespace build2 pair find_override (const variable&, - pair&& original, + pair original, bool target = false) const; // Return a value suitable for assignment (or append if you only diff --git a/build2/scope.cxx b/build2/scope.cxx index 19f3b56..f0d28e0 100644 --- a/build2/scope.cxx +++ b/build2/scope.cxx @@ -67,7 +67,7 @@ namespace build2 pair scope:: find_override (const variable& var, - pair&& original, + pair original, bool target) const { // Normally there would be no overrides and if there are, there will only diff --git a/build2/variable b/build2/variable index ebf0f02..ea60832 100644 --- a/build2/variable +++ b/build2/variable @@ -119,7 +119,7 @@ namespace build2 // value // - enum class value_state {null, empty, filled}; + enum class value_state: uint8_t {null, empty, filled}; class value { @@ -127,6 +127,13 @@ namespace build2 const value_type* type; // NULL means this value is not (yet) typed. value_state state; + // Extra data that is associated with the value that can be used to store + // flags, etc. It is initialized to 0 and copied (but not assigned) from + // one value to another but is otherwise untouched (not even when the + // value is reset to NULL). + // + uint16_t extra; + bool null () const {return state == value_state::null;} bool empty () const {return state == value_state::empty;} @@ -145,7 +152,7 @@ namespace build2 explicit value (const value_type* t = nullptr) - : type (t), state (value_state::null) {} + : type (t), state (value_state::null), extra (0) {} // Note: preserves type. // diff --git a/build2/variable.cxx b/build2/variable.cxx index c4e2721..d6a42b3 100644 --- a/build2/variable.cxx +++ b/build2/variable.cxx @@ -27,7 +27,7 @@ namespace build2 value:: value (value&& v) - : type (v.type), state (v.state) + : type (v.type), state (v.state), extra (v.extra) { if (!null ()) { @@ -42,7 +42,7 @@ namespace build2 value:: value (const value& v) - : type (v.type), state (v.state) + : type (v.type), state (v.state), extra (v.extra) { if (!null ()) { -- cgit v1.1