diff options
| -rw-r--r-- | libbuild2/adhoc-rule-regex-pattern.cxx | 3 | ||||
| -rw-r--r-- | libbuild2/bash/rule.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/cc/compile-rule.cxx | 3 | ||||
| -rw-r--r-- | libbuild2/cc/install-rule.cxx | 3 | ||||
| -rw-r--r-- | libbuild2/cc/link-rule.cxx | 5 | ||||
| -rw-r--r-- | libbuild2/config/functions.cxx | 5 | ||||
| -rw-r--r-- | libbuild2/config/operation.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/dist/operation.cxx | 2 | ||||
| -rw-r--r-- | libbuild2/scope.hxx | 7 | ||||
| -rw-r--r-- | libbuild2/target.hxx | 114 |
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. |