Add lookup limit to {scope,target}::lookup_original()

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<string> (
-            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 <lang>[,...].
         //
@@ -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<bool> (*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<vector<name>> (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<vector<name>> (l), r, seen))
               return false;
@@ -1310,7 +1310,7 @@ namespace build2
                 //
                 if (const string* t = cast_null<string> (
                       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<string> (
               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<bool> (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<lookup, size_t> 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_type, size_t>
     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_type, size_t> 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_type, size_t>
     lookup_override (const variable& var,
                      pair<lookup_type, size_t> 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<lookup, size_t> 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<lookup, size_t> target::opstate::
-  lookup_original (const variable& var, bool target_only) const
+  lookup_original (const variable& var, lookup_limit limit) const
   {
     pair<lookup_type, size_t> 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_type, size_t>
     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_type, size_t>
     lookup_original (const variable&,
-                     bool target_only = false,
+                     lookup_limit = lookup_limit::none,
                      const scope* bs = nullptr,
                      bool locked = false) const;
 
+    pair<lookup_type, size_t>
+    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_type, size_t>
-      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