From 9b138ccbebdcdc6bfdd6f6d52e534ae14df280af Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 20 Apr 2022 10:55:50 +0200 Subject: Replace match_extra::buffer with more general data storage facility --- libbuild2/adhoc-rule-cxx.hxx | 2 +- libbuild2/adhoc-rule-regex-pattern.cxx | 15 ++++++-- libbuild2/adhoc-rule-regex-pattern.hxx | 2 +- libbuild2/rule.hxx | 6 +-- libbuild2/target.hxx | 68 +++++++++++++++++++++++++++++++++- libbuild2/target.ixx | 5 +-- 6 files changed, 84 insertions(+), 14 deletions(-) diff --git a/libbuild2/adhoc-rule-cxx.hxx b/libbuild2/adhoc-rule-cxx.hxx index 29d8aa1..9a17447 100644 --- a/libbuild2/adhoc-rule-cxx.hxx +++ b/libbuild2/adhoc-rule-cxx.hxx @@ -26,7 +26,7 @@ namespace build2 }; // Note that when used as part of a pattern, the implementation cannot use - // the match_extra::buffer nor the target auxilary data storage. + // the match_extra::data() facility nor the target auxiliary data storage. // class LIBBUILD2_SYMEXPORT cxx_rule_v1: public cxx_rule { diff --git a/libbuild2/adhoc-rule-regex-pattern.cxx b/libbuild2/adhoc-rule-regex-pattern.cxx index 98acfb6..59a63bc 100644 --- a/libbuild2/adhoc-rule-regex-pattern.cxx +++ b/libbuild2/adhoc-rule-regex-pattern.cxx @@ -197,11 +197,18 @@ namespace build2 // iterators pointing to the string being matched. Which means this string // must be kept around until we are done with replacing the subsitutions. // In fact, we cannot even move it because this may invalidate the - // iterators (e.g., in case of a small string optimization). So the plan - // is to store the string in match_extra::buffer and regex_match_results - // (which we can move) in the auxiliary data storage. + // iterators (e.g., in case of a small string optimization). We also + // cannot set the data ahead of time because we may not match. Plus, + // resorting to a dynamic memory allocation even if we don't match feels + // heavy-handed. // - string& ns (me.buffer); + // So the plan is to store the string in match_extra::data() and + // regex_match_results (which we can move) in the auxiliary data storage. + // + static_assert (sizeof (string) <= match_extra::data_size, + "match data too large"); + + string& ns (me.data (string ())); auto append_name = [&ns, first = true, diff --git a/libbuild2/adhoc-rule-regex-pattern.hxx b/libbuild2/adhoc-rule-regex-pattern.hxx index e6fb938..597f30d 100644 --- a/libbuild2/adhoc-rule-regex-pattern.hxx +++ b/libbuild2/adhoc-rule-regex-pattern.hxx @@ -14,7 +14,7 @@ namespace build2 { // Ad hoc rule regex pattern. // - // The name signature is stored in match_extra::buffer while the regex + // The name signature string is stored in match_extra::data while the regex // match_results object -- in the target auxiliary data storage. Both must // remain valid until after the apply_*() calls. // diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx index 1429ab2..77c2d2c 100644 --- a/libbuild2/rule.hxx +++ b/libbuild2/rule.hxx @@ -175,9 +175,9 @@ namespace build2 // // Used for both ad hoc pattern rules and ad hoc recipes. For recipes, it's // essentially a rule of one case. Note that when used as part of a pattern, - // the implementation cannot use the match_extra::buffer nor the target - // auxiliary data storage until the pattern's apply_*() calls have been - // made. + // the implementation cannot use the match_extra::data() facility nor the + // target auxiliary data storage until the pattern's apply_*() calls have + // been made. // // Note also that when used as part of a pattern, the rule is also register // for the dist meta-operation (unless there is an explicit recipe for dist) diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 563c264..bf8c4fe 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -145,8 +145,70 @@ namespace build2 // struct match_extra { - bool fallback; // True if matching a fallback rule (see match_rule()). - string buffer; // Auxiliary buffer that's reused during match/apply. + bool fallback; // True if matching a fallback rule (see match_rule()). + + // Auxiliary data storage. + // + // A rule (whether matches or not) may use this pad to pass data between + // its match and apply functions (but not the recipe). The rule should + // static assert that the size of the pad is sufficient for its needs. + // + // This facility is complementary to the auxiliary data storage in target: + // it can store slightly more/extra data without dynamic memory allocation + // but can only be used during match/apply. + // + // 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 storage. + // + static constexpr size_t data_size = (sizeof (string) > sizeof (void*) * 4 + ? sizeof (string) + : sizeof (void*) * 4); + + std::aligned_storage::type data_; + void (*data_dtor_) (void*) = nullptr; + + template ::type>::type> + typename std::enable_if::value,T&>::type + data (R&& d) + { + assert (sizeof (T) <= data_size); + clear_data (); + return *new (&data_) T (forward (d)); + } + + template ::type>::type> + typename std::enable_if::value,T&>::type + data (R&& d) + { + assert (sizeof (T) <= data_size); + clear_data (); + T& r (*new (&data_) T (forward (d))); + data_dtor_ = [] (void* p) {static_cast (p)->~T ();}; + return r; + } + + template + T& + data () {return *reinterpret_cast (&data_);} + + template + const T& + data () const {return *reinterpret_cast (&data_);} + + void + clear_data () + { + if (data_dtor_ != nullptr) + { + data_dtor_ (&data_); + data_dtor_ = nullptr; + } + } // Implementation details. // @@ -840,6 +902,8 @@ namespace build2 // } // }; // + // Note: see also similar facility in match_extra. + // // After the recipe is executed, the recipe/data is destroyed, unless // explicitly requested not to (see below). The rule may static assert // that the small size of the storage (which doesn't require dynamic diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx index bdfa20f..5432f7c 100644 --- a/libbuild2/target.ixx +++ b/libbuild2/target.ixx @@ -138,15 +138,14 @@ namespace build2 inline void match_extra:: init (bool f) { + clear_data (); fallback = f; - buffer.clear (); } inline void match_extra:: free () { - string s; - buffer.swap (s); + clear_data (); } // target -- cgit v1.1