diff options
| -rw-r--r-- | build2/functions-process-path.cxx | 10 | ||||
| -rw-r--r-- | build2/parser | 51 | ||||
| -rw-r--r-- | build2/parser.cxx | 132 | ||||
| -rw-r--r-- | build2/test/script/parser.cxx | 2 | ||||
| -rw-r--r-- | build2/variable | 5 | ||||
| -rw-r--r-- | build2/variable.cxx | 73 | ||||
| -rw-r--r-- | tests/buildfile | 2 | ||||
| -rw-r--r-- | tests/expansion/buildfile | 7 | ||||
| -rw-r--r-- | tests/expansion/common.test | 11 | ||||
| -rw-r--r-- | tests/expansion/type.test | 53 | ||||
| -rw-r--r-- | tests/test/script/builtin/touch.test | 2 | ||||
| -rw-r--r-- | tests/variable/dir-path/buildfile | 6 | ||||
| -rw-r--r-- | unit-tests/function/call.test | 4 | ||||
| -rw-r--r-- | unit-tests/function/driver.cxx | 2 |
14 files changed, 265 insertions, 95 deletions
diff --git a/build2/functions-process-path.cxx b/build2/functions-process-path.cxx index 7069c33..a1d6d7c 100644 --- a/build2/functions-process-path.cxx +++ b/build2/functions-process-path.cxx @@ -21,15 +21,5 @@ namespace build2 { return move (p.effect.empty () ? p.recall : p.effect); }; - - //@@ TMP kludge - // - f["effect"] = [](names n) - { - auto p (value_traits<process_path>::convert ( - move (n[0]), n.size () > 1 ? &n[1] : nullptr)); - - return move (p.effect.empty () ? p.recall : p.effect); - }; } } diff --git a/build2/parser b/build2/parser index 9b326b1..e1f6e77 100644 --- a/build2/parser +++ b/build2/parser @@ -110,8 +110,9 @@ namespace build2 value&& rhs, token_type kind); - // Return the value as well as the indication of whether this is a non- - // empty eval context (i.e., '()' potentially with whitespace in between). + // Return the value (can be NULL/typed) as well as the indication of + // whether this is a non-empty eval context (i.e., '()' potentially with + // whitespace in between). // pair<value, bool> parse_eval (token&, token_type&); @@ -187,26 +188,46 @@ namespace build2 return ns; } - // As above but return the result as a value (always untyped) indicating - // if it is NULL. The only way for the value to be NULL is if it is the - // result of a sole, unquoted variable expansion, function call, or - // context evaluation. + // As above but return the result as a value, which can be typed and NULL. // value - parse_names_value (token& t, token_type& tt, - const char* what = "name", - const string* separators = &name_separators) + parse_value (token& t, token_type& tt, + const char* what = "name", + const string* separators = &name_separators) { names ns; - return parse_names ( - t, tt, ns, false, what, separators, 0, nullptr, nullptr, nullptr) - ? value (move (ns)) - : value (nullptr); + pair<bool, const value_type*> p ( + parse_names ( + t, tt, ns, false, what, separators, 0, nullptr, nullptr, nullptr)); + + value r (p.second); // Potentially typed NULL value. + + // This should not fail since we are typing the result of reversal from + // the typed value. + // + if (p.first) // Not NULL. + r.assign (move (ns), nullptr); + + return r; } - // Append names and return true if the parsed value is NOT NULL. + // Append names and return the indication if the parsed value is NOT NULL + // (first) and whether it is typed (second). // - bool + // You may have noticed that what we return here is essentially a value + // and doing it this way (i.e., reversing it to untyped names and + // returning its type so that it can potentially be "typed back") is kind + // of backwards. The reason we are doing it this way is because in many + // places we expect things untyped and if we were to always return a + // (potentially typed) value, then we would have to reverse it in all + // those places. Still it may make sense to look into redesigning the + // whole thing one day. + // + // Currently the only way for the result to be NULL or have a type is if + // it is the result of a sole, unquoted variable expansion, function call, + // or context evaluation. + // + pair<bool, const value_type*> parse_names (token&, token_type&, names&, bool chunk = false, diff --git a/build2/parser.cxx b/build2/parser.cxx index df2d168..5f9850d 100644 --- a/build2/parser.cxx +++ b/build2/parser.cxx @@ -626,7 +626,8 @@ namespace build2 if (lhs.extra != 0 && lhs.type != nullptr) fail (at) << "typed prepend/append to target type/pattern-" - << "specific variable " << var; + << "specific variable " << var << + info << "use quoting to untypify the value"; } } @@ -1138,6 +1139,7 @@ namespace build2 // The rest is a value. Parse it as a variable value to get expansion, // attributes, etc. build2::import() will check the names, if required. // + location l (get_location (t)); value rhs (parse_variable_value (t, tt)); // While it may seem like supporting attributes is a good idea here, @@ -1155,7 +1157,11 @@ namespace build2 attributes_pop (); if (!rhs) - fail (t) << "null value in export"; + fail (l) << "null value in export"; + + if (rhs.type != nullptr) + fail (l) << "typed value in export" << + info << "use quoting to untypify the value"; export_value_ = move (rhs).as<names> (); @@ -1526,7 +1532,7 @@ namespace build2 attributes_push (t, tt, true); return tt != type::newline && tt != type::eos - ? parse_names_value (t, tt) + ? parse_value (t, tt) : value (names ()); } @@ -1643,6 +1649,23 @@ namespace build2 fail (l) << "unexpected value for attribute " << k << ": " << v; } + // If we have both attribute and value types, then they better match. + // + if (rhs.type != nullptr) + { + if (type != nullptr && type != rhs.type) + { + fail (l) << "conflicting attribute type " << type->name + << " and value type " << rhs.type->name << + info << "use quoting to untypify the value"; + } + + // Reduce this to the untyped value case for simplicity. + // + type = rhs.type; + untypify (rhs); + } + // 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 @@ -1720,14 +1743,14 @@ namespace build2 { // Parse the next value (if any) handling its attributes. // - auto parse_value = [&t, &tt, this] () -> value + auto parse = [&t, &tt, this] () -> value { // Parse value attributes if any. Note that it's ok not to have anything // after the attributes, as in, ($foo == [null]), or even ([null]) // auto at (attributes_push (t, tt, true)); - // Note that if names() gets called, it expects to see a name. + // Note that if parse_value() gets called, it expects to see a value. // value r (tt != type::rparen && tt != type::colon && @@ -1737,7 +1760,7 @@ namespace build2 tt != type::less_equal && tt != type::greater && tt != type::greater_equal - ? parse_names_value (t, tt) + ? parse_value (t, tt) : value (names ())); if (pre_parse_) @@ -1756,7 +1779,7 @@ namespace build2 return v; }; - value lhs (parse_value ()); + value lhs (parse ()); // We continue evaluating from left to right until we reach ')', storing // the result in lhs (think 'a == b == false'). @@ -1814,7 +1837,7 @@ namespace build2 // @@ In C++ == and != have lower precedence than <, etc. // next (t, tt); - value rhs (parse_value ()); + value rhs (parse ()); if (pre_parse_) break; @@ -2076,7 +2099,7 @@ namespace build2 const string parser::name_separators ( string (path::traits::directory_separators) + '%'); - bool parser:: + pair<bool, const value_type*> parser:: parse_names (token& t, type& tt, names& ns, bool chunk, @@ -2092,7 +2115,10 @@ namespace build2 tracer trace ("parser::parse_names", &path_); - bool null (false); + // Returned value NULL/type (see below). + // + bool vnull (false); + const value_type* vtype (nullptr); // If pair is not 0, then it is an index + 1 of the first half of // the pair for which we are parsing the second halves, e.g., @@ -2315,11 +2341,11 @@ namespace build2 names lv_storage; names_view lv; - // Check if we should set/propagate NULL. We only do this if this is - // the only expansion, that is, it is the first and the text token is - // not part of the name. + // Check if we should set/propagate value NULL/type. We only do this + // if this is the only expansion, that is, it is the first and the + // text token is not part of the name. // - auto set_null = [first, &tt] () + auto set_value = [first, &tt] () { return first && tt != type::word && @@ -2423,17 +2449,15 @@ namespace build2 args.second ? 1 : 0), loc); - // See if we should propagate the NULL indicator. + // See if we should propagate the value NULL/type. // - if (!result) + if (set_value ()) { - if (set_null ()) - null = true; - - continue; + vnull = result.null; + vtype = result.type; } - if (result.empty ()) + if (!result || result.empty ()) continue; lv = reverse (result, lv_storage); @@ -2448,17 +2472,15 @@ namespace build2 // lookup l (lookup_variable (move (qual), move (name), loc)); - if (!l) + // See if we should propagate the value NULL/type. + // + if (set_value ()) { - // See if we should set the NULL indicator. - // - if (set_null ()) - null = true; - - continue; + vnull = !l; + vtype = l.defined () ? l->type : nullptr; } - if (l->empty ()) + if (!l || l->empty ()) continue; lv = reverse (*l, lv_storage); @@ -2475,17 +2497,15 @@ namespace build2 if (pre_parse_) continue; // As if empty result. - // See if we should propagate the NULL indicator. + // See if we should propagate the value NULL/type. // - if (!result) + if (set_value ()) { - if (set_null ()) - null = true; - - continue; + vnull = result.null; + vtype = result.type; } - if (result.empty ()) + if (!result || result.empty ()) continue; lv = reverse (result, lv_storage); @@ -2678,7 +2698,7 @@ namespace build2 continue; } - // Note: remember to update set_null test if adding new recognized + // Note: remember to update set_value() test if adding new recognized // tokens. if (!first) @@ -2714,7 +2734,7 @@ namespace build2 string ()); } - return !null; + return make_pair (!vnull, vtype); } void parser:: @@ -2791,23 +2811,23 @@ namespace build2 // // Here is the problem: we "overload" '(' and ')' to mean operation - // application rather than the eval context. At the same time we want - // to use names() to parse names, get variable expansion/function calls, - // quoting, etc. We just need to disable the eval context. The way this - // is done has two parts: Firstly, we parse names in chunks and detect - // and handle the opening paren. In other words, a buildspec like - // 'clean (./)' is "chunked" as 'clean', '(', etc. While this is fairly - // straightforward, there is one snag: concatenating eval contexts, as - // in 'clean(./)'. Normally, this will be treated as a single chunk and - // we don't want that. So here comes the trick (or hack, if you like): - // we will make every opening paren token "separated" (i.e., as if it - // was proceeded by a space). This will disable concatenating eval. In - // fact, we will even go a step further and only do this if we are in - // the original value mode. This will allow us to still use eval - // contexts in buildspec, provided that we quote it: '"cle(an)"'. Note - // also that function calls still work as usual: '$filter (clean test)'. - // To disable a function call and make it instead a var that is expanded - // into operation name(s), we can use quoting: '"$ops"(./)'. + // application rather than the eval context. At the same time we want to use + // parse_names() to parse names, get variable expansion/function calls, + // quoting, etc. We just need to disable the eval context. The way this is + // done has two parts: Firstly, we parse names in chunks and detect and + // handle the opening paren. In other words, a buildspec like 'clean (./)' + // is "chunked" as 'clean', '(', etc. While this is fairly straightforward, + // there is one snag: concatenating eval contexts, as in + // 'clean(./)'. Normally, this will be treated as a single chunk and we + // don't want that. So here comes the trick (or hack, if you like): we will + // make every opening paren token "separated" (i.e., as if it was proceeded + // by a space). This will disable concatenating eval. In fact, we will even + // go a step further and only do this if we are in the original value + // mode. This will allow us to still use eval contexts in buildspec, + // provided that we quote it: '"cle(an)"'. Note also that function calls + // still work as usual: '$filter (clean test)'. To disable a function call + // and make it instead a var that is expanded into operation name(s), we can + // use quoting: '"$ops"(./)'. // static void paren_processor (token& t, const lexer& l) @@ -2889,7 +2909,7 @@ namespace build2 // opening paren, then they are operation/meta-operation names. // Otherwise they are targets. // - if (tt == type::lparen) // Peeked into by names(). + if (tt == type::lparen) // Peeked into by parse_names(). { if (ns.empty ()) fail (t) << "operation name expected before '('"; diff --git a/build2/test/script/parser.cxx b/build2/test/script/parser.cxx index 5730647..9afef75 100644 --- a/build2/test/script/parser.cxx +++ b/build2/test/script/parser.cxx @@ -1190,7 +1190,7 @@ namespace build2 attributes_push (t, tt, true); return tt != type::newline && tt != type::semi - ? parse_names_value (t, tt, "variable value", nullptr) + ? parse_value (t, tt, "variable value", nullptr) : value (names ()); } diff --git a/build2/variable b/build2/variable index 4fd802f..dd4f551 100644 --- a/build2/variable +++ b/build2/variable @@ -296,6 +296,11 @@ namespace build2 void typify (value&, const variable&); void typify (value&, const value_type&, const variable*); + // Remove value type from the value reversing it to names. This is similar + // to reverse() below except that it modifies the value itself. + // + void untypify (value&); + // Reverse the value back to names. The value should not be NULL and storage // should be empty. // diff --git a/build2/variable.cxx b/build2/variable.cxx index 39fdb5d..5a52557 100644 --- a/build2/variable.cxx +++ b/build2/variable.cxx @@ -336,6 +336,41 @@ namespace build2 } } + void + untypify (value& v) + { + if (v.type == nullptr) + return; + + if (v.null) + { + v.type = nullptr; + return; + } + + names ns; + names_view nv (v.type->reverse (v, ns)); + + if (nv.empty () || nv.data () == ns.data ()) + { + // If the data is in storage, then we are all set. + // + ns.resize (nv.size ()); // Just to be sure. + } + else + { + // If the data is somewhere in the value itself, then steal it. + // + auto b (const_cast<name*> (nv.data ())); + ns.assign (make_move_iterator (b), + make_move_iterator (b + nv.size ())); + } + + v = nullptr; // Free old data. + v.type = nullptr; // Change type. + v.assign (move (ns), nullptr); // Assign new data. + } + // Throw invalid_argument for an invalid simple value. // [[noreturn]] static void @@ -736,6 +771,34 @@ namespace build2 } void + process_path_assign (value& v, names&& ns, const variable* var) + { + using traits = value_traits<process_path>; + + size_t n (ns.size ()); + + if (n <= 2) + { + try + { + traits::assign ( + v, + (n == 0 + ? process_path () + : traits::convert (move (ns[0]), n == 2 ? &ns[1] : nullptr))); + return; + } + catch (const invalid_argument&) {} // Fall through. + } + + diag_record dr (fail); + dr << "invalid process_path value '" << ns << "'"; + + if (var != nullptr) + dr << " in variable " << var->name; + } + + void process_path_copy_ctor (value& l, const value& r, bool m) { const auto& rhs (r.as<process_path> ()); @@ -798,15 +861,15 @@ namespace build2 { type_name, sizeof (process_path), - nullptr, // No base. + nullptr, // No base. &default_dtor<process_path>, &process_path_copy_ctor, &process_path_copy_assign, - &simple_assign<process_path, true>, // Allow empty values. - nullptr, // Append not supported. - nullptr, // Prepend not supported. + &process_path_assign, + nullptr, // Append not supported. + nullptr, // Prepend not supported. &process_path_reverse, - nullptr, // No cast (cast data_ directly). + nullptr, // No cast (cast data_ directly). &simple_compare<process_path>, &default_empty<process_path> }; diff --git a/tests/buildfile b/tests/buildfile index f38b3ca..405df8f 100644 --- a/tests/buildfile +++ b/tests/buildfile @@ -2,6 +2,6 @@ # copyright : Copyright (c) 2014-2016 Code Synthesis Ltd # license : MIT; see accompanying LICENSE file -d = directive/ function/ test/ +d = directive/ expansion/ function/ test/ ./: $d include $d diff --git a/tests/expansion/buildfile b/tests/expansion/buildfile new file mode 100644 index 0000000..81df59e --- /dev/null +++ b/tests/expansion/buildfile @@ -0,0 +1,7 @@ +# file : tests/expansion/buildfile +# copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +# license : MIT; see accompanying LICENSE file + +./: test{type} file{common.test} + +test{*}: test = $effect($build.path) diff --git a/tests/expansion/common.test b/tests/expansion/common.test new file mode 100644 index 0000000..881c74e --- /dev/null +++ b/tests/expansion/common.test @@ -0,0 +1,11 @@ +# file : tests/expansion/assert.test +# copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +# license : MIT; see accompanying LICENSE file + ++mkdir build ++cat <<EOI >>>build/bootstrap.build +project = test +amalgamation = +EOI + +test.options += -q --buildfile - noop diff --git a/tests/expansion/type.test b/tests/expansion/type.test new file mode 100644 index 0000000..1aae5b6 --- /dev/null +++ b/tests/expansion/type.test @@ -0,0 +1,53 @@ +# file : tests/expansion/type.test +# copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +# license : MIT; see accompanying LICENSE file + +# Test type propagation during expansion. + +.include common.test + +: var +: +$* <<EOI +x = [bool] true +y = \$x +assert \(\$type\(\$y) == bool) +EOI + +: eval +: +$* <<EOI +y = \([bool] true) +assert \(\$type\(\$y) == bool) +EOI + +: func +: +$* <<EOI +y = \$identity\([bool] true) +assert \(\$type\(\$y) == bool) +EOI + +: untypify +: +$* <<EOI +x = [bool] true +y = "\$x" +assert \(\$type\(\$y) == "") +EOI + +: type-conflict +: +$* <'print [bool] ([string] true)' 2>>EOE != 0 +<stdin>:1:7: error: conflicting attribute type bool and value type string + info: use quoting to untypify the value +EOE + +: retypify +: +$* <'print [bool] "([string] true)"' >'true' + +: retypify-name +: Test the "steal" case of untypify() +: +$* <'print [bool] "([name] true)"' >'true' diff --git a/tests/test/script/builtin/touch.test b/tests/test/script/builtin/touch.test index 4d2ff57..ef950ea 100644 --- a/tests/test/script/builtin/touch.test +++ b/tests/test/script/builtin/touch.test @@ -39,7 +39,7 @@ touch '' 2>"touch: invalid path ''" == 1 : : Test touching an existing directory. : -a = [path] $~; +a = $~; a += "a"; mkdir a; touch 2>"touch: '$a' exists and is not a file" a == 1 diff --git a/tests/variable/dir-path/buildfile b/tests/variable/dir-path/buildfile index 5c99f07..c6fdb46 100644 --- a/tests/variable/dir-path/buildfile +++ b/tests/variable/dir-path/buildfile @@ -27,7 +27,7 @@ print [strings] -I $y print -I $y/baz print -z = [strings] $x # Re-typed. +z = [strings] "$x" # Re-typed. print $z print @@ -37,9 +37,9 @@ r = [dir_path] / print $r/foo r += foo -print [strings] $r +print [strings] "$r" r += bar -print [strings] $r +print [strings] "$r" ./: diff --git a/unit-tests/function/call.test b/unit-tests/function/call.test index 003a828..2b91cc3 100644 --- a/unit-tests/function/call.test +++ b/unit-tests/function/call.test @@ -80,11 +80,11 @@ $* <'print $optional(abc)' >'false' : null-true : -$* <'print $null([null])' >'true' +$* <'print $nullable([null])' >'true' : null-false : -$* <'print $null(nonull)' >'false' +$* <'print $nullable(nonull)' >'false' : null-fail : diff --git a/unit-tests/function/driver.cxx b/unit-tests/function/driver.cxx index 6677c24..3dea374 100644 --- a/unit-tests/function/driver.cxx +++ b/unit-tests/function/driver.cxx @@ -33,7 +33,7 @@ namespace build2 f["fail"] = []() {fail << "failed" << endf;}; f["fail_arg"] = [](names a) {return convert<uint64_t> (move (a[0]));}; - f["null"] = [](names* a) {return a == nullptr;}; + f["nullable"] = [](names* a) {return a == nullptr;}; f["optional"] = [](optional<names> a) {return !a;}; f["dummy0"] = []() {return "abc";}; |