Tighten code that operates during both search/match and execute

9 files changed, 215 insertions, 100 deletions

diff --git a/build2/cc/common b/build2/cc/common
index 860913e..c631f38 100644
--- a/build2/cc/common
+++ b/build2/cc/common
@@ -217,7 +217,16 @@ namespace build2
       target*
       search_library (const dir_paths&,
                       optional<dir_paths>&,
-                      const prerequisite_key&) const;
+                      const prerequisite_key&,
+                      bool existing = false) const;
+
+      const target*
+      search_library_existing (const dir_paths& sysd,
+                               optional<dir_paths>& usrd,
+                               const prerequisite_key& pk) const
+      {
+        return search_library (sysd, usrd, pk, true);
+      }
 
       dir_paths
       extract_library_dirs (const scope&) const;
@@ -237,12 +246,14 @@ namespace build2
       bin::liba*
       msvc_search_static (const process_path&,
                           const dir_path&,
-                          const prerequisite_key&) const;
+                          const prerequisite_key&,
+                          bool existing) const;
 
       bin::libs*
       msvc_search_shared (const process_path&,
                           const dir_path&,
-                          const prerequisite_key&) const;
+                          const prerequisite_key&,
+                          bool existing) const;
 
     };
   }
diff --git a/build2/cc/common.cxx b/build2/cc/common.cxx
index 62a4ab2..654ff53 100644
--- a/build2/cc/common.cxx
+++ b/build2/cc/common.cxx
@@ -422,15 +422,16 @@ namespace build2
         //
         dir_path out;
         prerequisite_key pk {n.proj, {tt, &n.dir, &out, &n.value, ext}, &s};
-        xt = search_library (sysd, usrd, pk); //@@ TM const
+        xt = search_library_existing (sysd, usrd, pk);
 
         if (xt == nullptr)
         {
           if (n.qualified ())
-            xt = &import (pk); //@@ TM const
-          else
-            fail << "unable to find library " << pk;
+            xt = import_existing (pk);
         }
+
+        if (xt == nullptr)
+          fail << "unable to find library " << pk;
       }
 
       // If this is lib{}, pick appropriate member.
