aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--libbuild2/algorithm.cxx4
-rw-r--r--libbuild2/algorithm.hxx8
-rw-r--r--libbuild2/target.hxx13
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.
//