From 52296a1e3e57bdfa80003f37b63f4f263734e2c0 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 5 Sep 2022 13:33:44 +0200 Subject: Allow empty prerequisites in prerequisite-specific variable assignment/block The old semantics was not very consistent, consider: exe{foo}: cxx{$empty} # Ok. exe{foo}: cxx{$empty}: include = false # Not ok? So now both are ok, as well as the block variant: exe{foo}: cxx{$empty}: { include = false } Note that the empty prerequisite list in the dependency chain is still an error: ./: exe{$empty}: cxx{foo} # Error. Note also that the syntactically-empty prerequisite list was and still is an error: exe{foo}: : include = false # Error. --- libbuild2/parser.cxx | 79 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 27 deletions(-) (limited to 'libbuild2') diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 00366e6..d288bf2 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -2590,21 +2590,6 @@ namespace build2 return; } - // What should we do if there are no prerequisites (for example, because - // of an empty wildcard result)? We can fail or we can ignore. In most - // cases, however, this is probably an error (for example, forgetting to - // checkout a git submodule) so let's not confuse the user and fail (one - // can always handle the optional prerequisites case with a variable and - // an if). - // - if (pns.empty ()) - fail (ploc) << "no prerequisites in dependency chain or prerequisite-" - << "specific variable assignment"; - - next_with_attributes (t, tt); // Recognize attributes after `:`. - - auto at (attributes_push (t, tt)); - // If we are here, then this can be one of three things: // // 1. A prerequisite-specific variable bloc: @@ -2618,10 +2603,37 @@ namespace build2 // // foo: bar: x = y // - // 3. A further dependency chain : + // 3. A further dependency chain: // // foo: bar: baz ... // + // What should we do if there are no prerequisites, for example, because + // of an empty wildcard result or empty variable expansion? We can fail or + // we can ignore. In most cases, however, this is probably an error (for + // example, forgetting to checkout a git submodule) so let's not confuse + // the user and fail (one can always handle the optional prerequisites + // case with a variable and an if). + // + // On the other hand, we allow just empty prerequisites (which is also the + // more common case by far) and so it's strange that we don't allow the + // same with, say, `include = false`: + // + // exe{foo}: cxx{$empty} # Ok. + // exe{foo}: cxx{$empty}: include = false # Not Ok? + // + // So let's ignore in the first two cases (variable block and assignment) + // for consistency. The dependency chain is iffy both conceptually and + // implementation-wise (it could be followed by a variable block). So + // let's keep it an error for now. + // + // Note that the syntactically-empty prerequisite list is still an error: + // + // exe{foo}: : include = false # Error. + // + next_with_attributes (t, tt); // Recognize attributes after `:`. + + auto at (attributes_push (t, tt)); + if (tt == type::newline || tt == type::eos) { attributes_pop (); // Must be none since can't be standalone. @@ -2636,15 +2648,22 @@ namespace build2 // Parse the block for each prerequisites of each target. // - for_each_p ([this] (token& t, token_type& tt) - { - next (t, tt); // First token inside the block. + if (!pns.empty ()) + for_each_p ([this] (token& t, token_type& tt) + { + next (t, tt); // First token inside the block. - parse_variable_block (t, tt); + parse_variable_block (t, tt); - if (tt != type::rcbrace) - fail (t) << "expected '}' instead of " << t; - }); + if (tt != type::rcbrace) + fail (t) << "expected '}' instead of " << t; + }); + else + { + skip_block (t, tt); + if (tt != type::rcbrace) + fail (t) << "expected '}' instead of " << t; + } next (t, tt); // Presumably newline after '}'. next_after_newline (t, tt, '}'); // Should be on its own line. @@ -2667,10 +2686,13 @@ namespace build2 // Parse the assignment for each prerequisites of each target. // - for_each_p ([this, &var, at] (token& t, token_type& tt) - { - parse_variable (t, tt, var, at); - }); + if (!pns.empty ()) + for_each_p ([this, &var, at] (token& t, token_type& tt) + { + parse_variable (t, tt, var, at); + }); + else + skip_line (t, tt); next_after_newline (t, tt); @@ -2689,6 +2711,9 @@ namespace build2 // else { + if (pns.empty ()) + fail (ploc) << "no prerequisites in dependency chain"; + // @@ This is actually ambiguous: prerequisite or target attributes // (or both or neither)? Perhaps this should be prerequisites for // the same reason as below (these are prerequsites first). -- cgit v1.1