Switch to using std::function for target::data_pad

10 files changed, 98 insertions, 52 deletions

diff --git a/libbuild2/adhoc-rule-regex-pattern.cxx b/libbuild2/adhoc-rule-regex-pattern.cxx
index 637a16f..3bd3bb1 100644
--- a/libbuild2/adhoc-rule-regex-pattern.cxx
+++ b/libbuild2/adhoc-rule-regex-pattern.cxx
@@ -272,10 +272,7 @@ namespace build2
       return false;
     }
 
-    static_assert (sizeof (regex_match_results) <= target::data_size,
-                   "insufficient space");
     t.data (move (mr));
-
     return true;
   }
 
diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx
index db3cc3e..fec85be 100644
--- a/libbuild2/bash/rule.cxx
+++ b/libbuild2/bash/rule.cxx
@@ -35,8 +35,8 @@ namespace build2
       optional<bool> for_install;
     };
 
-    static_assert (sizeof (match_data) <= target::data_size,
-                   "insufficient space");
+    static_assert (sizeof (match_data) <= target::small_data_size,
+                   "match data requires dynamic allocation");
 
     // in_rule
     //
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index c72c94c..f680001 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -252,9 +252,6 @@ namespace build2
         : common (move (d)),
           rule_id (string (x) += ".compile 6")
     {
-      static_assert (sizeof (match_data) <= target::data_size,
-                     "insufficient space");
-
       // Locate the header cache (see enter_header() for details).
       //
       {
diff --git a/libbuild2/cc/install-rule.cxx b/libbuild2/cc/install-rule.cxx
index e8c87ae..d83bf70 100644
--- a/libbuild2/cc/install-rule.cxx
+++ b/libbuild2/cc/install-rule.cxx
@@ -192,9 +192,6 @@ namespace build2
         // storage if we are un/installing (used in the *_extra() functions
         // below).
         //
-        static_assert (sizeof (link_rule::libs_paths) <= target::data_size,
-                       "insufficient space");
-
         if (file* f = t.is_a<libs> ())
         {
           if (!f->path ().empty ()) // Not binless.
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 5462371..f2e3775 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -256,8 +256,6 @@ namespace build2
         : common (move (d)),
           rule_id (string (x) += ".link 3")
     {
-      static_assert (sizeof (match_data) <= target::data_size,
-                     "insufficient space");
     }
 
     link_rule::match_result link_rule::
@@ -877,7 +875,8 @@ namespace build2
       // Note that for_install is signalled by install_rule and therefore
       // can only be relied upon during execute.
       //
-      match_data& md (t.data (match_data ()));
+      t.data (match_data ());
+      match_data& md (t.data<match_data> ());
 
       const scope& bs (t.base_scope ());
       const scope& rs (*bs.root_scope ());
diff --git a/libbuild2/config/functions.cxx b/libbuild2/config/functions.cxx
index 398512c..3d35a01 100644
--- a/libbuild2/config/functions.cxx
+++ b/libbuild2/config/functions.cxx
@@ -40,7 +40,10 @@ namespace build2
         if (s == nullptr)
           fail << "config.save() called out of project" << endf;
 
-        module* mod (s->find_module<module> (module::name));
+        // See save_config() for details.
+        //
+        assert (s->ctx.phase == run_phase::load);
+        module* mod (s->rw ().find_module<module> (module::name));
 
         if (mod == nullptr)
           fail << "config.save() called without config module";
diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx
index 98ad9d0..ed98f90 100644
--- a/libbuild2/config/operation.cxx
+++ b/libbuild2/config/operation.cxx
@@ -972,7 +972,7 @@ namespace build2
           configure_project (a,
                              rs->rw (),
                              c_s,
-                             *rs->find_module<module> (module::name),
+                             *rs->rw ().find_module<module> (module::name),
                              projects);
         }
       }
@@ -1053,7 +1053,7 @@ namespace build2
         }
       }
 
