From 333b13f697dfe04d5f2515c03baa1c4f5eccbe48 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 18 May 2020 12:08:33 +0200 Subject: Make build script variable pool local to execution environment --- libbuild2/build/script/parser.cxx | 13 +++++------ libbuild2/build/script/script.cxx | 49 ++++++++------------------------------- libbuild2/build/script/script.hxx | 24 +++++++------------ libbuild2/script/parser.cxx | 24 ++++++++++++++++--- libbuild2/script/parser.hxx | 8 ++++++- 5 files changed, 53 insertions(+), 65 deletions(-) diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index 4bdf5c7..d26b155 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -95,14 +95,12 @@ namespace build2 // Check if we are trying to modify any of the special variables // ($>). // - string& n (t.value); + if (t.value == ">") + fail (t) << "attempt to set '" << t.value << "' variable"; - if (n == ">") - fail (t) << "attempt to set '" << n << "' variable"; - - // Pre-enter the variable. + // We don't pre-enter variables. // - ln.var = &script_->var_pool.insert (move (n)); + ln.var = nullptr; next (t, tt); // Assignment kind. @@ -317,7 +315,8 @@ namespace build2 exec_lines (s.lines.begin (), s.lines.end (), exec_set, exec_cmd, exec_if, - li); + li, + &environment_->var_pool); runner_->leave (*environment_, s.end_loc); } diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index 26fe006..eb5b78c 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -15,14 +15,6 @@ namespace build2 { namespace script { - // script - // - script:: - script () - : primary_target_var (var_pool.insert (">")) - { - } - // environment // static const string wd_name ("current directory"); @@ -35,13 +27,13 @@ namespace build2 work, wd_name), script (s), - vars (context, false /* global */), - primary_target (pt) + primary_target (pt), + vars (context, false /* global */) { // Set the $> variable. // { - value& v (assign (s.primary_target_var)); + value& v (assign (var_pool.insert (">"))); if (auto* t = pt.is_a ()) v = t->path (); @@ -53,19 +45,9 @@ namespace build2 void environment:: set_variable (string&& nm, names&& val, const string& attrs) { - // Set the variable value and attributes. Note that we need to aquire - // unique lock before potentially changing the script's variable - // pool. The obtained variable reference can safelly be used with no - // locking as the variable pool is an associative container - // (underneath) and we are only adding new variables into it. + // Set the variable value and attributes. // - ulock ul (script.var_pool_mutex); - - const variable& var ( - const_cast (script).var_pool. - insert (move (nm))); - - ul.unlock (); + const variable& var (var_pool.insert (move (nm))); value& lhs (assign (var)); @@ -100,23 +82,12 @@ namespace build2 lookup environment:: lookup (const string& name) const { - // Every variable that is ever set in a script has been pre-entered - // during pre-parse or introduced with the set builtin during - // execution. Which means that if one is not found in the script pool - // then it can only possibly be set in the buildfile. - // - // Note that we need to acquire the variable pool lock. The pool can - // be changed from multiple threads by the set builtin. The obtained - // variable pointer can safelly be used with no locking as the - // variable pool is an associative container (underneath) and we are - // only adding new variables into it. + // Every variable that is ever set in a script has been added during + // variable line execution or introduced with the set builtin. Which + // means that if one is not found in the environment pool then it can + // only possibly be set in the buildfile. // - const variable* pvar (nullptr); - - slock sl (script.var_pool_mutex); - pvar = script.var_pool.find (name); - sl.unlock (); - + const variable* pvar (var_pool.find (name)); return pvar != nullptr ? lookup (*pvar) : lookup_in_buildfile (name); } diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx index 18ed4c4..6beb5a4 100644 --- a/libbuild2/build/script/script.hxx +++ b/libbuild2/build/script/script.hxx @@ -23,27 +23,18 @@ namespace build2 using build2::script::command_expr; // Once parsed, the script can be executed in multiple threads with the - // state (variable values, etc) maintained by the environment object. + // state (variable values, etc) maintained in the environment object. // class script { public: + // Note that the variables are not pre-entered into a pool during the + // parsing phase, so the line variable pointers are NULL. + // build2::script::lines lines; - variable_pool var_pool; - mutable shared_mutex var_pool_mutex; - - const variable& primary_target_var; // $> - location start_loc; location end_loc; - - script (); - - script (script&&) = delete; - script (const script&) = delete; - script& operator= (script&&) = delete; - script& operator= (const script&) = delete; }; class environment: public build2::script::environment @@ -58,14 +49,17 @@ namespace build2 public: const build::script::script& script; + const target& primary_target; + + // Script-local variable pool. + // + variable_pool var_pool; // Note that if we pass the variable name as a string, then it will // be looked up in the wrong pool. // variable_map vars; - const target& primary_target; - virtual void set_variable (string&& name, names&&, const string& attrs) override; diff --git a/libbuild2/script/parser.cxx b/libbuild2/script/parser.cxx index 5c7510b..160374d 100644 --- a/libbuild2/script/parser.cxx +++ b/libbuild2/script/parser.cxx @@ -3,6 +3,7 @@ #include +#include #include // exit #include @@ -1716,7 +1717,8 @@ namespace build2 const function& exec_set, const function& exec_cmd, const function& exec_if, - size_t& li) + size_t& li, + variable_pool* var_pool) { try { @@ -1743,7 +1745,20 @@ namespace build2 { case line_type::var: { - exec_set (*ln.var, t, tt, ll); + // Enter the variable into the pool if this is not done during + // the script parsing. Note that in this case the pool is + // expected to be provided. + // + const variable* var (ln.var); + + if (var == nullptr) + { + assert (var_pool != nullptr); + + var = &var_pool->insert (t.value); + } + + exec_set (*var, t, tt, ll); replay_stop (); break; @@ -1861,7 +1876,10 @@ namespace build2 // Next if-else. // lines::const_iterator j (next (i, false, false)); - if (!exec_lines (i + 1, j, exec_set, exec_cmd, exec_if, li)) + if (!exec_lines (i + 1, j, + exec_set, exec_cmd, exec_if, + li, + var_pool)) return false; i = j->type == line_type::cmd_end ? j : next (j, true, true); diff --git a/libbuild2/script/parser.hxx b/libbuild2/script/parser.hxx index 850794e..ecd9f5a 100644 --- a/libbuild2/script/parser.hxx +++ b/libbuild2/script/parser.hxx @@ -146,12 +146,18 @@ namespace build2 size_t li, const location&); + // If a parser implementation doesn't pre-enter variables into a pool + // during the pre-parsing phase, then they are entered during the + // execution phase and so the variable pool must be provided. Note that + // in this case the variable pool insertions are not MT-safe. + // bool exec_lines (lines::const_iterator b, lines::const_iterator e, const function&, const function&, const function&, - size_t& li); + size_t& li, + variable_pool* = nullptr); // Set lexer pointers for both the current and the base classes. // -- cgit v1.1