From 8e0e8edb727a5367d991880b033eb13060f4c8eb Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Mon, 26 Aug 2019 07:34:15 +0200
Subject: Make target types project-wide

---
 build2/bin/init.cxx     | 69 +++++++++++++++++++++++++------------------------
 build2/cc/module.cxx    | 12 ++++-----
 build2/cli/init.cxx     |  9 +++----
 libbuild2/bash/init.cxx |  5 ++--
 libbuild2/context.cxx   | 13 ++++++----
 libbuild2/context.hxx   | 12 +++++----
 libbuild2/file.cxx      |  7 ++---
 libbuild2/in/init.cxx   |  2 +-
 libbuild2/parser.cxx    |  2 +-
 libbuild2/scope.cxx     | 39 ++++++++++------------------
 libbuild2/scope.hxx     | 47 ++++++++++++++++++++++++++++-----
 libbuild2/test/init.cxx |  6 ++---
 12 files changed, 124 insertions(+), 99 deletions(-)

diff --git a/build2/bin/init.cxx b/build2/bin/init.cxx
index 4e80834..54bd84a 100644
--- a/build2/bin/init.cxx
+++ b/build2/bin/init.cxx
@@ -402,7 +402,7 @@ namespace build2
           scope& bs,
           const location& loc,
           unique_ptr<module_base>&,
