aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2018-11-01 13:00:16 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2018-11-01 13:00:16 +0200
commiteacf7f7ccd40a56d1fe761d3d30ced6c6acd58da (patch)
tree841a87d57720704654e8e8fa94cecd05c17b6cfa
parent8ba507252cb932023d16e5d4dfef267c039feb78 (diff)
Add support for rule-specific variables, use to fix cc.type data race
-rw-r--r--build2/action.hxx6
-rw-r--r--build2/algorithm.cxx10
-rw-r--r--build2/cc/common.cxx5
-rw-r--r--build2/cc/init.cxx18
-rw-r--r--build2/cc/link-rule.cxx4
-rw-r--r--build2/dump.cxx88
-rw-r--r--build2/dump.hxx8
-rw-r--r--build2/operation.cxx2
-rw-r--r--build2/scope.cxx22
-rw-r--r--build2/scope.hxx3
-rw-r--r--build2/target.cxx27
-rw-r--r--build2/target.hxx102
-rw-r--r--build2/variable.hxx3
-rw-r--r--build2/variable.ixx1
14 files changed, 231 insertions, 68 deletions
diff --git a/build2/action.hxx b/build2/action.hxx
index 8c9b7e5..321d2c1 100644
--- a/build2/action.hxx
+++ b/build2/action.hxx
@@ -114,10 +114,10 @@ namespace build2
template <typename T>
struct action_state
{
- T states[2]; // [0] -- inner, [1] -- outer.
+ T data[2]; // [0] -- inner, [1] -- outer.
- T& operator[] (action a) {return states[a.inner () ? 0 : 1];}
- const T& operator[] (action a) const {return states[a.inner () ? 0 : 1];}
+ T& operator[] (action a) {return data[a.inner () ? 0 : 1];}
+ const T& operator[] (action a) const {return data[a.inner () ? 0 : 1];}
};
// Id constants for build-in and pre-defined meta/operations.
diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index fbeb365..b4f2843 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -434,11 +434,12 @@ namespace build2
// Match.
//
- // Clear the resolved targets list and the data pad before calling
- // match(). The rule is free to modify these in its match()
- // (provided that it matches) in order to, for example, convey some
- // information to apply().
+ // Clear the rule-specific variables, resolved targets list, and the
+ // data pad before calling match(). The rule is free to modify these
+ // in its match() (provided that it matches) in order to, for
+ // example, convey some information to apply().
//
+ s.vars.clear ();
t.prerequisite_targets[a].clear ();
if (a.inner ()) t.clear_data ();
@@ -479,6 +480,7 @@ namespace build2
// As a sanity measure clear the target data since it can be incomplete
// or invalid (mark()/unmark() should give you some ideas).
//
+ s.vars.clear ();
t.prerequisite_targets[a].clear ();
if (a.inner ()) t.clear_data ();
diff --git a/build2/cc/common.cxx b/build2/cc/common.cxx
index ae125a5..a1a17f7 100644
--- a/build2/cc/common.cxx
+++ b/build2/cc/common.cxx
@@ -80,9 +80,10 @@ namespace build2
// See what type of library this is (C, C++, etc). Use it do decide
// which x.libs variable name to use. If it's unknown, then we only
- // look into prerequisites.
+ // look into prerequisites. Note: lookup starting from rule-specific
+ // variables (target should already be matched).
//
- const string* t (cast_null<string> (l.vars[c_type]));
+ const string* t (cast_null<string> (l.state[a][c_type]));
bool impl (proc_impl && proc_impl (l, la));
bool cc (false), same (false);
diff --git a/build2/cc/init.cxx b/build2/cc/init.cxx
index 2f88542..bb0269c 100644
--- a/build2/cc/init.cxx
+++ b/build2/cc/init.cxx
@@ -78,6 +78,8 @@ namespace build2
//
auto& v (var_pool.rw (rs));
+ auto v_t (variable_visibility::target);
+
v.insert<strings> ("config.cc.poptions", true);
v.insert<strings> ("config.cc.coptions", true);
v.insert<strings> ("config.cc.loptions", true);
@@ -106,24 +108,26 @@ namespace build2
v.insert<string> ("cc.stdlib");
// Target type, for example, "C library" or "C++ library". Should be set
- // on the target by the matching rule to the name of the module (e.g.,
- // "c", "cxx"). Currenly only set for libraries and is used to decide
- // which *.libs to use during static linking.
+ // on the target as a rule-specific variable by the matching rule to the
+ // name of the module (e.g., "c", "cxx"). Currenly only set for
+ // libraries and is used to decide which *.libs to use during static
+ // linking.
//
// It can also be the special "cc" value which means a C-common library
- // but specific language is not known. Used in import installed logic.
+ // but specific language is not known. Used in the import installed
+ // logic.
//
- v.insert<string> ("cc.type");
+ v.insert<string> ("cc.type", v_t);
// If set and is true, then this (imported) library has been found in a
// system library search directory.
//
- v.insert<bool> ("cc.system");
+ v.insert<bool> ("cc.system", v_t);
// C++ module name. Should be set on the bmi*{} target by the matching
// rule.
//
- v.insert<string> ("cc.module_name");
+ v.insert<string> ("cc.module_name", v_t);
// Ability to disable using preprocessed output for compilation.
//
diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index 230fc6f..ba6a22d 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -356,10 +356,10 @@ namespace build2
otype ot (lt.type);
linfo li (link_info (bs, ot));
- // Set the library type (C, C++, etc).
+ // Set the library type (C, C++, etc) as rule-specific variable.
//
if (lt.library ())
- t.vars.assign (c_type) = string (x);
+ t.state[a].assign (c_type) = string (x);
bool binless (lt.library ()); // Binary-less until proven otherwise.
diff --git a/build2/dump.cxx b/build2/dump.cxx
index 0c7e839..fe120d0 100644
--- a/build2/dump.cxx
+++ b/build2/dump.cxx
@@ -53,7 +53,7 @@ namespace build2
}
}
- enum class variable_kind {scope, tt_pat, target, prerequisite};
+ enum class variable_kind {scope, tt_pat, target, rule, prerequisite};
static void
dump_variable (ostream& os,
@@ -107,7 +107,10 @@ namespace build2
//
lookup l (
s.find_override (
- var, make_pair (org, 1), k == variable_kind::target).first);
+ var,
+ make_pair (org, 1),
+ k == variable_kind::target || k == variable_kind::rule,
+ k == variable_kind::rule).first);
assert (l.defined ()); // We at least have the original.
@@ -190,7 +193,8 @@ namespace build2
}
static void
- dump_target (ostream& os,
+ dump_target (optional<action> a,
+ ostream& os,
string& ind,
const target& t,
const scope& s,
@@ -213,26 +217,54 @@ namespace build2
os << ind << t << ':';
- // First print target-specific variables, if any.
+ // First print target/rule-specific variables, if any.
//
- if (!t.vars.empty ())
{
- if (rel)
- stream_verb (os, osv); // We want variable values in full.
+ bool tv (!t.vars.empty ());
+ bool rv (a && !t.state[*a].vars.empty ());
- os << endl
- << ind << '{';
- ind += " ";
- dump_variables (os, ind, t.vars, s, variable_kind::target);
- ind.resize (ind.size () - 2);
- os << endl
- << ind << '}';
+ if (tv || rv)
+ {
+ if (rel)
+ stream_verb (os, osv); // We want variable values in full.
+
+ os << endl
+ << ind << '{';
+ ind += " ";
- if (rel)
- stream_verb (os, nsv);
+ if (tv)
+ dump_variables (os, ind, t.vars, s, variable_kind::target);
- os << endl
- << ind << t << ':';
+ if (rv)
+ {
+ // To distinguish target and rule-specific variables, we put the
+ // latter into a nested block.
+ //
+ // @@ Maybe if we also print the rule name, then we could make
+ // the block associated with that?
+
+ if (tv)
+ os << endl;
+
+ os << endl
+ << ind << '{';
+ ind += " ";
+ dump_variables (os, ind, t.state[*a].vars, s, variable_kind::rule);
+ ind.resize (ind.size () - 2);
+ os << endl
+ << ind << '}';
+ }
+
+ ind.resize (ind.size () - 2);
+ os << endl
+ << ind << '}';
+
+ if (rel)
+ stream_verb (os, nsv);
+
+ os << endl
+ << ind << t << ':';
+ }
}
bool used (false); // Target header has been used to display prerequisites.
@@ -318,7 +350,8 @@ namespace build2
}
static void
- dump_scope (ostream& os,
+ dump_scope (optional<action> a,
+ ostream& os,
string& ind,
scope_map::const_iterator& i,
bool rel)
@@ -383,7 +416,7 @@ namespace build2
os << endl; // Extra newline between scope blocks.
os << endl;
- dump_scope (os, ind, i, true /* relative */);
+ dump_scope (a, os, ind, i, true /* relative */);
sb = true;
}
@@ -406,7 +439,7 @@ namespace build2
}
os << endl;
- dump_target (os, ind, t, p, true /* relative */);
+ dump_target (a, os, ind, t, p, true /* relative */);
tb = true;
}
@@ -418,7 +451,7 @@ namespace build2
}
void
- dump ()
+ dump (optional<action> a)
{
auto i (scopes.cbegin ());
assert (&i->second == global_scope);
@@ -428,7 +461,7 @@ namespace build2
//
string ind;
ostream& os (*diag_stream);
- dump_scope (os, ind, i, false /* relative */);
+ dump_scope (a, os, ind, i, false /* relative */);
os << endl;
}
@@ -441,7 +474,7 @@ namespace build2
string ind (cind);
ostream& os (*diag_stream);
- dump_scope (os, ind, i, false /* relative */);
+ dump_scope (nullopt /* action */, os, ind, i, false /* relative */);
os << endl;
}
@@ -450,7 +483,12 @@ namespace build2
{
string ind (cind);
ostream& os (*diag_stream);
- dump_target (os, ind, t, t.base_scope (), false /* relative */);
+ dump_target (nullopt /* action */,
+ os,
+ ind,
+ t,
+ t.base_scope (),
+ false /* relative */);
os << endl;
}
}
diff --git a/build2/dump.hxx b/build2/dump.hxx
index 74ecbe7..e9a3b30 100644
--- a/build2/dump.hxx
+++ b/build2/dump.hxx
@@ -8,15 +8,19 @@
#include <build2/types.hxx>
#include <build2/utility.hxx>
+#include <build2/action.hxx>
+
namespace build2
{
class scope;
class target;
- // Dump the build state to diag_stream.
+ // Dump the build state to diag_stream. If action is specified, then assume
+ // rules have been matched for this action and dump action-specific
+ // information (like rule-specific variables).
//
void
- dump ();
+ dump (optional<action> = nullopt);
void
dump (const scope&, const char* ind = "");
diff --git a/build2/operation.cxx b/build2/operation.cxx
index c42f92a..229ea4e 100644
--- a/build2/operation.cxx
+++ b/build2/operation.cxx
@@ -238,7 +238,7 @@ namespace build2
assert (phase == run_phase::load);
if (verb >= 6)
- dump ();
+ dump (a);
}
void
diff --git a/build2/scope.cxx b/build2/scope.cxx
index bf83830..b9dabe8 100644
--- a/build2/scope.cxx
+++ b/build2/scope.cxx
@@ -178,8 +178,11 @@ namespace build2
pair<lookup, size_t> scope::
find_override (const variable& var,
pair<lookup, size_t> original,
- bool target) const
+ bool target,
+ bool rule) const
{
+ assert (!rule || target); // Rule-specific is target-specific.
+
// Normally there would be no overrides and if there are, there will only
// be a few of them. As a result, here we concentrate on keeping the logic
// as straightforward as possible without trying to optimize anything.
@@ -204,14 +207,15 @@ namespace build2
const variable_map* inner_vars (nullptr);
const scope* inner_proj (nullptr);
- // One special case is if the original is target-specific, which is the
- // most innermost. Or is it innermostest?
+ // One special case is if the original is target/rule-specific, which is
+ // the most innermost. Or is it innermostest?
//
bool targetspec (false);
if (target)
{
- targetspec = orig.defined () && (orig_depth == 1 || orig_depth == 2);
-
+ targetspec = orig.defined () && (orig_depth == 1 ||
+ orig_depth == 2 ||
+ (rule && orig_depth == 3));
if (targetspec)
{
inner_vars = orig.vars;
@@ -354,7 +358,7 @@ namespace build2
size_t stem_depth (0);
const scope* stem_proj (nullptr);
- // Again the special case of a target-specific variable.
+ // Again the special case of a target/rule-specific variable.
//
if (targetspec)
{
@@ -363,7 +367,9 @@ namespace build2
stem_proj = root_scope ();
}
- size_t ovr_depth (target ? 2 : 0); // For implied target-specific lookup.
+ // For implied target/rule-specific lookup.
+ //
+ size_t ovr_depth (target ? (rule ? 3 : 2) : 0);
for (s = this; s != nullptr; s = s->parent_scope ())
{
@@ -462,7 +468,7 @@ namespace build2
const variable_map* vars (stem.vars);
const scope* proj (stem_proj);
- ovr_depth = target ? 2 : 0;
+ ovr_depth = target ? (rule ? 3 : 2) : 0;
for (s = this; s != nullptr; s = s->parent_scope ())
{
diff --git a/build2/scope.hxx b/build2/scope.hxx
index d40d14a..770f320 100644
--- a/build2/scope.hxx
+++ b/build2/scope.hxx
@@ -135,7 +135,8 @@ namespace build2
pair<lookup, size_t>
find_override (const variable&,
pair<lookup, size_t> original,
- bool target = false) const;
+ bool target = false,
+ bool rule = false) const;
// Return a value suitable for assignment (or append if you only want to
// append to the value from this scope). If the value does not exist in
diff --git a/build2/target.cxx b/build2/target.cxx
index 5d11337..20b9fd6 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -192,6 +192,31 @@ namespace build2
return r;
}
+ pair<lookup, size_t> target::opstate::
+ find_original (const variable& var, bool target_only) const
+ {
+ pair<lookup, size_t> r (lookup (), 0);
+
+ ++r.second;
+ {
+ auto p (vars.find (var));
+ if (p.first != nullptr)
+ r.first = lookup (*p.first, p.second, vars);
+ }
+
+ // Delegate to target's find_original().
+ //
+ if (!r.first)
+ {
+ auto p (target_->find_original (var, target_only));
+
+ r.first = move (p.first);
+ r.second = r.first ? r.second + p.second : p.second;
+ }
+
+ return r;
+ }
+
optional<string> target::
split_name (string& v, const location& loc)
{
@@ -366,6 +391,8 @@ namespace build2
{
t->ext_ = &i->first.ext;
t->implied = implied;
+ t->state.data[0].target_ = t;
+ t->state.data[1].target_ = t;
return pair<target&, ulock> (*t, move (ul));
}
diff --git a/build2/target.hxx b/build2/target.hxx
index fb1c68c..80a5d6d 100644
--- a/build2/target.hxx
+++ b/build2/target.hxx
@@ -357,6 +357,8 @@ namespace build2
// Target-specific variables.
//
+ // See also rule-specific variables below.
+ //
public:
variable_map vars;
@@ -460,9 +462,10 @@ namespace build2
// Inner/outer operation state. See operation.hxx for details.
//
- struct opstate
+ class opstate
{
- mutable atomic_count task_count {0}; // Start offset_touched - 1.
+ public:
+ mutable atomic_count task_count = 0; // Start offset_touched - 1.
// Number of direct targets that depend on this target in the current
// operation. It is incremented during match and then decremented during
@@ -470,7 +473,7 @@ namespace build2
// detect the last chance (i.e., last dependent) to execute the command
// (see also the first/last execution modes in <operation.hxx>).
//
- mutable atomic_count dependents {0};
+ mutable atomic_count dependents = 0;
// Matched rule (pointer to hint_rule_map element). Note that in case of
// a direct recipe assignment we may not have a rule (NULL).
@@ -485,6 +488,79 @@ namespace build2
// a rule is matched and recipe applied (see set_recipe()).
//
target_state state;
+
+ // Rule-specific variables.
+ //
+ // The rule (for this action) has to be matched before these variables
+ // can be accessed and only the rule being matched can modify them (so
+ // no iffy modifications of the group's variables by member's rules).
+ //
+ // They are also automatically cleared before another rule is matched,
+ // similar to the data pad. In other words, rule-specific variables are
+ // only valid for this match-execute phase.
+ //
+ variable_map vars;
+
+ // Lookup, continuing in the target-specific variables, etc. Note that
+ // the group's rule-specific variables are not included. If you only
+ // want to lookup in this target, do it on the variable map directly
+ // (and note that there will be no overrides).
+ //
+ lookup
+ operator[] (const variable& var) const
+ {
+ return find (var).first;
+ }
+
+ lookup
+ operator[] (const variable* var) const // For cached variables.
+ {
+ assert (var != nullptr);
+ return operator[] (*var);
+ }
+
+ lookup
+ operator[] (const string& name) const
+ {
+ const variable* var (var_pool.find (name));
+ return var != nullptr ? operator[] (*var) : lookup ();
+ }
+
+ // As above but also return the depth at which the value is found. The
+ // depth is calculated by adding 1 for each test performed. So a value
+ // that is from the rule will have depth 1. That from the target - 2,
+ // and so on, similar to target-specific variables.
+ //
+ pair<lookup, size_t>
+ find (const variable& var) const
+ {
+ auto p (find_original (var));
+ return var.override == nullptr
+ ? p
+ : target_->base_scope ().find_override (var, move (p), true, true);
+ }
+
+ // If target_only is true, then only look in target and its target group
+ // without continuing in scopes.
+ //
+ pair<lookup, size_t>
+ find_original (const variable&, bool target_only = false) const;
+
+ // Return a value suitable for assignment. See target for details.
+ //
+ value&
+ assign (const variable& var) {return vars.assign (var);}
+
+ value&
+ assign (const variable* var) {return vars.assign (var);} // For cached.
+
+ public:
+ opstate (): vars (false /* global */) {}
+
+ private:
+ friend class target_set;
+
+ const target* target_ = nullptr; // Back-pointer, set by target_set.
};
action_state<opstate> state;
@@ -672,21 +748,23 @@ namespace build2
static void
combine_name (string&, const optional<string>&, bool default_extension);
+ // Targets should be created via the targets set below.
+ //
public:
- virtual
- ~target ();
+ target (dir_path d, dir_path o, string n)
+ : dir (move (d)), out (move (o)), name (move (n)),
+ vars (false /* global */) {}
+
+ target (target&&) = delete;
+ target& operator= (target&&) = delete;
target (const target&) = delete;
target& operator= (const target&) = delete;
- // The only way to create a target should be via the targets set below.
- //
- public:
- friend class target_set;
+ virtual
+ ~target ();
- target (dir_path d, dir_path o, string n)
- : dir (move (d)), out (move (o)), name (move (n)),
- vars (false /* global */) {}
+ friend class target_set;
};
// All targets are from the targets set below.
diff --git a/build2/variable.hxx b/build2/variable.hxx
index 7a0e531..7dc3d1a 100644
--- a/build2/variable.hxx
+++ b/build2/variable.hxx
@@ -1351,6 +1351,9 @@ namespace build2
explicit
variable_map (bool global = false): global_ (global) {}
+ void
+ clear () {m_.clear ();}
+
private:
friend class variable_type_map;
diff --git a/build2/variable.ixx b/build2/variable.ixx
index bf5fb92..6a219ce 100644
--- a/build2/variable.ixx
+++ b/build2/variable.ixx
@@ -280,7 +280,6 @@ namespace build2
typify (v, t, var, memory_order_relaxed);
}
-
inline vector_view<const name>
reverse (const value& v, names& storage)
{