diff options
| -rw-r--r-- | libbuild2/algorithm.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/algorithm.hxx | 8 | ||||
| -rw-r--r-- | libbuild2/target.hxx | 13 |
3 files changed, 19 insertions, 6 deletions
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 0c90642..bc4b835 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -1241,8 +1241,8 @@ namespace build2 // Reapply if any new options. // match_extra& me (s.match_extra); - assert ((me.cur_options & options) != options); // Otherwise no lock. - me.new_options = options; + me.new_options = options & ~me.cur_options; // Clear existing. + assert (me.new_options != 0); // Otherwise should not have locked. // Feels like this can only be a logic bug since to end up with a // subset of options requires a rule (see match_extra for details). diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx index 5ebbae2..19f7db2 100644 --- a/libbuild2/algorithm.hxx +++ b/libbuild2/algorithm.hxx @@ -363,7 +363,7 @@ namespace build2 // to be unchanged after match. If it is unmatch::safe, then unmatch the // target if it is safe (this includes unchanged or if we know that someone // else will execute this target). Return true in first half of the pair if - // unmatch succeeded. Always throw if failed. Note that unmatching doesn't + // unmatch succeeded. Always throw if failed. Note that unmatching may not // play well with options -- if unmatch succeeds, the options that have been // passed to match will not be cleared. // @@ -489,6 +489,12 @@ namespace build2 // of the match_*() functions. Note that natually you cannot rematch a // target that you have unmatched. // + // Note also that there is no way to check if the rematch is unnecessary + // (i.e., because the target is already matched with this option) because + // that would require MT-safety considerations (since there could be a + // concurrent rematch). Instead, you should rematch unconditionally and if + // the option is already present, it will be a cheap noop. + // target_state rematch_sync (action, const target&, uint64_t options, diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 78f3e49..f537d59 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -189,7 +189,7 @@ namespace build2 // options are defined for performing a specific operation on a specific // target type and would normally be part of the target type semantics. // To put it another way, when a rule matches a target of certain type for - // certain operation, there is an expectaion of certain semantics, some + // certain operation, there is an expectation of certain semantics, some // parts of which could be made optional. // // As a concrete example, consider installing libs{}, which traditionally @@ -228,7 +228,14 @@ namespace build2 // match options for update and clean operations is a bad idea (update of // pretty much any target can happen during match as a result of a tool // update while clean might have to be performed during match to provide - // the mirro semantics). + // the mirror semantics). + // + // Note also that with rematches the assumption that in the match phase + // after matching the target we can MT-safely examine its state (such as + // its prerequisite_targets) no longer holds since such state could be + // modified during a rematch. As a result, if the target type specifies + // options for a certain operation, then you should not rely on this + // assumption for targets of this type during this operation. // // A rule that supports match options must also be prepared to handle the // apply() call with new_options set to 0, for example, by using a @@ -260,7 +267,7 @@ namespace build2 uint64_t cur_options; uint64_t new_options; - static const uint64_t all_options = ~uint64_t (0); + static constexpr uint64_t all_options = ~uint64_t (0); // Auxiliary data storage. // |