From 7253ffee27f6cae34e63a72b2d3d10db10571ecc Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 31 Mar 2016 10:59:45 +0200 Subject: Clean up variable lookup interfaces --- build2/cxx/compile.cxx | 2 +- build2/cxx/link.cxx | 4 ++-- build2/dist/operation.cxx | 8 +++---- build2/file.cxx | 13 +++++----- build2/scope | 45 ++++++++++++++++++----------------- build2/scope.cxx | 27 ++++++++++++++------- build2/target | 17 +++++++------- build2/target.cxx | 49 ++++++++------------------------------ build2/target.txx | 2 +- build2/test/rule.cxx | 13 +++++----- build2/utility | 8 +++---- build2/utility.cxx | 6 ++--- build2/variable | 60 ++++++++++++++++++++--------------------------- build2/variable.cxx | 16 ++++++------- build2/variable.ixx | 18 ++------------ 15 files changed, 121 insertions(+), 167 deletions(-) diff --git a/build2/cxx/compile.cxx b/build2/cxx/compile.cxx index c4e9cee..914cd10 100644 --- a/build2/cxx/compile.cxx +++ b/build2/cxx/compile.cxx @@ -236,7 +236,7 @@ namespace build2 auto test = [&s, &n, &e, &var] (const target_type& tt) -> const target_type* { - if (auto l = s.lookup (tt, n, var)) + if (auto l = s.find (var, tt, n)) if (cast (l) == e) return &tt; diff --git a/build2/cxx/link.cxx b/build2/cxx/link.cxx index aa60c7e..e4154ab 100644 --- a/build2/cxx/link.cxx +++ b/build2/cxx/link.cxx @@ -780,7 +780,7 @@ namespace build2 if (dd.expect ("cxx.link 1") != nullptr) l4 ([&]{trace << "rule mismatch forcing update of " << t;}); - lookup ranlib; + lookup ranlib; // Then the linker checksum (ar/ranlib or C++ compiler). // @@ -789,7 +789,7 @@ namespace build2 ranlib = rs["config.bin.ranlib"]; if (ranlib->empty ()) // @@ TMP until proper NULL support. - ranlib = lookup (); + ranlib = lookup (); const char* rl ( ranlib diff --git a/build2/dist/operation.cxx b/build2/dist/operation.cxx index 86917ed..8a890fa 100644 --- a/build2/dist/operation.cxx +++ b/build2/dist/operation.cxx @@ -72,7 +72,7 @@ namespace build2 // Make sure we have the necessary configuration before // we get down to business. // - auto l (rs->vars["dist.root"]); + auto l (rs->vars["dist.root"]); //@@ OVR if (!l || l->empty ()) fail << "unknown root distribution directory" << @@ -84,14 +84,14 @@ namespace build2 fail << "root distribution directory " << dist_root << " does not exist"; - l = rs->vars["dist.package"]; + l = rs->vars["dist.package"]; //@@ OVR if (!l || l->empty ()) fail << "unknown distribution package name" << info << "did you forget to set dist.package?"; const string& dist_package (cast (l)); - const path& dist_cmd (cast (rs->vars["dist.cmd"])); + const path& dist_cmd (cast (rs->vars["dist.cmd"])); // @@ OVR // Get the list of operations supported by this project. Skip // default_id. @@ -281,7 +281,7 @@ namespace build2 // Archive if requested. // - if (auto l = rs->vars["dist.archives"]) + if (auto l = rs->vars["dist.archives"]) // @@ OVR { for (const string& e: cast (l)) archive (dist_root, dist_package, e); diff --git a/build2/file.cxx b/build2/file.cxx index 1f4e517..10461c4 100644 --- a/build2/file.cxx +++ b/build2/file.cxx @@ -263,9 +263,9 @@ namespace build2 source_once (bf, root, root); } - // Extract the specified variable value from a buildfile. It is - // expected to be the first non-comment line and not to rely on - // any variable expansion other than those from the global scope. + // Extract the specified variable value from a buildfile. It is expected to + // be the first non-comment line and not to rely on any variable expansion + // other than those from the global scope or any variable overrides. // static value extract_variable (const path& bf, const char* name) @@ -297,10 +297,9 @@ namespace build2 temp_scope tmp (*global_scope); p.parse_variable (lex, tmp, var, tt); - auto l (tmp.vars[var]); - assert (l.defined ()); - value& v (*l); - return move (v); // Steal the value, the scope is going away. + value* v (tmp.vars.find (var)); + assert (v != nullptr); + return move (*v); // Steal the value, the scope is going away. } catch (const ifstream::failure&) { diff --git a/build2/scope b/build2/scope index 7ec8c3b..6050bf8 100644 --- a/build2/scope +++ b/build2/scope @@ -85,51 +85,52 @@ namespace build2 public: variable_map vars; - // Lookup, including in outer scopes. If you only want to lookup - // in this scope, do it on the the variables map directly. + // Lookup, including in outer scopes. If you only want to lookup in this + // scope, do it on the the variables map directly (and note that there + // will be no overrides). // - build2::lookup + lookup operator[] (const variable& var) const { - return lookup (nullptr, nullptr, var); + return find (var, nullptr, nullptr); } - build2::lookup + lookup operator[] (const string& name) const { return operator[] (var_pool.find (name)); } - // As above, but includes target type/pattern-specific variables. + // As above, but include target type/pattern-specific variables. // - build2::lookup - lookup (const target_key& tk, const variable& var) const + lookup + find (const variable& var, const target_key& tk) const { - return lookup (tk.type, tk.name, var); + return find (var, tk.type, tk.name); } - build2::lookup - lookup (const target_key& tk, const string& var) const + lookup + find (const string& var, const target_key& tk) const { - return lookup (tk, var_pool.find (var)); + return find (var_pool.find (var), tk); } - build2::lookup - lookup (const target_type& tt, - const string& name, - const variable& var) const + lookup + find (const variable& var, const target_type& tt, const string& tn) const { - return lookup (&tt, &name, var); + return find (var, &tt, &tn); } - build2::lookup - lookup (const target_type& tt, const string& name, const string& var) const + lookup + find (const string& var, const target_type& tt, const string& tn) const { - return lookup (tt, name, var_pool.find (var)); + return find (var_pool.find (var), tt, tn); } - build2::lookup - lookup (const target_type*, const string* name, const variable&) const; + lookup + find (const variable&, + const target_type* tt, const string* tn, + const target_type* gt = nullptr, const string* gn = nullptr) const; // Return a value suitable for assignment (or append if you only // want to append to the value from this scope). If the variable diff --git a/build2/scope.cxx b/build2/scope.cxx index 294833e..77efccd 100644 --- a/build2/scope.cxx +++ b/build2/scope.cxx @@ -12,21 +12,30 @@ namespace build2 { // scope // - lookup scope:: - lookup (const target_type* tt, const string* name, const variable& var) const + lookup scope:: + find (const variable& var, + const target_type* tt, const string* tn, + const target_type* gt, const string* gn) const { - using result = build2::lookup; - for (const scope* s (this); s != nullptr; ) { - if (tt != nullptr && !s->target_vars.empty ()) + if (!s->target_vars.empty ()) { - if (auto l = s->target_vars.lookup (*tt, *name, var)) - return l; + if (tt != nullptr) + { + if (auto l = s->target_vars.find (*tt, *tn, var)) + return l; + } + + if (gt != nullptr) + { + if (auto l = s->target_vars.find (*gt, *gn, var)) + return l; + } } if (auto r = s->vars.find (var)) - return result (r, &s->vars); + return lookup (r, &s->vars); switch (var.visibility) { @@ -42,7 +51,7 @@ namespace build2 } } - return result (); + return lookup (); } value& scope:: diff --git a/build2/target b/build2/target index bf5ff04..af573fa 100644 --- a/build2/target +++ b/build2/target @@ -142,10 +142,9 @@ namespace build2 virtual void reset (action_type); - const dir_path dir; // Absolute and normalized. + const dir_path dir; // Absolute and normalized. const string name; - const string* ext; // Extension, NULL means unspecified, - // empty means no extension. + const string* ext; // Extension, NULL - unspecified, empty - no extension. // Target group to which this target belongs, if any. Note that // we assume that the group and all its members are in the same @@ -271,15 +270,15 @@ namespace build2 public: variable_map vars; - // Lookup, including in groups to which this target belongs and - // then in outer scopes (including target type/pattern-specific - // variables). If you only want to lookup in this target, do it - // on the variable map directly. + // Lookup, including in groups to which this target belongs and then in + // outer scopes (including target type/pattern-specific variables). If you + // only want to lookup in this target, do it on the variable map directly + // (and note that there will be no overrides). // - lookup + lookup operator[] (const variable&) const; - lookup + lookup operator[] (const string& name) const { return operator[] (var_pool.find (name)); diff --git a/build2/target.cxx b/build2/target.cxx index 9d2f7af..e945d6d 100644 --- a/build2/target.cxx +++ b/build2/target.cxx @@ -114,55 +114,26 @@ namespace build2 return *r; } - lookup target:: + lookup target:: operator[] (const variable& var) const { - using result = lookup; - if (auto p = vars.find (var)) - return result (p, &vars); + return lookup (p, &vars); if (group != nullptr) { if (auto p = group->vars.find (var)) - return result (p, &group->vars); + return lookup (p, &group->vars); } - // We cannot simply delegate to scope's lookup() since we also need - // to check the group. + // Delegate to scope's find(). // - for (const scope* s (&base_scope ()); s != nullptr; ) - { - if (!s->target_vars.empty ()) - { - if (auto l = s->target_vars.lookup (type (), name, var)) - return l; - - if (group != nullptr) - { - if (auto l = s->target_vars.lookup (group->type (), group->name, var)) - return l; - } - } - - if (auto r = s->vars.find (var)) - return result (r, &s->vars); - - switch (var.visibility) - { - case variable_visibility::scope: - s = nullptr; - break; - case variable_visibility::project: - s = s->root () ? nullptr : s->parent_scope (); - break; - case variable_visibility::normal: - s = s->parent_scope (); - break; - } - } - - return result (); + return base_scope ().find ( + var, + &type (), + &name, + group != nullptr ? &group->type () : nullptr, + group != nullptr ? &group->name : nullptr); } value& target:: diff --git a/build2/target.txx b/build2/target.txx index b7d41f2..fe21016 100644 --- a/build2/target.txx +++ b/build2/target.txx @@ -22,7 +22,7 @@ namespace build2 { // Include target type/pattern-specific variables. // - if (auto l = s.lookup (tk, var)) + if (auto l = s.find (var, tk)) { // Help the user here and strip leading '.' from the extension. // diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx index 063d148..2815cf9 100644 --- a/build2/test/rule.cxx +++ b/build2/test/rule.cxx @@ -26,7 +26,7 @@ namespace build2 // this is a test. So take care of that as well. // bool r (false); - lookup l; + lookup l; // @@ This logic doesn't take into account target type/pattern- // specific variables. @@ -44,7 +44,7 @@ namespace build2 // if (var.name == "test") { - l = lookup (val, t); + l = lookup (val, t); break; } @@ -126,6 +126,7 @@ namespace build2 // First check the target-specific vars since they override any // scope ones. // + //@@ OVR auto il (t.vars["test.input"]); auto ol (t.vars["test.output"]); auto rl (t.vars["test.roundtrip"]); @@ -162,12 +163,12 @@ namespace build2 // for (scope* s (&bs); s != nullptr; s = s->parent_scope ()) { - ol = s->vars[on]; + ol = s->vars[on]; //@@ OVR if (!al) // Not overriden at target level by test.arguments? { - il = s->vars[in]; - rl = s->vars[rn]; + il = s->vars[in]; //@@ OVR + rl = s->vars[rn]; //@@ OVR } if (il || ol || rl) @@ -289,7 +290,7 @@ namespace build2 string var ("test."); var += n; - auto l (t.vars[var]); + auto l (t.vars[var]); //@@ OVR if (!l) { diff --git a/build2/utility b/build2/utility index e906b55..e433b5a 100644 --- a/build2/utility +++ b/build2/utility @@ -149,13 +149,13 @@ namespace build2 // As above but from the strings value directly. // class value; - template struct lookup; + struct lookup; void - append_options (cstrings&, const lookup&); + append_options (cstrings&, const lookup&); void - hash_options (sha256&, const lookup&); + hash_options (sha256&, const lookup&); void append_options (cstrings&, const strings&); @@ -171,7 +171,7 @@ namespace build2 find_option (const char* option, T&, const char* variable); bool - find_option (const char* option, const lookup&); + find_option (const char* option, const lookup&); // Parse version string in the X.Y.Z[-{a|b}N] to a version integer in the // AABBCCDD form, where: diff --git a/build2/utility.cxx b/build2/utility.cxx index 71aed79..8b772e0 100644 --- a/build2/utility.cxx +++ b/build2/utility.cxx @@ -144,14 +144,14 @@ namespace build2 const dir_path empty_dir_path; void - append_options (cstrings& args, const lookup& l) + append_options (cstrings& args, const lookup& l) { if (l) append_options (args, cast (l)); } void - hash_options (sha256& csum, const lookup& l) + hash_options (sha256& csum, const lookup& l) { if (l) hash_options (csum, cast (l)); @@ -177,7 +177,7 @@ namespace build2 } bool - find_option (const char* option, const lookup& l) + find_option (const char* option, const lookup& l) { if (l) { diff --git a/build2/variable b/build2/variable index 7b137aa..15f9c47 100644 --- a/build2/variable +++ b/build2/variable @@ -21,7 +21,7 @@ namespace build2 { class value; struct variable; - template struct lookup; + struct lookup; struct value_type { @@ -77,6 +77,8 @@ namespace build2 // // The two variables are considered the same if they have the same name. // + // @@ Document override semantics. + // struct variable { string name; @@ -197,18 +199,14 @@ namespace build2 template T& cast (value&); template T&& cast (value&&); template const T& cast (const value&); - - template T& cast (const lookup&); - template const T& cast (const lookup&); + template const T& cast (const lookup&); // As above but returns NULL if the value is NULL (or not defined, in // case of lookup). // template T* cast_null (value&); template const T* cast_null (const value&); - - template T* cast_null (const lookup&); - template const T* cast_null (const lookup&); + template const T* cast_null (const lookup&); // Assign value type to the value. Variable is normally only used for // diagnostics. @@ -230,10 +228,11 @@ namespace build2 // struct variable_map; - template struct lookup { - V* value; + using value_type = build2::value; + + const value_type* value; const variable_map* vars; bool @@ -243,22 +242,23 @@ namespace build2 // explicit operator bool () const {return defined () && !value->null ();} - V& operator* () const {return *value;} - V* operator-> () const {return value;} + const value_type& operator* () const {return *value;} + const value_type* operator-> () const {return value;} // Return true if this value belongs to the specified scope or target. - // Note that it can also be a target type/pattern-specific value. + // Note that it can also be a target type/pattern-specific value (in + // which case it won't belong to either). // template bool belongs (const T& x) const {return vars == &x.vars;} lookup (): value (nullptr), vars (nullptr) {} - lookup (V* v, const variable_map* vs) + lookup (const value_type* v, const variable_map* vs) : value (v), vars (v != nullptr ? vs : nullptr) {} template - lookup (V& v, const T& x): lookup (&v, &x.vars) {} + lookup (const value_type& v, const T& x): lookup (&v, &x.vars) {} }; // Representation types. @@ -590,32 +590,21 @@ namespace build2 using const_iterator = iterator_adapter; - lookup + // Lookup. Note that variable overrides will not be applied, even if + // set in this map. + // + lookup operator[] (const variable& var) const { - return lookup (find (var), this); + return lookup (find (var), this); } - lookup + lookup operator[] (const string& name) const { return operator[] (var_pool.find (name)); } - // Non-const lookup. Only exposed on the map directly. - // - lookup - operator[] (const variable& var) - { - return lookup (find (var), this); - } - - lookup - operator[] (const string& name) - { - return operator[] (var_pool.find (name)); - } - const value* find (const variable&) const; @@ -675,12 +664,13 @@ namespace build2 // consider its lifetime. // using variable_pattern_map = std::map; + using variable_type_map_base = std::map, + variable_pattern_map>; - struct variable_type_map: std::map, - variable_pattern_map> + struct variable_type_map: variable_type_map_base { - build2::lookup - lookup (const target_type&, const string& name, const variable&) const; + lookup + find (const target_type&, const string& tname, const variable&) const; }; } diff --git a/build2/variable.cxx b/build2/variable.cxx index 4597e9e..0870000 100644 --- a/build2/variable.cxx +++ b/build2/variable.cxx @@ -640,18 +640,16 @@ namespace build2 // variable_type_map // - lookup variable_type_map:: - lookup (const target_type& type, - const string& name, - const variable& var) const + lookup variable_type_map:: + find (const target_type& type, + const string& name, + const variable& var) const { - using result = build2::lookup; - // Search across target type hierarchy. // for (auto tt (&type); tt != nullptr; tt = tt->base) { - auto i (find (*tt)); + auto i (variable_type_map_base::find (*tt)); if (i == end ()) continue; @@ -698,11 +696,11 @@ namespace build2 //@@ TODO: should we detect ambiguity? 'foo-*' '*-foo' and // 'foo-foo'? Right now the last defined will be used. // - return result (v, &j->second); + return lookup (v, &j->second); } } } - return result (); + return lookup (); } } diff --git a/build2/variable.ixx b/build2/variable.ixx index a4d7dc3..ee0f647 100644 --- a/build2/variable.ixx +++ b/build2/variable.ixx @@ -104,15 +104,8 @@ namespace build2 } template - inline T& - cast (const lookup& l) - { - return cast (*l); - } - - template inline const T& - cast (const lookup& l) + cast (const lookup& l) { return cast (*l); } @@ -132,15 +125,8 @@ namespace build2 } template - inline T* - cast_null (const lookup& l) - { - return l ? &cast (*l) : nullptr; - } - - template inline const T* - cast_null (const lookup& l) + cast_null (const lookup& l) { return l ? &cast (*l) : nullptr; } -- cgit v1.1