@@ -448,7 +449,8 @@ namespace build2
     target* common::
     search_library (const dir_paths& sysd,
                     optional<dir_paths>& usrd,
-                    const prerequisite_key& p) const
+                    const prerequisite_key& p,
+                    bool exist) const
     {
       tracer trace (x, "search_library");
 
@@ -544,11 +546,31 @@ namespace build2
       path f; // Reuse the buffer.
       const dir_path* pd (nullptr);
 
+      // Insert a target verifying that it already exists if requested.
+      //
+      auto insert = [&name, exist, &trace] (auto*& r,
+                                            const dir_path& d,
+                                            optional<string> ext)
+      {
+        using T = typename std::remove_reference<decltype (*r)>::type;
+
+        auto p (targets.insert (T::static_type,
+                                d,
+                                dir_path (),
+                                name,
+                                move (ext),
+                                true, // Implied.
+                                trace));
+
+        assert (!exist || !p.second);
+        r = static_cast<T*> (&p.first);
+      };
+
       auto search =[&a, &s,
                     &an, &ae,
                     &sn, &se,
                     &name, ext,
-                    &p, &f, &trace, this] (const dir_path& d) -> bool
+                    &p, &f, &insert, exist, this] (const dir_path& d) -> bool
       {
         timestamp mt;
 
@@ -571,17 +593,17 @@ namespace build2
             //
             if (tclass == "windows")
             {
-              s = &targets.insert<libs> (d, dir_path (), name, nullopt, trace);
+              insert (s, d, nullopt);
 
               if (s->member == nullptr)
               {
-                libi& i (
-                  targets.insert<libi> (d, dir_path (), name, se, trace));
+                libi* i;
+                insert (i, d, se);
 
-                if (i.path ().empty ())
-                  i.path (move (f));
+                if (i->path ().empty ())
+                  i->path (move (f));
 
-                i.mtime (mt);
+                i->mtime (mt);
 
                 // Presumably there is a DLL somewhere, we just don't know
                 // where (and its possible we might have to look for one if we
@@ -590,12 +612,12 @@ namespace build2
                 // but valid timestamp (aka "trust me, it's there").
                 //
                 s->mtime (mt);
-                s->member = &i;
+                s->member = i;
               }
             }
             else
             {
-              s = &targets.insert<libs> (d, dir_path (), name, se, trace);
+              insert (s, d, se);
 
               if (s->path ().empty ())
                 s->path (move (f));
@@ -617,7 +639,7 @@ namespace build2
 
             if (mt != timestamp_nonexistent)
             {
-              s = &targets.insert<libs> (d, dir_path (), name, se, trace);
+              insert (s, d, se);
 
               if (s->path ().empty ())
                 s->path (move (f));
@@ -644,7 +666,7 @@ namespace build2
             // Note that this target is outside any project which we treat
             // as out trees.
             //
-            a = &targets.insert<liba> (d, dir_path (), name, ae, trace);
+            insert (a, d, ae);
 
             if (a->path ().empty ())
               a->path (move (f));
@@ -661,10 +683,10 @@ namespace build2
           const process_path& ld (cast<process_path> (rs["bin.ld.path"]));
 
           if (s == nullptr && !sn.empty ())
-            s = msvc_search_shared (ld, d, p);
+            s = msvc_search_shared (ld, d, p, exist);
 
           if (a == nullptr && !an.empty ())
-            a = msvc_search_static (ld, d, p);
+            a = msvc_search_static (ld, d, p, exist);
         }
 
         return a != nullptr || s != nullptr;
@@ -705,6 +727,62 @@ namespace build2
       if (pd == nullptr)
         return nullptr;
 
+      // Enter (or find) the lib{} target group. Note that we must be careful
+      // here since its possible we have already imported some of its members.
+      //
+      lib* lt;
+      insert (lt, *pd, l ? p.tk.ext : nullopt);
+
+      // It should automatically link-up to the members we have found.
+      //
+      assert (a == nullptr || lt->a == a);
+      assert (s == nullptr || lt->s == s);
+
+      // Update the bin.lib variable to indicate what's available. Assume
+      // already done if existing.
+      //
+      if (!exist)
+      {
+        const char* bl (lt->a != nullptr
+                        ? (lt->s != nullptr ? "both" : "static")
+                        : "shared");
+        lt->assign (var_pool["bin.lib"]) = bl;
+      }
+
+      target* r (l ? lt : (p.is_a<liba> () ? static_cast<target*> (a) : s));
+
+      // Mark as a "cc" library (unless already marked) and set the system
+      // flag.
+      //
+      auto mark_cc = [sys, this] (target& t) -> bool
+      {
+        auto p (t.vars.insert (c_type));
+
+        if (p.second)
+        {
+          p.first.get () = string ("cc");
+
+          if (sys)
+            t.vars.assign (c_system) = true;
+        }
+
+        return p.second;
+      };
+
+      // If the library already has cc.type, then assume it was either already
+      // imported or was matched by a rule.
+      //
+      // Assume already done if existing.
+      //
+      if (!exist)
+      {
+        if (a != nullptr && !mark_cc (*a))
+          a = nullptr;
+
+        if (s != nullptr && !mark_cc (*s))
+          s = nullptr;
+      }
+
       // Add the "using static/shared library" macro (used, for example, to
       // handle DLL export). The absence of either of these macros would mean
       // some other build system that cannot distinguish between the two (and
@@ -754,55 +832,9 @@ namespace build2
         }
       };
 
-      // Enter (or find) the lib{} target group. Note that we must be careful
-      // here since its possible we have already imported some of its members.
-      //
-      lib& lt (
-        targets.insert<lib> (
-          *pd, dir_path (), name, l ? p.tk.ext : nullopt, trace));
-
-      // It should automatically link-up to the members we have found.
-      //
-      assert (a == nullptr || lt.a == a);
-      assert (s == nullptr || lt.s == s);
-
-      // Update the bin.lib variable to indicate what's available.
-      //
-      const char* bl (lt.a != nullptr
-                      ? (lt.s != nullptr ? "both" : "static")
-                      : "shared");
-      lt.assign (var_pool["bin.lib"]) = bl;
-
-      target* r (l ? &lt : (p.is_a<liba> () ? static_cast<target*> (a) : s));
-
-      // Mark as a "cc" library (unless already marked) and set the system
-      // flag.
-      //
-      auto mark_cc = [sys, this] (target& t) -> bool
-      {
-        auto p (t.vars.insert (c_type));
-
-        if (p.second)
-        {
-          p.first.get () = string ("cc");
-
-          if (sys)
-            t.vars.assign (c_system) = true;
-        }
-
-        return p.second;
-      };
-
-      // If the library already has cc.type, then assume it was either already
-      // imported or was matched by a rule.
+      // Assume already done if existing.
       //
