From b13b031b2b8a7390d1dd4b2658d0f0b62e43db47 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 31 Jul 2024 09:34:05 +0200 Subject: Add lookup limit to {scope,target}::lookup_original() --- libbuild2/cc/common.cxx | 12 +++++----- libbuild2/cc/link-rule.cxx | 10 ++++---- libbuild2/cc/pkgconfig.cxx | 4 ++-- libbuild2/scope.cxx | 49 +++++++++++++++++++++------------------- libbuild2/scope.hxx | 36 ++++++++++------------------- libbuild2/target.cxx | 15 ++++++------ libbuild2/target.hxx | 40 ++++++++++++++++++++++---------- libbuild2/test/script/script.cxx | 5 +++- libbuild2/variable.hxx | 9 ++++++++ 9 files changed, 100 insertions(+), 80 deletions(-) diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx index 9a4a07c..fcc8961 100644 --- a/libbuild2/cc/common.cxx +++ b/libbuild2/cc/common.cxx @@ -185,7 +185,7 @@ namespace build2 // const string* pt ( cast_null ( - l.state[a].lookup_original (c_type, true /* target_only */).first)); + l.state[a].lookup_original (c_type, lookup_limit::target).first)); // cc.type value format is [,...]. // @@ -242,7 +242,7 @@ namespace build2 // { const variable& v (impl ? c_export_impl_libs : c_export_libs); - c_e_libs = l.lookup_original (v, false, &bs).first; + c_e_libs = l.lookup_original (v, &bs).first; } if (!cc) @@ -251,7 +251,7 @@ namespace build2 same ? (impl ? x_export_impl_libs : x_export_libs) : vp[t + (impl ? ".export.impl_libs" : ".export.libs")]); - x_e_libs = l.lookup_original (v, false, &bs).first; + x_e_libs = l.lookup_original (v, &bs).first; } // Process options first. @@ -669,7 +669,7 @@ namespace build2 // See the link rule for the lookup semantics. // lookup l ( - t->lookup_original (var, true /* target_only */).first); + t->lookup_original (var, lookup_limit::target).first); if (l ? cast (*l) : u) lf |= lflag_whole; @@ -778,8 +778,8 @@ namespace build2 if (proc_lib) { const variable& v (same ? x_libs : vp[t + ".libs"]); - proc_impl (l.lookup_original (c_libs, false, &bs).first); - proc_impl (l.lookup_original (v, false, &bs).first); + proc_impl (l.lookup_original (c_libs, &bs).first); + proc_impl (l.lookup_original (v, &bs).first); } } } diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index 417cba5..8c68b64 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -102,13 +102,13 @@ namespace build2 // const scope& bs (t->base_scope ()); - if (lookup l = t->lookup_original (c_export_libs, false, &bs).first) + if (lookup l = t->lookup_original (c_export_libs, &bs).first) { if (!deduplicate_export_libs (bs, cast> (l), r, seen)) return false; } - if (lookup l = t->lookup_original (x_export_libs, false, &bs).first) + if (lookup l = t->lookup_original (x_export_libs, &bs).first) { if (!deduplicate_export_libs (bs, cast> (l), r, seen)) return false; @@ -1310,7 +1310,7 @@ namespace build2 // if (const string* t = cast_null ( ft->state[a].lookup_original ( - c_type, true /* target_only */).first)) + c_type, lookup_limit::target).first)) { if (recursively_binless (*t)) continue; @@ -1330,7 +1330,7 @@ namespace build2 { auto find = [&t, &bs] (const variable& v) -> lookup { - return t.lookup_original (v, false, &bs).first; + return t.lookup_original (v, &bs).first; }; auto has_simple = [] (lookup l) @@ -1975,7 +1975,7 @@ namespace build2 lookup l (p.prerequisite.vars[var]); if (!l.defined ()) - l = pt->lookup_original (var, true /* target_only */).first; + l = pt->lookup_original (var, lookup_limit::target).first; if (!l.defined ()) { diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx index 046fbc8..7e47534 100644 --- a/libbuild2/cc/pkgconfig.cxx +++ b/libbuild2/cc/pkgconfig.cxx @@ -2256,7 +2256,7 @@ namespace build2 const string& t ( cast ( l.state[a].lookup_original ( - c_type, true /* target_only */).first)); + c_type, lookup_limit::target).first)); // If common, then only save the language (the rest could be // static/shared-specific; strictly speaking even the language could @@ -2276,7 +2276,7 @@ namespace build2 // if (cast_false (l.lookup_original ( ctx.var_pool["bin.whole"], - true /* target_only */).first)) + lookup_limit::target).first)) { os << endl << "bin.whole = true" << endl; diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx index e0bc55a..8710a3d 100644 --- a/libbuild2/scope.cxx +++ b/libbuild2/scope.cxx @@ -43,20 +43,23 @@ namespace build2 // Definition of adhoc_rule_pattern. } - scope::original_info scope:: - lookup_original_info (const variable& var, - const target_key* tk, - const target_key* g1k, - const target_key* g2k, - size_t start_d) const + pair scope:: + lookup_original (const variable& var, + const target_key* tk, + const target_key* g1k, + const target_key* g2k, + lookup_limit limit, + size_t start_d) const { - assert (tk != nullptr || var.visibility != variable_visibility::target); - assert (g2k == nullptr || g1k != nullptr); + assert ((tk != nullptr || var.visibility != variable_visibility::target) && + (g2k == nullptr || g1k != nullptr) && + (limit == lookup_limit::none || + (limit == lookup_limit::target_type || tk != nullptr))); size_t d (0); if (var.visibility == variable_visibility::prereq) - return original_info {{lookup_type (), d}, false}; + return make_pair (lookup_type (), d); // Process target type/pattern-specific prepend/append values. // @@ -79,7 +82,9 @@ namespace build2 // group, then we shouldn't be looking for stem in the target's // variables. In other words, once we "jump" to group, we stay there. // - lookup_type stem (s->lookup_original (var, tk, g1k, g2k, 2).first); + lookup_type stem ( + s->lookup_original ( + var, tk, g1k, g2k, lookup_limit::none, 2).first); // Check the cache. // @@ -162,11 +167,10 @@ namespace build2 if (l.defined ()) { - bool pa (l->extra != 0); // Prepend/append? - if (pa) + if (l->extra != 0) // Prepend/append? pre_app (l, s, tk, g1k, g2k, move (*tn)); - return original_info {{move (l), d}, pa}; + return make_pair (move (l), d); } } } @@ -181,11 +185,10 @@ namespace build2 if (l.defined ()) { - bool pa (l->extra != 0); // Prepend/append? - if (pa) + if (l->extra != 0) // Prepend/append? pre_app (l, s, g1k, g2k, nullptr, move (*g1n)); - return original_info {{move (l), d}, pa}; + return make_pair (move (l), d); } if (g2k != nullptr) @@ -194,11 +197,10 @@ namespace build2 if (l.defined ()) { - bool pa (l->extra != 0); // Prepend/append? - if (pa) + if (l->extra != 0) // Prepend/append? pre_app (l, s, g2k, nullptr, nullptr, move (*g2n)); - return original_info {{move (l), d}, pa}; + return make_pair (move (l), d); } } } @@ -208,12 +210,13 @@ namespace build2 // Note that we still increment the lookup depth so that we can compare // depths of variables with different visibilities. // - if (++d >= start_d && var.visibility != variable_visibility::target) + if (++d >= start_d && + limit != lookup_limit::target_type && + var.visibility != variable_visibility::target) { auto p (s->vars.lookup (var)); if (p.first != nullptr) - return original_info { - {lookup_type (*p.first, p.second, s->vars), d}, false}; + return make_pair (lookup_type (*p.first, p.second, s->vars), d); } switch (var.visibility) @@ -233,7 +236,7 @@ namespace build2 } } - return original_info {{lookup_type (), size_t (~0)}, false}; + return make_pair (lookup_type (), size_t (~0)); } scope::override_info scope:: diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx index f821411..ece78b7 100644 --- a/libbuild2/scope.hxx +++ b/libbuild2/scope.hxx @@ -186,37 +186,25 @@ namespace build2 return var.overrides == nullptr ? p : lookup_override (var, move (p)); } - // Implementation details (used by scope target lookup). The start_depth - // can be used to skip a number of initial lookups. + // Implementation details (used by scope and target lookup). + // + // The only valid values for limit are none and target_type and in the + // latter case the target key should not be NULL. If it is target_type, + // then only look in target type/pattern-specific variables. Note that if + // a target type/pattern-specific append/prepend modifies a scope + // variable, then the resulting value is considered target type/pattern- + // specific. + // + // The start_depth can be used to skip a number of initial lookups. // pair lookup_original (const variable& var, const target_key* tk = nullptr, const target_key* g1k = nullptr, const target_key* g2k = nullptr, - size_t start_depth = 1) const - { - return lookup_original_info (var, tk, g1k, g2k, start_depth).lookup; - } - - // As above but also return an indication of whether the resulting value - // was modified by a target type/pattern-specific append/prepend. - // - struct original_info - { - pair lookup; - bool modified; - }; - - original_info - lookup_original_info (const variable&, - const target_key* tk, - const target_key* g1k = nullptr, - const target_key* g2k = nullptr, - size_t start_depth = 1) const; + lookup_limit limit = lookup_limit::none, + size_t start_depth = 1) const; - // Implementation details (used by scope target lookup). - // pair lookup_override (const variable& var, pair original, diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index 1a72788..65e18d3 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -142,7 +142,7 @@ namespace build2 pair target:: lookup_original (const variable& var, - bool target_only, + lookup_limit limit, const scope* bs, bool locked) const { @@ -204,7 +204,7 @@ namespace build2 // if (!r.first) { - if (!target_only) + if (limit != lookup_limit::target) { auto key = [locked] (const target* t) { @@ -221,7 +221,8 @@ namespace build2 auto p (bs->lookup_original (var, &tk, g1 != nullptr ? &g1k : nullptr, - g2 != nullptr ? &g2k : nullptr)); + g2 != nullptr ? &g2k : nullptr, + limit)); r.first = move (p.first); r.second = r.first ? r.second + p.second : p.second; @@ -241,7 +242,7 @@ namespace build2 // Note that here we want the original value without any overrides // applied. // - auto l (lookup_original (var, false, bs).first); + auto l (lookup_original (var, bs).first); if (l.defined () && l.belongs (*this)) // Existing var in this target. return vars.modify (l); // Ok since this is original. @@ -257,7 +258,7 @@ namespace build2 value& target:: append_locked (const variable& var, const scope* bs) { - auto l (lookup_original (var, false, bs, true /* locked */).first); + auto l (lookup_original (var, bs, true /* locked */).first); if (l.defined () && l.belongs (*this)) // Existing var in this target. return vars.modify (l); // Ok since this is original. @@ -271,7 +272,7 @@ namespace build2 } pair target::opstate:: - lookup_original (const variable& var, bool target_only) const + lookup_original (const variable& var, lookup_limit limit) const { pair r (lookup_type (), 0); @@ -286,7 +287,7 @@ namespace build2 // if (!r.first) { - auto p (target_->lookup_original (var, target_only)); + auto p (target_->lookup_original (var, limit)); r.first = move (p.first); r.second = r.first ? r.second + p.second : p.second; diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index da2ab73..c1da26c 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -773,14 +773,16 @@ namespace build2 // From the innermost scope's target type/patter-specific variables for // the target -- 3. From the innermost scope's target type/patter-specific // variables for the group -- 4. From the innermost scope's variables -- - // 5. And so on. The idea is that given two lookups from the same target, - // we can say which one came earlier. If no value is found, then the depth - // is set to ~0. + // 5. And so on. Note that if a target type/patter-specific append/prepend + // was applied, then the returned depth is of the innermost such + // append/prepend. The idea is that given two lookups from the same + // target, we can say which one came earlier. If no value is found, then + // the depth is set to ~0. // pair lookup (const variable& var, const scope* bs = nullptr) const { - auto p (lookup_original (var, false, bs)); + auto p (lookup_original (var, bs)); return var.overrides == nullptr ? p : (bs != nullptr @@ -788,17 +790,31 @@ namespace build2 : base_scope ()).lookup_override (var, move (p), true); } - // If target_only is true, then only look in target and its target group - // without continuing in scopes. As an optimization, the caller can also - // pass the base scope of the target, if already known. If locked is true, - // assume the targets mutex is locked. + // If limit is target, then only look in target and its target group + // without continuing in scopes. If it is target_type, then additionally + // look in target type/pattern-specific variables in scopes (see + // scope::lookup_original() for the exact semantics). + // + // As an optimization, the caller can also pass the base scope of the + // target, if already known. + // + // If locked is true, assume the targets mutex is locked (not relevant if + // limit is target). // pair lookup_original (const variable&, - bool target_only = false, + lookup_limit = lookup_limit::none, const scope* bs = nullptr, bool locked = false) const; + pair + lookup_original (const variable& var, + const scope* bs, + bool locked = false) const + { + return lookup_original (var, lookup_limit::none, bs, locked); + } + // Return a value suitable for assignment. See scope for details. // value& @@ -991,11 +1007,11 @@ namespace build2 : target_->base_scope ().lookup_override (var, move (p), true, true); } - // If target_only is true, then only look in target and its target group - // without continuing in scopes. + // The limit semantics is the same as in target::lookup_original(). // pair - lookup_original (const variable&, bool target_only = false) const; + lookup_original (const variable&, + lookup_limit = lookup_limit::none) const; // Return a value suitable for assignment. See target for details. // diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index f7827f6..7862120 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -369,7 +369,10 @@ namespace build2 // value. In this case, presumably the override also affects the // script target and we will pick it up there. A bit fuzzy. // - auto p (root.test_target.lookup_original (var, target_only)); + auto p ( + root.test_target.lookup_original ( + var, + target_only ? lookup_limit::target : lookup_limit::none)); if (p.first) { diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx index 6dfbbc6..eebb767 100644 --- a/libbuild2/variable.hxx +++ b/libbuild2/variable.hxx @@ -612,6 +612,15 @@ namespace build2 vars (v != nullptr ? m : nullptr) {} }; + // Variable lookup limit (see {scope,target}::lookup_original()). + // + enum class lookup_limit + { + none, + target_type, + target + }; + // Two lookups are equal if they point to the same variable. // inline bool -- cgit v1.1