Redesign target_set interface in preparation for locking

6 files changed, 44 insertions, 43 deletions

diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx
index 1fed762..590ba7c 100644
--- a/build2/cc/compile.cxx
+++ b/build2/cc/compile.cxx
@@ -981,8 +981,7 @@ namespace build2
             // an existing file) and go straight for the target object since
             // we need to find the target explicitly spelled out.
             //
-            auto i (targets.find (*tt, d, out, n, e, trace));
-            r = i != targets.end () ? i->get () : nullptr;
+            r = targets.find (*tt, d, out, n, e, trace);
           }
 
           return static_cast<path_target*> (r);
diff --git a/build2/operation.cxx b/build2/operation.cxx
index 4e65783..5815834 100644
--- a/build2/operation.cxx
+++ b/build2/operation.cxx
@@ -76,11 +76,10 @@ namespace build2
 
     phase_guard pg (run_phase::search_match);
 
-    auto i (targets.find (tk, trace));
-    if (i == targets.end ())
+    if (target* t = targets.find (tk, trace))
+      ts.push_back (t);
+    else
       fail (l) << "unknown target " << tk;
-
-    ts.push_back (i->get ());
   }
 
   void
diff --git a/build2/parser.cxx b/build2/parser.cxx
index c48159a..e3f0c70 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -3549,7 +3549,7 @@ namespace build2
                       dir_path (),         // Out tree target.
                       string (),
                       nullopt,
-                      trace) != targets.end ())
+                      trace) != nullptr)
       return;
 
     target& dt (*default_target_);
diff --git a/build2/search.cxx b/build2/search.cxx
index 65eb2fd..8f621c2 100644
--- a/build2/search.cxx
+++ b/build2/search.cxx
@@ -71,15 +71,13 @@ namespace build2
         o.clear ();
     }
 
-    auto i (targets.find (*tk.type, d, o, *tk.name, tk.ext, trace));
+    target* t (targets.find (*tk.type, d, o, *tk.name, tk.ext, trace));
 
-    if (i == targets.end ())
-      return 0;
+    if (t != nullptr)
+      l5 ([&]{trace << "existing target " << t
+                    << " for prerequisite " << pk;});
 
-    target& t (**i);
-
-    l5 ([&]{trace << "existing target " << t << " for prerequisite " << pk;});
-    return &t;
+    return t;
   }
 
   target*
diff --git a/build2/target b/build2/target
index 42c0361..fc6e782 100644
--- a/build2/target
+++ b/build2/target
@@ -971,16 +971,19 @@ namespace build2
   // map. The key's hash ignores the extension, so the hash will stay stable
   // across extension updates.
   //
+  // Note also that once the extension is specified, it becomes immutable.
+  //
   class target_set
   {
   public:
-    typedef std::unordered_map<target_key, unique_ptr<target>> map;
-    typedef butl::map_iterator_adapter<map::const_iterator> iterator;
+    using map_type = std::unordered_map<target_key, unique_ptr<target>>;
 
-    iterator
+    // Return existing target or NULL.
+    //
+    target*
     find (const target_key& k, tracer& trace) const;
 
-    iterator
+    target*
     find (const target_type& type,
           const dir_path& dir,
           const dir_path& out,
@@ -991,8 +994,7 @@ namespace build2
       return find (target_key {&type, &dir, &out, &name, ext}, trace);
     }
 
-    // As above but ignore the extension and return the target or nullptr
-    // instead of the iterator.
+    // As above but ignore the extension.
     //
     template <typename T>
     T*
@@ -1003,9 +1005,6 @@ namespace build2
       return i != map_.end () ? static_cast<T*> (i->second.get ()) : nullptr;
     }
 
-    iterator begin () const {return map_.begin ();}
-    iterator end () const {return map_.end ();}
-
     pair<target&, bool>
     insert (const target_type&,
             dir_path dir,
@@ -1057,11 +1056,19 @@ namespace build2
         insert (T::static_type, dir, out, name, nullopt, false, t).first);
     }
 
+    // Note: not MT-safe so can only be used during serial execution.
+    //
+  public:
+    using iterator = butl::map_iterator_adapter<map_type::const_iterator>;
+
+    iterator begin () const {return map_.begin ();}
+    iterator end ()   const {return map_.end ();}
+
     void
     clear () {map_.clear ();}
 
   private:
-    map map_;
+    map_type map_;
   };
 
   extern target_set targets;
diff --git a/build2/target.cxx b/build2/target.cxx
index ca3f6f2..10b1432 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -198,21 +198,22 @@ namespace build2
   //
   target_set targets;
 
-  auto target_set::
-  find (const target_key& k, tracer& trace) const -> iterator
+  target* target_set::
+  find (const target_key& k, tracer& trace) const
   {
-    map::const_iterator i (map_.find (k));
+    target* t (nullptr);
+    map_type::const_iterator i (map_.find (k));
 
     if (i != map_.end ())
     {
-      target& t (*i->second);
+      t = i->second.get ();
       optional<string>& ext (i->first.ext);
 
       if (ext != k.ext)
       {
         l5 ([&]{
             diag_record r (trace);
-            r << "assuming target " << t << " is the same as the one with ";
+            r << "assuming target " << *t << " is the same as the one with ";
             if (!k.ext)
               r << "unspecified extension";
             else if (k.ext->empty ())
@@ -227,7 +228,7 @@ namespace build2
       }
     }
 
-    return i;
+    return t;
   }
 
   pair<target&, bool> target_set::
@@ -241,8 +242,8 @@ namespace build2
   {
     target_key tk {&tt, &dir, &out, &name, move (ext)};
 
-    iterator i (find (tk, trace));
-    bool r (i == end ());
+    target* t (find (tk, trace));
+    bool r (t == nullptr);
 
     if (r)
     {
@@ -250,28 +251,25 @@ namespace build2
         tt.factory (
           tt, move (dir), move (out), move (name), move (tk.ext)));
 
-      target& t (*p.first);
+      t = p.first;
 
-      map::iterator j (
+      map_type::iterator i (
         map_.emplace (
-          target_key {&tt, &t.dir, &t.out, &t.name, move (p.second)},
+          target_key {&tt, &t->dir, &t->out, &t->name, move (p.second)},
           p.first).first);
 
-      t.ext_ = &j->first.ext;
-      t.implied = implied;
-      i = j;
+      t->ext_ = &i->first.ext;
+      t->implied = implied;
     }
     else if (!implied)
     {
       // Clear the implied flag.
       //
-      target& t (**i);
-
-      if (t.implied)
-        t.implied = false;
+      if (t->implied)
+        t->implied = false;
     }
 
-    return pair<target&, bool> (**i, r);
+    return pair<target&, bool> (*t, r);
   }
 
   ostream&