From 363aaeff5ffe398e1b82ef4bef3a96e6e46313ca Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 11 Apr 2016 15:47:36 +0200 Subject: Cleanup find_override() implementation --- build2/scope.cxx | 160 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 91 insertions(+), 69 deletions(-) diff --git a/build2/scope.cxx b/build2/scope.cxx index f0d28e0..b22cb0c 100644 --- a/build2/scope.cxx +++ b/build2/scope.cxx @@ -76,24 +76,23 @@ namespace build2 // // Note also that we rely (e.g., in the config module) on the fact that if // no overrides apply, then we return the original value and not its copy - // in the cache (this can be used to detect if the value was overriden). - // + // in the cache (this is used to detect if the value was overriden). // assert (var.override != nullptr); - lookup& origl (original.first); - size_t origd (original.second); + const lookup& orig (original.first); + size_t orig_depth (original.second); // The first step is to find out where our cache will reside. After some - // meditation it becomes clear it should be next to the innermost (scope- - // wise) value (override or original) that could contribute to the end - // result. + // meditation you will see it should be next to the innermost (scope-wise) + // value of this variable (override or original). // - const variable_map* vars (nullptr); - - // Root scope of a project from which our initial value comes. See below. + // We also keep track of the root scope of the project from which this + // innermost value comes. This is used to decide whether a non-recursive + // project-wise override applies. // - const scope* proj (nullptr); + const variable_map* inner_vars (nullptr); + const scope* inner_proj (nullptr); // One special case is if the original is target-specific, which is the // most innermost. Or is it innermostest? @@ -101,28 +100,30 @@ namespace build2 bool targetspec (false); if (target) { - targetspec = origl.defined () && (origd == 1 || origd == 2); + targetspec = orig.defined () && (orig_depth == 1 || orig_depth == 2); if (targetspec) { - vars = origl.vars; - proj = root_scope (); + inner_vars = orig.vars; + inner_proj = root_scope (); } } const scope* s; // Return true if the override applies. Note that it expects vars and proj - // to be not NULL; if there is nothing "inner", then any override will - // still be "visible". + // to be not NULL; if there is nothing "more inner", then any override + // will still be "visible". // - auto applies = [&vars, &s] (const variable* o, const scope* proj) -> bool + auto applies = [&s] (const variable* o, + const variable_map* vars, + const scope* proj) -> bool { switch (o->visibility) { case variable_visibility::scope: { - // Does not apply if the innermost value is not in this scope. + // Does not apply if in a different scope. // if (vars != &s->vars) return false; @@ -145,7 +146,8 @@ namespace build2 return true; }; - // Return override value if it is present and optionally ends with suffix. + // Return the override value if it is present and (optionally) ends with + // a suffix. // auto find = [&s] (const variable* o, const char* sf = nullptr) -> lookup { @@ -158,17 +160,20 @@ namespace build2 // Return true if a value is from this scope (either target type/pattern- // specific or ordinary). // - auto belongs = [&s] (const lookup& l) -> bool + auto belongs = [&s, target] (const lookup& l) -> bool { - for (auto& p1: s->target_vars) - for (auto& p2: p1.second) - if (l.vars == &p2.second) - return true; + if (target) + { + for (auto& p1: s->target_vars) + for (auto& p2: p1.second) + if (l.vars == &p2.second) + return true; + } return l.vars == &s->vars; }; - // While looking for the cache we can also detect if none of the overrides + // While looking for the cache we also detect if none of the overrides // apply. In this case the result is simply the original value (if any). // bool apply (false); @@ -180,43 +185,44 @@ namespace build2 // the target type/patter-specific variables, which is "more inner" than // normal scope variables (see find_original()). // - if (vars == nullptr && origl.defined () && belongs (origl)) + if (inner_vars == nullptr && orig.defined () && belongs (orig)) { - vars = origl.vars; - proj = s->root_scope (); // This is so we skip non-recursive overrides - // that would not apply. We reset it later. + inner_vars = orig.vars; + inner_proj = s->root_scope (); } for (const variable* o (var.override.get ()); o != nullptr; o = o->override.get ()) { - if (vars != 0 && !applies (o, proj)) + if (inner_vars != nullptr && !applies (o, inner_vars, inner_proj)) continue; auto l (find (o)); if (l.defined ()) { - if (vars == nullptr) - vars = l.vars; + if (inner_vars == nullptr) + { + inner_vars = l.vars; + inner_proj = s->root_scope (); // Not actually used. + } apply = true; break; } } - // If we found the cache and at least one override applies, then we can - // stop. + // We can stop if we found the cache and at least one override applies. // - if (vars != nullptr && apply) + if (inner_vars != nullptr && apply) break; } if (!apply) return move (original); - assert (vars != nullptr); + assert (inner_vars != nullptr); // Implementing proper caching is tricky so for now we are going to re- // calculate the value every time. Later, the plan is to use value @@ -228,15 +234,26 @@ namespace build2 // @@ MT // variable_override_value& cache ( - variable_override_cache[make_pair (vars, &var)]); + variable_override_cache[make_pair (inner_vars, &var)]); // Now find our "stem", that is the value to which we will be appending // suffixes and prepending prefixes. This is either the original or the - // __override provided it applies. We may also not have either. + // __override, provided it applies. We may also not have either. // - lookup stem (targetspec ? origl : lookup ()); - size_t depth (targetspec ? origd : 0); - size_t ovrd (target ? 2 : 0); // For implied target-specific lookup. + lookup stem; + size_t stem_depth (0); + const scope* stem_proj (nullptr); + + // Again the special case of a target-specific variable. + // + if (targetspec) + { + stem = orig; + stem_depth = orig_depth; + stem_proj = root_scope (); + } + + size_t ovr_depth (target ? 2 : 0); // For implied target-specific lookup. for (s = this; s != nullptr; s = s->parent_scope ()) { @@ -244,15 +261,15 @@ namespace build2 // First check if the original is from this scope. // - if (origl.defined () && belongs (origl)) + if (orig.defined () && belongs (orig)) { - stem = origl; - depth = origd; - proj = s->root_scope (); + stem = orig; + stem_depth = orig_depth; + stem_proj = s->root_scope (); // Keep searching. } - ++ovrd; + ++ovr_depth; // Then look for an __override that applies. // @@ -263,16 +280,16 @@ namespace build2 // If we haven't yet found anything, then any override will still be // "visible" even if it doesn't apply. // - if (stem.defined () && !applies (o, root_scope ())) + if (stem.defined () && !applies (o, stem.vars, stem_proj)) continue; auto l (find (o, ".__override")); if (l.defined ()) { - depth = ovrd; stem = move (l); - proj = s->root_scope (); + stem_depth = ovr_depth; + stem_proj = s->root_scope (); done = true; break; } @@ -311,26 +328,30 @@ namespace build2 if (cache.value.type == nullptr && var.type != nullptr) typify (cache.value, *var.type, var); - // Now apply override prefixes and suffixes. + // Now apply override prefixes and suffixes. Also calculate the vars and + // depth of the result, which will be those of the stem or prefix/suffix + // that applies, whichever is the innermost. // - ovrd = target ? 2 : 0; - const variable_map* ovrv (cache.stem_vars); + size_t depth (stem_depth); + const variable_map* vars (stem.vars); + const scope* proj (stem_proj); + + ovr_depth = target ? 2 : 0; for (s = this; s != nullptr; s = s->parent_scope ()) { - ++ovrd; + ++ovr_depth; for (const variable* o (var.override.get ()); o != nullptr; o = o->override.get ()) { - // First see if this override applies. This is actually tricky: what - // if the stem is a "visible" override from an outer project? - // Shouldn't its overrides apply? Sure sounds logical. So it seems - // we should use the project of the stem's scope and not the project - // of this scope. + // First see if this override applies. This is tricky: what if the + // stem is a "visible" override from an outer project? Shouldn't its + // overrides apply? Sure sounds logical. So we use the project of the + // stem's scope. // - if (proj != nullptr && !applies (o, proj)) + if (vars != nullptr && !applies (o, vars, proj)) continue; // Note that we keep override values as untyped names even if the @@ -350,20 +371,21 @@ namespace build2 if (l.defined ()) { - // If we had no stem, use the scope of the first override that - // applies as the project. For vars/depth we need to pick the - // innermost. + // If we had no stem, use the first override as a surrogate stem. // - if (proj == nullptr) + if (vars == nullptr) { + depth = ovr_depth; + vars = &s->vars; proj = s->root_scope (); - depth = ovrd; - ovrv = &s->vars; } - else if (ovrd < depth) + // Otherwise, pick the innermost location between the stem and + // prefix/suffix. + // + else if (ovr_depth < depth) { - depth = ovrd; - ovrv = &s->vars; + depth = ovr_depth; + vars = &s->vars; } } } @@ -372,7 +394,7 @@ namespace build2 // Use the location of the innermost value that contributed as the // location of the result. // - return make_pair (lookup (&cache.value, ovrv), depth); + return make_pair (lookup (&cache.value, vars), depth); } value& scope:: -- cgit v1.1