Allow attributes in if-else, assert directive's conditions

7 files changed, 78 insertions, 26 deletions

diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 1b4bf53..735677e 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -1966,9 +1966,16 @@ namespace build2
           if (tt == type::newline || tt == type::eos)
             fail (t) << "expected " << k << "-expression instead of " << t;
 
-          // Parse as names to get variable expansion, evaluation, etc. Note
-          // that we also expand patterns (could be used in nested contexts,
-          // etc; e.g., "if pattern expansion is empty" condition).
+          // Parse the condition similar to a value on the RHS of an
+          // assignment (expansion, attributes). While at this stage the
+          // attribute's usefulness in this context is not entirely clear, we
+          // allow it for consistency with other similar directives (switch,
+          // for) and also who knows what attributes we will have in the
+          // future (maybe there will be a way to cast 0/[null] to bool, for
+          // example).
+          //
+          // Note also that we expand patterns (they could be used in nested
+          // contexts, etc; e.g., "if pattern expansion is empty" condition).
           //
           const location l (get_location (t));
 
@@ -1978,10 +1985,10 @@ namespace build2
             //
             bool e (
               convert<bool> (
-                parse_value (t, tt,
-                             pattern_mode::expand,
-                             "expression",
-                             nullptr)));
+                parse_value_with_attributes (t, tt,
+                                             pattern_mode::expand,
+                                             "expression",
+                                             nullptr)));
 
             take = (k.back () == '!' ? !e : e);
           }
@@ -2531,9 +2538,9 @@ namespace build2
     bool neg (t.value.back () == '!');
     const location al (get_location (t));
 
-    // Parse the next chunk as names to get variable expansion, evaluation,
-    // etc. Do it in the value mode so that we don't treat ':', etc., as
-    // special.
+    // Parse the next chunk (the condition) similar to a value on the RHS of
+    // an assignment. We allow attributes (which will only apply to the
+    // condition) for the same reason as in if-else (see parse_if_else()).
     //
     mode (lexer_mode::value);
     next (t, tt);
@@ -2546,11 +2553,11 @@ namespace build2
       //
       bool e (
         convert<bool> (
-          parse_value (t, tt,
-                       pattern_mode::expand,
-                       "expression",
-                       nullptr,
-                       true)));
+          parse_value_with_attributes (t, tt,
+                                       pattern_mode::expand,
+                                       "expression",
+                                       nullptr,
+                                       true /* chunk */)));
       e = (neg ? !e : e);
 
       if (e)
@@ -3057,7 +3064,8 @@ namespace build2
   parse_value_with_attributes (token& t, token_type& tt,
                                pattern_mode pmode,
                                const char* what,
-                               const string* separators)
+                               const string* separators,
+                               bool chunk)
   {
     // Parse value attributes if any. Note that it's ok not to have anything
     // after the attributes (think [null]).
@@ -3065,7 +3073,7 @@ namespace build2
     attributes_push (t, tt, true);
 
     value rhs (tt != type::newline && tt != type::eos
-               ? parse_value (t, tt, pmode, what, separators)
+               ? parse_value (t, tt, pmode, what, separators, chunk)
                : value (names ()));
 
     if (pre_parse_)
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index 7174b3e..d0e5b57 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -342,7 +342,8 @@ namespace build2
     parse_value_with_attributes (token& t, token_type& tt,
                                  pattern_mode pmode,
                                  const char* what = "name",
-                                 const string* separators = &name_separators);
+                                 const string* separators = &name_separators,
+                                 bool chunk = false);
 
     // Append names and return the indication if the parsed value is not NULL
     // and whether it is typed (and whether it is a pattern if pattern_mode is
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index 3eed61e..3cb3d7f 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -621,7 +621,8 @@ namespace build2
   //template <typename T> T convert (names&&); (declaration causes ambiguity)
 
   // Convert value to T. If value is already of type T, then simply cast it.
-  // Otherwise call convert(names) above.
+  // Otherwise call convert(names) above. If value is NULL, then throw
+  // invalid_argument (with an appropriate message).
   //
   template <typename T> T convert (value&&);
 
diff --git a/libbuild2/variable.txx b/libbuild2/variable.txx
index 9b7490a..92b7169 100644
--- a/libbuild2/variable.txx
+++ b/libbuild2/variable.txx
@@ -60,14 +60,27 @@ namespace build2
   T
   convert (value&& v)
   {
-    if (v.type == nullptr)
-      return convert<T> (move (v).as<names> ());
-    else if (v.type == &value_traits<T>::value_type)
-      return move (v).as<T> ();
+    if (v)
+    {
+      if (v.type == nullptr)
+        return convert<T> (move (v).as<names> ());
+      else if (v.type == &value_traits<T>::value_type)
+        return move (v).as<T> ();
+    }
 
-    throw invalid_argument (
-      string ("invalid ") + value_traits<T>::value_type.name +
-      " value: conversion from " + v.type->name);
+    string m ("invalid ");
+    m += value_traits<T>::value_type.name;
+    m += " value: ";
+
+    if (v)
+    {
+      m += "conversion from ";
+      m += v.type->name;
+    }
+    else
+      m += "null";
+
+    throw invalid_argument (move (m));
   }
 
   template <typename T>
diff --git a/tests/directive/assert.testscript b/tests/directive/assert.testscript
index e3774cf..a050051 100644
--- a/tests/directive/assert.testscript
+++ b/tests/directive/assert.testscript
@@ -26,3 +26,9 @@ EOE
 $* <'assert junk' 2>>EOE != 0
 <stdin>:1:8: error: invalid bool value: 'junk'
 EOE
+
+: null
+:
+$* <'assert [null]' 2>>EOE != 0
+<stdin>:1:8: error: invalid bool value: null
+EOE
diff --git a/tests/if-else/buildfile b/tests/if-else/buildfile
new file mode 100644
index 0000000..9c791a4
--- /dev/null
+++ b/tests/if-else/buildfile
@@ -0,0 +1,5 @@
+# file      : tests/if-else/buildfile
+# copyright : Copyright (c) 2014-2019 Code Synthesis Ltd
+# license   : MIT; see accompanying LICENSE file
+
+./: testscript $b
diff --git a/tests/if-else/testscript b/tests/if-else/testscript
new file mode 100644
index 0000000..afc0302
--- /dev/null
+++ b/tests/if-else/testscript
@@ -0,0 +1,18 @@
+# file      : tests/if-else/testscript
+# copyright : Copyright (c) 2014-2019 Code Synthesis Ltd
+# license   : MIT; see accompanying LICENSE file
+
+# Test if-else.
+
+# @@ TODO: more test from old-tests/
+
+.include ../common.testscript
+
+: null
+:
+$* <<EOI 2>>EOE != 0
+if [null]
+  print t
+EOI
+<stdin>:1:4: error: invalid bool value: null
+EOE