From d7cb460833e6dde3e3b958b993eee3eee4ae3bf0 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 29 Sep 2022 09:46:58 +0200 Subject: Fix variable append logic in script --- libbuild2/build/script/parser+for.test.testscript | 4 +- libbuild2/build/script/parser.cxx | 37 ++++++++++---- libbuild2/parser.cxx | 12 +++++ libbuild2/parser.hxx | 12 +++++ libbuild2/script/parser.cxx | 62 +++-------------------- libbuild2/script/parser.hxx | 19 ++++--- libbuild2/test/script/parser+for.test.testscript | 4 +- libbuild2/test/script/parser.cxx | 46 +++++++++++++---- 8 files changed, 105 insertions(+), 91 deletions(-) diff --git a/libbuild2/build/script/parser+for.test.testscript b/libbuild2/build/script/parser+for.test.testscript index f4ebf3c..877f958 100644 --- a/libbuild2/build/script/parser+for.test.testscript +++ b/libbuild2/build/script/parser+for.test.testscript @@ -87,9 +87,7 @@ cmd $x end EOI - error: type mismatch in variable x - info: value type is dir_path - info: variable type is string + buildfile:11:1: error: conflicting variable x type string and value type dir_path EOE : defined-var diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index cb0dbbb..8759cdf 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -1305,19 +1305,25 @@ namespace build2 // Note that we rely on "small function object" optimization for the // exec_*() lambdas. // - auto exec_assign = [this] (const variable& var, - value&& val, - type kind, - const location& l) + auto exec_set = [this] (const variable& var, + token& t, build2::script::token_type& tt, + const location&) { + next (t, tt); + type kind (tt); // Assignment kind. + + mode (lexer_mode::variable_line); + value rhs (parse_variable_line (t, tt)); + + assert (tt == type::newline); + + // Assign. + // value& lhs (kind == type::assign ? environment_->assign (var) : environment_->append (var)); - if (kind == type::assign) - lhs = move (val); - else - append_value (&var, lhs, move (val), l); + apply_value_attributes (&var, lhs, move (rhs), kind); }; auto exec_cond = [this] (token& t, build2::script::token_type& tt, @@ -1333,9 +1339,22 @@ namespace build2 return runner_->run_cond (*environment_, ce, ii, li, ll); }; + auto exec_for = [this] (const variable& var, + value&& val, + const location& l) + { + value& lhs (environment_->assign (var)); + + // To match the function semantics also pass the value's type + // attribute, restoring it from RHS. Note that the value can't be + // NULL. + // + apply_value (&var, lhs, move (val), type::assign, l, val.type); + }; + build2::script::parser::exec_lines ( begin, end, - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, nullptr /* iteration_index */, environment_->exec_line, &environment_->var_pool); diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 5dbbfac..1cffb85 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -5001,6 +5001,18 @@ namespace build2 fail (l) << "unexpected value in attribute " << a; } + apply_value (var, v, move (rhs), kind, l, type, null); + } + + void parser:: + apply_value (const variable* var, + value& v, + value&& rhs, + type kind, + const location& l, + const value_type* type, + bool null) + { // When do we set the type and when do we keep the original? This gets // tricky for append/prepend where both values contribute. The guiding // rule here is that if the user specified the type, then they reasonable diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx index 0d1e9e2..02d4f74 100644 --- a/libbuild2/parser.hxx +++ b/libbuild2/parser.hxx @@ -304,6 +304,18 @@ namespace build2 value&& rhs, token_type assign_kind); + void + apply_value (const variable*, // Optional. + value& lhs, + value&& rhs, + token_type assign_kind, + const location&, + // + // Attributes: + // + const value_type* = nullptr, + bool null = false); + // Return the value pack (values can be NULL/typed). Note that for an // empty eval context ('()' potentially with whitespaces in between) the // result is an empty pack, not a pack of one empty. diff --git a/libbuild2/script/parser.cxx b/libbuild2/script/parser.cxx index aa186ff..536821b 100644 --- a/libbuild2/script/parser.cxx +++ b/libbuild2/script/parser.cxx @@ -2035,37 +2035,6 @@ namespace build2 build2::parser::apply_value_attributes (var, lhs, move (rhs), kind); } - void parser:: - append_value (const variable* var, - value& v, - value&& rhs, - const location& l) - { - if (rhs) // Don't append/prepent NULL. - { - // Perform the type conversion (see - // build2::parser::apply_value_attributes() for the approach - // reasoning). - // - if (rhs.type != nullptr) - { - if (!v) - v.type = rhs.type; - else if (v.type == nullptr) - typify (v, *rhs.type, var); - else if (v.type != rhs.type) - fail (l) << "conflicting original value type " << v.type->name - << " and append value type " << rhs.type->name; - - // Reduce this to the untyped value case. - // - untypify (rhs); - } - - v.append (move (rhs).as (), var); - } - } - line_type parser:: pre_parse_line_start (token& t, token_type& tt, lexer_mode stm) { @@ -2122,9 +2091,10 @@ namespace build2 bool parser:: exec_lines (lines::const_iterator i, lines::const_iterator e, - const function& exec_assign, + const function& exec_set, const function& exec_cmd, const function& exec_cond, + const function& exec_for, const iteration_index* ii, size_t& li, variable_pool* var_pool) { @@ -2233,25 +2203,7 @@ namespace build2 var = &var_pool->insert (t.value); } - next (t, tt); - type kind (tt); // Assignment kind. - - assert (kind == type::assign || kind == type::append); - - // Parse the value with the potential attributes. - // - // Note that we don't really need to change the mode since we - // are replaying the tokens. - // - value val; - apply_value_attributes (var, - val, - parse_variable_line (t, tt), - kind); - - assert (tt == type::newline); - - exec_assign (*var, move (val), kind, ll); + exec_set (*var, t, tt, ll); replay_stop (); break; @@ -2312,7 +2264,7 @@ namespace build2 lines::const_iterator j (fcend (i, false, false)); if (!exec_lines (i + 1, j, - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, ii, li, var_pool)) return false; @@ -2364,7 +2316,7 @@ namespace build2 we = fcend (i, true, false); if (!exec_lines (i + 1, we, - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, &wi, li, var_pool)) return false; @@ -2471,7 +2423,7 @@ namespace build2 if (etype != nullptr) typify (v, *etype, var); - exec_assign (*var, move (v), type::assign, ll); + exec_for (*var, move (v), ll); // Find the construct end, if it is not found yet. // @@ -2479,7 +2431,7 @@ namespace build2 fe = fcend (i, true, false); if (!exec_lines (i + 1, fe, - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, &fi, li, var_pool)) return false; diff --git a/libbuild2/script/parser.hxx b/libbuild2/script/parser.hxx index 0f276e1..3a4c46f 100644 --- a/libbuild2/script/parser.hxx +++ b/libbuild2/script/parser.hxx @@ -42,11 +42,6 @@ namespace build2 using build2::parser::apply_value_attributes; - // The variable is optional and is only used for diagnostics. - // - void - append_value (const variable*, value& lhs, value&& rhs, const location&); - // Return true if a command line element needs to be re-lexed. // // Specifically, it needs to be re-lexed if it contains any of the @@ -167,10 +162,9 @@ namespace build2 // builtin). For unsuccessful termination the failed exception is // thrown. // - using exec_assign_function = void (const variable&, - value&&, - token_type kind, - const location&); + using exec_set_function = void (const variable&, + token&, token_type&, + const location&); using exec_cmd_function = void (token&, token_type&, const iteration_index*, size_t li, @@ -181,6 +175,10 @@ namespace build2 const iteration_index*, size_t li, const location&); + using exec_for_function = void (const variable&, + value&&, + 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 @@ -188,9 +186,10 @@ namespace build2 // bool exec_lines (lines::const_iterator b, lines::const_iterator e, - const function&, + const function&, const function&, const function&, + const function&, const iteration_index*, size_t& li, variable_pool* = nullptr); diff --git a/libbuild2/test/script/parser+for.test.testscript b/libbuild2/test/script/parser+for.test.testscript index 92fc714..70c1c89 100644 --- a/libbuild2/test/script/parser+for.test.testscript +++ b/libbuild2/test/script/parser+for.test.testscript @@ -87,9 +87,7 @@ cmd $x end EOI - error: type mismatch in variable x - info: value type is dir_path - info: variable type is string + testscript:1:1: error: conflicting variable x type string and value type dir_path EOE : scope-var diff --git a/libbuild2/test/script/parser.cxx b/libbuild2/test/script/parser.cxx index 0ba5a79..f302aee 100644 --- a/libbuild2/test/script/parser.cxx +++ b/libbuild2/test/script/parser.cxx @@ -1549,19 +1549,27 @@ namespace build2 // Note that we rely on "small function object" optimization for the // exec_*() lambdas. // - auto exec_assign = [this] (const variable& var, - value&& val, - type kind, - const location& l) + auto exec_set = [this] (const variable& var, + token& t, build2::script::token_type& tt, + const location&) { + next (t, tt); + type kind (tt); // Assignment kind. + + // We cannot reuse the value mode (see above for details). + // + mode (lexer_mode::variable_line); + value rhs (parse_variable_line (t, tt)); + + assert (tt == type::newline); + + // Assign. + // value& lhs (kind == type::assign ? scope_->assign (var) : scope_->append (var)); - if (kind == type::assign) - lhs = move (val); - else - append_value (&var, lhs, move (val), l); + apply_value_attributes (&var, lhs, move (rhs), kind); if (script_->test_command_var (var.name)) scope_->reset_special (); @@ -1601,6 +1609,22 @@ namespace build2 return runner_->run_cond (*scope_, ce, ii, li, ll); }; + auto exec_for = [this] (const variable& var, + value&& val, + const location& l) + { + value& lhs (scope_->assign (var)); + + // To match the function semantics also pass the value's type + // attribute, restoring it from RHS. Note that the value can't be + // NULL. + // + apply_value (&var, lhs, move (val), type::assign, l, val.type); + + if (script_->test_command_var (var.name)) + scope_->reset_special (); + }; + size_t li (1); if (test* t = dynamic_cast (scope_)) @@ -1608,7 +1632,7 @@ namespace build2 ct = command_type::test; exec_lines (t->tests_.begin (), t->tests_.end (), - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, nullptr /* iteration_index */, li); } else if (group* g = dynamic_cast (scope_)) @@ -1617,7 +1641,7 @@ namespace build2 bool exec_scope ( exec_lines (g->setup_.begin (), g->setup_.end (), - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, nullptr /* iteration_index */, li)); if (exec_scope) @@ -1788,7 +1812,7 @@ namespace build2 ct = command_type::teardown; exec_lines (g->tdown_.begin (), g->tdown_.end (), - exec_assign, exec_cmd, exec_cond, + exec_set, exec_cmd, exec_cond, exec_for, nullptr /* iteration_index */, li); } else -- cgit v1.1