-      if (module* m = rs.find_module<module> (module::name))
+      if (const module* m = rs.find_module<module> (module::name))
       {
         for (auto hook: m->disfigure_pre_)
           r = hook (a, rs) || r;
diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx
index 150a5a1..b8ff57d 100644
--- a/libbuild2/dist/operation.cxx
+++ b/libbuild2/dist/operation.cxx
@@ -483,7 +483,7 @@ namespace build2
 
       // Copy over all the files. Apply post-processing callbacks.
       //
-      module& mod (*rs.find_module<module> (module::name));
+      const module& mod (*rs.find_module<module> (module::name));
 
       prog = prog && show_progress (1 /* max_verb */);
       size_t prog_percent (0);
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 9dc2be6..f21fe01 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -566,6 +566,13 @@ namespace build2
 
     template <typename T>
     T*
+    find_module (const string& name)
+    {
+      return root_extra->modules.find_module<T> (name);
+    }
+
+    template <typename T>
+    const T*
     find_module (const string& name) const
     {
       return root_extra->modules.find_module<T> (name);
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index 1562746..e3a64ca 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -789,65 +789,111 @@ namespace build2
     //
     // A rule that matches (i.e., returns true from its match() function) may
     // use this pad to pass data between its match and apply functions as well
-    // as the recipe. After the recipe is executed, the data is destroyed by
-    // calling data_dtor (if not NULL). The rule should static assert that the
-    // size of the pad is sufficient for its needs.
+    // as the recipe. After the recipe is executed, the data is destroyed. The
+    // rule may static assert that the small size of the pad (which doesn't
+    // require dynamic memory allocation) is sufficient for its needs.
     //
     // Note also that normally at least 2 extra pointers may be stored without
     // a dynamic allocation in the returned recipe (small object optimization
     // in std::function). So if you need to pass data only between apply() and
-    // the recipe, then this might be a more convenient way.
+    // the recipe, then this might be a more convenient way. @@ TMP
     //
     // Note also that a rule that delegates to another rule may not be able to
     // use this mechanism fully since the delegated-to rule may also need the
     // data pad.
     //
-    // Currenly the data is not destroyed until the next match.
+    // The data is not destroyed until the next match, which is relied upon to
+    // communicate between rules for inner/outer operations. @@ TMP
     //
     // Note that the recipe may modify the data. Currently reserved for the
-    // inner part of the action.
+    // inner part of the action. @@ TMP
+    //
+    // See also match_extra::buffer.
+
+    // Provide the small object optimization size for the common compilers
+    // (see recipe.hxx for details) in case a rule wants to make sure its data
+    // won't require a dynamic memory allocation. Note that using a minimum
+    // generally available (2 pointers) is not always possible because the
+    // data size may depend on sizes of other compiler-specific types (e.g.,
+    // std::string).
+    //
+    static constexpr size_t small_data_size =
+#if defined(__GLIBCXX__)
+      sizeof (void*) * 2
+#elif defined(_LIBCPP_VERSION)
+      sizeof (void*) * 3
+#elif defined(_MSC_VER)
+      sizeof (void*) * 6
+#else
+      // Assume at least 2 pointers.
+      //
+      sizeof (void*) * 2
+#endif
+      ;
+
+    mutable recipe data_pad;
+
+    template <typename T>
+    struct data_wrapper
+    {
+      T d;
+
+      target_state
+      operator() (action, const target&) const // Never called.
+      {
+        return target_state::unknown;
+      }
+    };
+
+    // Avoid wrapping the data if it is already a recipe.
+    //
+    // Note that this techniques requires a fix for LWG issue 2132 (which all
+    // our minimum supported compiler versions appear to have).
     //
-    static constexpr size_t data_size = sizeof (string) * 16;
-    mutable std::aligned_storage<data_size>::type data_pad;
+    template <typename T>
+    struct data_invocable: std::is_constructible<
+      std::function<recipe_function>,
+      std::reference_wrapper<typename std::remove_reference<T>::type>> {};
 
-    mutable void (*data_dtor) (void*)                = nullptr;
+    template <typename T>
+    typename std::enable_if<!data_invocable<T>::value, void>::type
+    data (T&& d) const
+    {
+      using V = typename std::remove_cv<
+        typename std::remove_reference<T>::type>::type;
 
-    template <typename R,
-              typename T = typename std::remove_cv<
-                typename std::remove_reference<R>::type>::type>
-    typename std::enable_if<std::is_trivially_destructible<T>::value,T&>::type
-    data (R&& d) const
+      data_pad = data_wrapper<V> {forward<T> (d)};
+    }
+
+    template <typename T>
+    typename std::enable_if<!data_invocable<T>::value, T&>::type&
+    data () const
     {
-      assert (sizeof (T) <= data_size);
-      clear_data ();
-      return *new (&data_pad) T (forward<R> (d));
+      using V = typename std::remove_cv<T>::type;
+      return data_pad.target<data_wrapper<V>> ()->d;
     }
 
-    template <typename R,
-              typename T = typename std::remove_cv<
-                typename std::remove_reference<R>::type>::type>
-    typename std::enable_if<!std::is_trivially_destructible<T>::value,T&>::type
-    data (R&& d) const
+    // Note that in this case we don't strip const (the expectation is that we
+    // move the recipe in/out of data).
+    //
+    template <typename T>
+    typename std::enable_if<data_invocable<T>::value, void>::type
+    data (T&& d) const
     {
-      assert (sizeof (T) <= data_size);
-      clear_data ();
-      T& r (*new (&data_pad) T (forward<R> (d)));
-      data_dtor = [] (void* p) {static_cast<T*> (p)->~T ();};
-      return r;
+      data_pad = forward<T> (d);
     }
 
     template <typename T>
-    T&
-    data () const {return *reinterpret_cast<T*> (&data_pad);}
+    typename std::enable_if<data_invocable<T>::value, T&>::type&
+    data () const
+    {
+      return *data_pad.target<T> ();
+    }
 
     void
     clear_data () const
     {
-      if (data_dtor != nullptr)
-      {
-        data_dtor (&data_pad);
-        data_dtor = nullptr;
-      }
+      data_pad = nullptr;
     }
 
     // Target type info and casting.