-          bool,
+          bool first,
           bool,
           const variable_map& hints)
     {
@@ -421,41 +421,41 @@ namespace build2
       // Register target types and configure their default "installability".
       //
       bool install_loaded (cast_false<bool> (rs["install.loaded"]));
-
       {
         using namespace install;
 
-        auto& t (bs.target_types);
-
-        t.insert<obj>  ();
-        t.insert<obje> ();
-        t.insert<obja> ();
-        t.insert<objs> ();
-
-        t.insert<bmi>  ();
-        t.insert<bmie> ();
-        t.insert<bmia> ();
-        t.insert<bmis> ();
-
-        t.insert<hbmi>  ();
-        t.insert<hbmie> ();
-        t.insert<hbmia> ();
-        t.insert<hbmis> ();
-
-        t.insert<libul> ();
-        t.insert<libue> ();
-        t.insert<libua> ();
-        t.insert<libus> ();
-
-        t.insert<lib>  ();
-        t.insert<liba> ();
-        t.insert<libs> ();
-
-        // Register the def{} target type. Note that we do it here since it is
-        // input and can be specified unconditionally (i.e., not only when
-        // building for Windows).
-        //
-        t.insert<def> ();
+        if (first)
+        {
+          rs.insert_target_type<obj>  ();
+          rs.insert_target_type<obje> ();
+          rs.insert_target_type<obja> ();
+          rs.insert_target_type<objs> ();
+
+          rs.insert_target_type<bmi>  ();
+          rs.insert_target_type<bmie> ();
+          rs.insert_target_type<bmia> ();
+          rs.insert_target_type<bmis> ();
+
+          rs.insert_target_type<hbmi>  ();
+          rs.insert_target_type<hbmie> ();
+          rs.insert_target_type<hbmia> ();
+          rs.insert_target_type<hbmis> ();
+
+          rs.insert_target_type<libul> ();
+          rs.insert_target_type<libue> ();
+          rs.insert_target_type<libua> ();
+          rs.insert_target_type<libus> ();
+
+          rs.insert_target_type<lib>  ();
+          rs.insert_target_type<liba> ();
+          rs.insert_target_type<libs> ();
+
+          // Register the def{} target type. Note that we do it here since it
+          // is input and can be specified unconditionally (i.e., not only
+          // when building for Windows).
+          //
+          rs.insert_target_type<def> ();
+        }
 
         // Note: libu*{} members are not installable.
         //
@@ -494,7 +494,8 @@ namespace build2
         {
           // Import library.
           //
-          t.insert<libi> ();
+          if (first)
+            rs.insert_target_type<libi> ();
 
           if (install_loaded)
           {
diff --git a/build2/cc/module.cxx b/build2/cc/module.cxx
index 064d954..bd853cc 100644
--- a/build2/cc/module.cxx
+++ b/build2/cc/module.cxx
@@ -642,13 +642,11 @@ namespace build2
       {
         using namespace install;
 
-        auto& tts (rs.target_types);
+        rs.insert_target_type (x_src);
 
-        tts.insert (x_src);
-
-        auto insert_hdr = [&rs, &tts, install_loaded] (const target_type& tt)
+        auto insert_hdr = [&rs, install_loaded] (const target_type& tt)
         {
-          tts.insert (tt);
+          rs.insert_target_type (tt);
 
           // Install headers into install.include.
           //
@@ -666,8 +664,8 @@ namespace build2
         if (*x_hdr != &h::static_type)
           insert_hdr (h::static_type);
 
-        tts.insert<pca> ();
-        tts.insert<pcs> ();
+        rs.insert_target_type<pca> ();
+        rs.insert_target_type<pcs> ();
 
         if (install_loaded)
           install_path<pc> (rs, dir_path ("pkgconfig"));
diff --git a/build2/cli/init.cxx b/build2/cli/init.cxx
index 2e553f7..3aebe8e 100644
--- a/build2/cli/init.cxx
+++ b/build2/cli/init.cxx
@@ -301,7 +301,7 @@ namespace build2
           scope& bs,
           const location& l,
           unique_ptr<module_base>&,
-          bool,
+          bool first,
           bool optional,
           const variable_map& hints)
     {
@@ -334,11 +334,10 @@ namespace build2
 
       // Register target types.
       //
+      if (first)
       {
-        auto& t (bs.target_types);
-
-        t.insert<cli> ();
-        t.insert<cli_cxx> ();
+        rs.insert_target_type<cli> ();
+        rs.insert_target_type<cli_cxx> ();
       }
 
       // Register our rules.
diff --git a/libbuild2/bash/init.cxx b/libbuild2/bash/init.cxx
index bd0e34b..65d9d0f 100644
--- a/libbuild2/bash/init.cxx
+++ b/libbuild2/bash/init.cxx
@@ -28,7 +28,7 @@ namespace build2
           scope& bs,
           const location& l,
           unique_ptr<module_base>&,
-          bool,
+          bool first,
           bool,
           const variable_map&)
     {
@@ -44,7 +44,8 @@ namespace build2
 
       // Register target types and configure default installability.
       //
-      bs.target_types.insert<bash> ();
+      if (first)
+        rs.insert_target_type<bash> ();
 
       if (install_loaded)
       {
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index 687e9aa..db46319 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -45,9 +45,11 @@ namespace build2
     target_set targets;
     variable_pool var_pool;
     variable_overrides var_overrides;
-    variable_override_cache global_override_cache;
     function_map functions;
 
+    target_type_map global_target_types;
+    variable_override_cache global_override_cache;
+
     data (context& c): scopes (c), targets (c), var_pool (&c /* global */) {}
   };
 
@@ -59,12 +61,13 @@ namespace build2
         keep_going (kg),
         phase_mutex (*this),
         scopes (data_->scopes),
-        global_scope (create_global_scope (data_->scopes)),
         targets (data_->targets),
         var_pool (data_->var_pool),
         var_overrides (data_->var_overrides),
-        global_override_cache (data_->global_override_cache),
-        functions (data_->functions)
+        functions (data_->functions),
+        global_scope (create_global_scope (data_->scopes)),
+        global_target_types (data_->global_target_types),
+        global_override_cache (data_->global_override_cache)
   {
     tracer trace ("context");
 
@@ -217,7 +220,7 @@ namespace build2
     // Register builtin target types.
     //
     {
-      target_type_map& t (gs.target_types);
+      target_type_map& t (data_->global_target_types);
 
       t.insert<file>  ();
       t.insert<alias> ();
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 243ad2f..f9432ae 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -25,6 +25,7 @@ namespace build2
   class scope;
   class scope_map;
   class target_set;
+  class target_type_map;
 
   class value;
   using values = small_vector<value, 1>;
@@ -274,16 +275,17 @@ namespace build2
     // Build state (scopes, targets, variables, etc).
     //
     const scope_map& scopes;
-    const scope& global_scope;
-
     target_set& targets;
-
     const variable_pool& var_pool;
     const variable_overrides& var_overrides; // Project and relative scope.
-    variable_override_cache& global_override_cache;
-
     function_map& functions;
 
+    // Global scope.
+    //
+    const scope& global_scope;
+    const target_type_map& global_target_types;
+    variable_override_cache& global_override_cache;
+
     // Cached variables.
     //
 
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index 7520af2..4eaf854 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -443,8 +443,8 @@ namespace build2
     assert (altn && root.root_extra == nullptr);
     bool a (*altn);
 
-    root.root_extra = unique_ptr<scope::root_data> (
-      new scope::root_data {
+    root.root_extra.reset (
+      new scope::root_extra_type {
         a,
         a ? alt_build_ext        : std_build_ext,
         a ? alt_build_dir        : std_build_dir,
@@ -460,7 +460,8 @@ namespace build2
         {}, /* meta_operations */
         {}, /* operations */
         {}, /* modules */
-        {}  /* override_cache */});
+        {}, /* override_cache */
+        {}} /* target_types */);
 
     // Enter built-in meta-operation and operation names. Loading of
     // modules (via the src bootstrap; see below) can result in
diff --git a/libbuild2/in/init.cxx b/libbuild2/in/init.cxx
index 8b27336..ece1bcf 100644
--- a/libbuild2/in/init.cxx
+++ b/libbuild2/in/init.cxx
@@ -65,7 +65,7 @@ namespace build2
 
       // Register target types.
       //
-      rs.target_types.insert<in> ();
+      rs.insert_target_type<in> ();
 
       return true;
     }
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index c1787d6..2af5762 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -1982,7 +1982,7 @@ namespace build2
       if (bt == nullptr)
         fail (t) << "unknown target type " << bn;
 
-      if (!scope_->derive_target_type (move (dn), *bt).second)
+      if (!root_->derive_target_type (move (dn), *bt).second)
         fail (dnl) << "target type " << dn << " already define in this scope";
 
       next (t, tt); // Get newline.
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index 6e00511..8d5e37a 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -589,48 +589,33 @@ namespace build2
   }
 
   const target_type* scope::
-  find_target_type (const string& tt, const scope** rs) const
+  find_target_type (const string& tt) const
   {
-    // Search scopes outwards, stopping at the project root.
+    // Search the project's root scope then the global scope.
     //
-    for (const scope* s (this);
-         s != nullptr;
-         s = s->root () ? &s->global_scope () : s->parent_scope ())
+    if (const scope* rs = root_scope ())
     {
-      if (s->target_types.empty ())
-        continue;
-
-      if (const target_type* r = s->target_types.find (tt))
-      {
-        if (rs != nullptr)
-          *rs = s;
-
+      if (const target_type* r = rs->root_extra->target_types.find (tt))
         return r;
-      }
     }
 
-    return nullptr;
+    return ctx.global_target_types.find (tt);
   }
 
   // Find target type from file name.
   //
   static const target_type*
-  find_file_target_type (const scope* s, const string& n)
+  find_target_type_file (const scope& s, const string& n)
   {
     // Pretty much the same logic as in find_target_type() above.
     //
-    for (;
-         s != nullptr;
-         s = s->root () ? &s->global_scope () : s->parent_scope ())
+    if (const scope* rs = s.root_scope ())
     {
-      if (s->target_types.empty ())
-        continue;
-
-      if (const target_type* r = s->target_types.find_file (n))
+      if (const target_type* r = rs->root_extra->target_types.find_file (n))
         return r;
     }
 
-    return nullptr;
+    return s.ctx.global_target_types.find_file (n);
   }
 
   pair<const target_type*, optional<string>> scope::
@@ -720,7 +705,7 @@ namespace build2
       // We only consider files without extension for file name mapping.
       //
       if (!ext)
-        tt = find_file_target_type (this, v);
+        tt = find_target_type_file (*this, v);
 
       //@@ TODO: derive type from extension.
 
@@ -767,6 +752,8 @@ namespace build2
   pair<reference_wrapper<const target_type>, bool> scope::
   derive_target_type (const string& name, const target_type& base)
   {
+    assert (root_scope () == this);
+
     // Base target type uses extensions.
     //
     bool ext (base.fixed_extension   != nullptr ||
@@ -818,7 +805,7 @@ namespace build2
       ? &target_print_0_ext_verb  // Fixed extension, no use printing.
       : nullptr;                  // Normal.
 
-    return target_types.insert (name, move (dt));
+    return root_extra->target_types.insert (name, move (dt));
   }
 
   // scope_map
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 35f07dd..08eb06b 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -202,11 +202,42 @@ namespace build2
 
     // Target types.
     //
+    // Note that target types are project-wide (even if the module that
+    // registers them is loaded in a base scope). The thinking here is that
+    // having target types only visible in certain scopes of a project just
+    // complicates and confuses things (e.g., you cannot refer to a target
+    // whose buildfile you just included). On the other hand, it feels highly
+    // unlikely that a target type will somehow need to be different for
+    // different parts of the project (unlike, say, a rule).
+    //
+    // The target types are also project-local. This means one has to use
+    // import to refer to targets across projects, even in own subprojects
+    // (because we stop searching at project boundaries).
+    //
+    // See also context::global_target_types.
+    //
   public:
-    target_type_map target_types;
+    const target_type&
+    insert_target_type (const target_type& tt)
+    {
+      return root_extra->target_types.insert (tt);
+    }
+
+    template <typename T>
+    const target_type&
+    insert_target_type ()
+    {
+      return root_extra->target_types.insert<T> ();
+    }
+
+    void
+    insert_target_type_file (const string& n, const target_type& tt)
+    {
+      root_extra->target_types.insert_file (n, tt);
+    }
 
     const target_type*
-    find_target_type (const string&, const scope** = nullptr) const;
+    find_target_type (const string&) const;
 
     // Given a target name, figure out its type, taking into account
     // extensions, special names (e.g., '.' and '..'), or anything else that
@@ -267,7 +298,7 @@ namespace build2
     // Extra root scope-only data.
     //
   public:
-    struct root_data
+    struct root_extra_type
     {
       bool altn; // True if using alternative build file/directory naming.
 
@@ -292,16 +323,20 @@ namespace build2
       build2::meta_operations meta_operations;
       build2::operations operations;
 
-      // Modules.
+      // Modules loaded by this project.
       //
       module_map modules;
 
       // Variable override cache.
       //
       mutable variable_override_cache override_cache;
+
+      // Target types.
+      //
+      target_type_map target_types;
     };
 
-    unique_ptr<root_data> root_extra;
+    unique_ptr<root_extra_type> root_extra;
 
     void
     insert_operation (operation_id id, const operation_info& in)
@@ -381,7 +416,7 @@ namespace build2
   project (const scope& root);
 
   // Temporary scope. The idea is to be able to create a temporary scope in
-  // order not to change the variables in the current scope.  Such a scope is
+  // order not to change the variables in the current scope. Such a scope is
   // not entered in to the scope map. As a result it can only be used as a
   // temporary set of variables. In particular, defining targets directly in
   // such a scope will surely end up badly. Defining any nested scopes will be
diff --git a/libbuild2/test/init.cxx b/libbuild2/test/init.cxx
index 3fb4df6..923f047 100644
--- a/libbuild2/test/init.cxx
+++ b/libbuild2/test/init.cxx
@@ -202,10 +202,8 @@ namespace build2
       // Register target types.
       //
       {
-        auto& t (rs.target_types);
-
-        auto& tt (t.insert<testscript> ());
-        t.insert_file ("testscript", tt);
+        auto& tt (rs.insert_target_type<testscript> ());
+        rs.insert_target_type_file ("testscript", tt);
       }
 
       // Register our test running rule.
-- 
cgit v1.1