-      if (a != nullptr && !mark_cc (*a))
-        a = nullptr;
-
-      if (s != nullptr && !mark_cc (*s))
-        s = nullptr;
-
-      if (a != nullptr || s != nullptr)
+      if (!exist && (a != nullptr || s != nullptr))
       {
         // Try to extract library information from pkg-config. We only add the
         // default macro if we could not extract more precise information. The
@@ -810,7 +842,7 @@ namespace build2
         // macros (or custom ones) from *.export.poptions.
         //
         if (pkgconfig == nullptr ||
-            !pkgconfig_extract (*p.scope, lt, a, s, p.proj, name, *pd, sysd))
+            !pkgconfig_extract (*p.scope, *lt, a, s, p.proj, name, *pd, sysd))
         {
           if (a != nullptr) add_macro (*a, "STATIC");
           if (s != nullptr) add_macro (*s, "SHARED");
diff --git a/build2/cc/link.cxx b/build2/cc/link.cxx
index 7c946aa..6c17037 100644
--- a/build2/cc/link.cxx
+++ b/build2/cc/link.cxx
@@ -693,7 +693,7 @@ namespace build2
       {
         // If we need an interface value, then use the group (lib{}).
         //
-        if (const target* g = exp && l.is_a<libs> () ? l.group : &l)
+        if (const target* g = exp && l.is_a<libs> () ? l.group.get () : &l)
         {
           const variable& var (
             com
@@ -738,7 +738,7 @@ namespace build2
       auto opt = [&cs, this] (
         const file& l, const string& t, bool com, bool exp)
       {
-        if (const target* g = exp && l.is_a<libs> () ? l.group : &l)
+        if (const target* g = exp && l.is_a<libs> () ? l.group.get () : &l)
         {
           const variable& var (
             com
diff --git a/build2/cc/msvc.cxx b/build2/cc/msvc.cxx
index fc2979c..86a7d38 100644
--- a/build2/cc/msvc.cxx
+++ b/build2/cc/msvc.cxx
@@ -225,7 +225,8 @@ namespace build2
                          const prerequisite_key& p,
                          otype lt,
                          const char* pfx,
-                         const char* sfx)
+                         const char* sfx,
+                         bool exist)
     {
       // Pretty similar logic to search_library().
       //
@@ -267,7 +268,15 @@ namespace build2
       {
         // Enter the target.
         //
-        T& t (targets.insert<T> (d, dir_path (), name, e, trace));
+        auto p (targets.insert (T::static_type,
+                                d,
+                                dir_path (),
+                                name,
+                                e,
+                                true, // Implied.
+                                trace));
+        assert (!exist || !p.second);
+        T& t (static_cast<T&> (p.first));
 
         if (t.path ().empty ())
           t.path (move (f));
@@ -282,14 +291,15 @@ namespace build2
     liba* common::
     msvc_search_static (const process_path& ld,
                         const dir_path& d,
-                        const prerequisite_key& p) const
+                        const prerequisite_key& p,
+                        bool exist) const
     {
       liba* r (nullptr);
 
-      auto search = [&r, &ld, &d, &p, this] (const char* pf, const char* sf)
-        -> bool
+      auto search = [&r, &ld, &d, &p, exist, this] (
+        const char* pf, const char* sf) -> bool
       {
-        r = msvc_search_library<liba> (x, ld, d, p, otype::a, pf, sf);
+        r = msvc_search_library<liba> (x, ld, d, p, otype::a, pf, sf, exist);
         return r != nullptr;
       };
 
@@ -309,20 +319,28 @@ namespace build2
     libs* common::
     msvc_search_shared (const process_path& ld,
                         const dir_path& d,
-                        const prerequisite_key& p) const
+                        const prerequisite_key& pk,
+                        bool exist) const
     {
       tracer trace (x, "msvc_search_shared");
 
       libs* r (nullptr);
 
-      auto search = [&r, &ld, &d, &p, &trace, this] (
+      auto search = [&r, &ld, &d, &pk, &trace, exist, this] (
         const char* pf, const char* sf) -> bool
       {
         if (libi* i =
-              msvc_search_library<libi> (x, ld, d, p, otype::s, pf, sf))
+            msvc_search_library<libi> (x, ld, d, pk, otype::s, pf, sf, exist))
         {
-          r = &targets.insert<libs> (
-            d, dir_path (), *p.tk.name, nullopt, trace);
+          auto p (targets.insert (libs::static_type,
+                                  d,
+                                  dir_path (),
+                                  *pk.tk.name,
+                                  nullopt,
+                                  true, // Implied.
+                                  trace));
+          assert (!exist || !p.second);
+          r = static_cast<libs*> (&p.first);
 
           if (r->member == nullptr)
           {
diff --git a/build2/file b/build2/file
index 696c5d7..482542f 100644
--- a/build2/file
+++ b/build2/file
@@ -172,6 +172,14 @@ namespace build2
 
   target&
   import (const prerequisite_key&);
+
+  // As above but only imports as an already existing target. Unlike the above
+  // version, this one can be called during the execute phase.
+  //
+  // Note: similar to search_existing().
+  //
+  const target*
+  import_existing (const prerequisite_key&);
 }
 
 #include <build2/file.ixx>
diff --git a/build2/file.cxx b/build2/file.cxx
index 3da1d35..c403eea 100644
--- a/build2/file.cxx
+++ b/build2/file.cxx
@@ -1142,8 +1142,8 @@ namespace build2
     return names (); // Never reached.
   }
 
-  target&
-  import (const prerequisite_key& pk)
+  target*
+  import (const prerequisite_key& pk, bool existing)
   {
     tracer trace ("import");
 
@@ -1155,7 +1155,7 @@ namespace build2
     const target_key& tk (pk.tk);
     const target_type& tt (*tk.type);
 
-    // Try to find the executable in PATH (or CWD is relative).
+    // Try to find the executable in PATH (or CWD if relative).
     //
     if (tt.is_a<exe> ())
     {
@@ -1174,24 +1174,36 @@ namespace build2
         path& p (pp.effect);
         assert (!p.empty ()); // We searched for a simple name.
 
-        exe& t (
-          targets.insert<exe> (
-            tt,
-            p.directory (),
-            dir_path (),                 // No out (out of project).
-            p.leaf ().base ().string (),
-            p.extension (),              // Always specified.
-            trace));
-
-        if (t.path ().empty ())
-          t.path (move (p));
-        else
-          assert (t.path () == p);
+        exe* t (
+          !existing
+          ? &targets.insert<exe> (tt,
+                                  p.directory (),
+                                  dir_path (),    // No out (out of project).
+                                  p.leaf ().base ().string (),
+                                  p.extension (), // Always specified.
+                                  trace)
+          : targets.find<exe> (tt,
+                               p.directory (),
+                               dir_path (),
+                               p.leaf ().base ().string (),
+                               p.extension (),
+                               trace));
+
+        if (t != nullptr)
+        {
+          if (t->path ().empty () && !existing)
+            t->path (move (p));
+          else
+            assert (t->path () == p);
 
-        return t;
+          return t;
+        }
       }
     }
 
+    if (existing)
+      return nullptr;
+
     // @@ We no longer have location. This is especially bad for the
     //    empty case, i.e., where do I need to specify the project
     //    name)? Looks like the only way to do this is to keep location
diff --git a/build2/file.ixx b/build2/file.ixx
index e12654c..be43857 100644
--- a/build2/file.ixx
+++ b/build2/file.ixx
@@ -9,4 +9,21 @@ namespace build2
   {
     return source_once (root, base, bf, base);
   }
+
+  target*
+  import (const prerequisite_key&, bool existing);
+
+  inline target&
+  import (const prerequisite_key& pk)
+  {
+    assert (phase == run_phase::search_match);
+    return *import (pk, false);
+  }
+
+  inline const target*
+  import_existing (const prerequisite_key& pk)
+  {
+    assert (phase == run_phase::search_match || phase == run_phase::execute);
+    return import (pk, true);
+  }
 }
diff --git a/build2/target b/build2/target
index 7653da6..2eb6a2c 100644
--- a/build2/target
+++ b/build2/target
@@ -1029,6 +1029,18 @@ namespace build2
       return find (target_key {&type, &dir, &out, &name, ext}, trace);
     }
 
+    template <typename T>
+    T*
+    find (const target_type& type,
+          const dir_path& dir,
+          const dir_path& out,
+          const string& name,
+          const optional<string>& ext,
+          tracer& trace) const
+    {
+      return static_cast<T*> (find (type, dir, out, name, ext, trace));
+    }
+
     // As above but ignore the extension.
     //
     template <typename T>
diff --git a/build2/target.cxx b/build2/target.cxx
index c032028..ad448cb 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -304,6 +304,11 @@ namespace build2
 
     if (t == nullptr)
     {
+      // We sometimes call insert() even if we expect to find an existing
+      // target in order to keep the same code (see cc/search_library()).
+      //
+      assert (phase != run_phase::execute);
+
       pair<target*, optional<string>> te (
         tt.factory (
           tt, move (dir), move (out), move (name), move (tk.ext)));