From ae20570f2ad55b2fa8e71cf450457cb9c4b21b1b Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Mon, 18 Apr 2016 10:17:37 +0200
Subject: Add support for using value attributes in eval context

For example:

if ($x == [null])

Or:

if ([uint64] 01 == [uint64] 1)
---
 build2/lexer.cxx                   |   2 +
 build2/parser                      |  78 +++++--
 build2/parser.cxx                  | 445 ++++++++++++++++++++++---------------
 build2/scope.cxx                   |   6 +-
 build2/variable                    |  52 +++--
 build2/variable.cxx                |  58 +++--
 build2/variable.ixx                |  22 +-
 build2/variable.txx                | 107 ++++++---
 tests/eval/buildfile               |  16 ++
 tests/eval/test.out                |   9 +
 tests/variable/override/buildfile  |   2 +-
 tests/variable/qualified/buildfile |   4 +-
 12 files changed, 526 insertions(+), 275 deletions(-)

diff --git a/build2/lexer.cxx b/build2/lexer.cxx
index 53b9e12..2319ecb 100644
--- a/build2/lexer.cxx
+++ b/build2/lexer.cxx
@@ -143,6 +143,8 @@ namespace build2
     case ':': return token (type::colon, sep, ln, cn);
     case '{': return token (type::lcbrace, sep, ln, cn);
     case '}': return token (type::rcbrace, sep, ln, cn);
+    case '[': return token (type::lsbrace, sep, ln, cn);
+    case ']': return token (type::rsbrace, sep, ln, cn);
     case '$': return token (type::dollar, sep, ln, cn);
     case '(': return token (type::lparen, sep, ln, cn);
     case ')':
diff --git a/build2/parser b/build2/parser
index ced81c3..0962348 100644
--- a/build2/parser
+++ b/build2/parser
@@ -5,6 +5,8 @@
 #ifndef BUILD2_PARSER
 #define BUILD2_PARSER
 
+#include <stack>
+
 #include <build2/types>
 #include <build2/utility>
 
@@ -87,31 +89,61 @@ namespace build2
     string
     variable_name (names_type&&, const location&);
 
-    pair<names_type, bool> // See names_null() return type.
+    // Note: calls attributes_push() that the caller must pop.
+    //
+    value
     variable_value (token&, token_type&);
 
     void
     variable_attributes (const variable_type&);
 
     void
-    value_attributes (const variable_type&,
+    value_attributes (const variable_type*, // Optional.
                       value& lhs,
-                      pair<names_type, bool>&& rhs,
+                      value&& rhs,
                       token_type kind);
 
-    pair<names_type, bool> // See names_null() return type.
+    value
     eval (token&, token_type&);
 
-    bool
-    eval_trailer (token&, token_type&, names_type&);
+    value
+    eval_trailer (token&, token_type&);
 
-    // If the next token is '[' parse the attribute sequence until ']' storing
-    // the result in the ha_/as_/al_ members (which are only valid until the
-    // next call). Then get the next token and, if standalone is false, verify
-    // it is not newline/eos (i.e., there is something after it).  Return ha_.
+    // Attributes stack. We can have nested attributes, for example:
     //
-    bool
-    attributes (token&, token_type&, bool standalone = false);
+    // x = [bool] ([uint64] $x == [uint64] $y)
+    //
+    // In this example we only apply the value attributes after evaluating
+    // the context, which has its own attributes.
+    //
+    struct attributes
+    {
+      bool has;                         // Has attributes flag.
+      location loc;                     // Start of attributes location.
+      vector<pair<string, string>> ats; // Attributes.
+
+      explicit operator bool () const {return has;}
+    };
+
+    // Push a new entry into the attributes_ stack. If the next token is '['
+    // parse the attribute sequence until ']' setting the 'has' flag and
+    // storing the result on the stack. Then get the next token and, if
+    // standalone is false, verify it is not newline/eos (i.e., there is
+    // something after it). Return the top of the stack.
+    //
+    attributes&
+    attributes_push (token&, token_type&, bool standalone = false);
+
+    attributes
+    attributes_pop ()
+    {
+      attributes r (move (attributes_.top ()));
+      attributes_.pop ();
+      return r;
+    }
+
+    attributes&
+    attributes_top () {return attributes_.top ();}
 
     // If chunk is true, then parse the smallest but complete, name-wise,
     // chunk of input. Note that in this case you may still end up with
@@ -125,18 +157,22 @@ namespace build2
       return ns;
     }
 
-    // As above but also indicate if the value is NOT 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 (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.
     //
-    pair<names_type, bool>
-    names_null (token& t, token_type& tt)
+    value
+    names_value (token& t, token_type& tt)
     {
       names_type ns;
-      bool n (names (t, tt, ns, false, 0, nullptr, nullptr, nullptr));
-      return make_pair (move (ns), n);
+      return names (t, tt, ns, false, 0, nullptr, nullptr, nullptr)
+        ? value (move (ns))
+        : value (nullptr);
     }
 
+    // Return true if the parsed value is NOT NULL.
+    //
     bool
     names (token& t, token_type& tt, names_type& ns, bool chunk = false)
     {
@@ -321,9 +357,7 @@ namespace build2
     scope* scope_;   // Current base scope (out_base).
     scope* root_;    // Current root scope (out_root).
 
-    bool ha_;                         // Has attributes flag.
-    location al_;                     // Current attributes location.
-    vector<pair<string, string>> as_; // Current attributes.
+    std::stack<attributes> attributes_;
 
     target* default_target_;
     names_type export_value_;
diff --git a/build2/parser.cxx b/build2/parser.cxx
index 2c21de3..80e744e 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -167,12 +167,12 @@ namespace build2
 
     type tt;
     token t (type::eos, false, 0, 0);
-    pair<names_type, bool> p (variable_value (t, tt));
+    value rhs (variable_value (t, tt));
 
-    value v;
-    value_attributes (var, v, move (p), type::assign);
+    value lhs;
+    value_attributes (&var, lhs, move (rhs), type::assign);
 
-    return make_pair (move (v), move (t));
+    return make_pair (move (lhs), move (t));
   }
 
   void parser::
@@ -190,7 +190,8 @@ namespace build2
     {
       // Extract attributes if any.
       //
-      attributes (t, tt);
+      assert (attributes_.empty ());
+      attributes& a (attributes_push (t, tt));
 
       // We always start with one or more names.
       //
@@ -199,7 +200,16 @@ namespace build2
           tt != type::dollar  && // Variable expansion: '$foo ...'
           tt != type::lparen  && // Eval context: '(foo) ...'
           tt != type::colon)     // Empty name: ': ...'
-        break; // Something else. Let our caller handle that.
+      {
+        // Something else. Let our caller handle that.
+        //
+        if (a)
+          fail (a.loc) << "attributes before " << t;
+        else
+          attributes_pop ();
+
+        break;
+      }
 
       // See if this is one of the directives.
       //
@@ -256,8 +266,10 @@ namespace build2
 
         if (f != nullptr)
         {
-          if (ha_)
-            fail (al_) << "attributes before " << n;
+          if (a)
+            fail (a.loc) << "attributes before " << n;
+          else
+            attributes_pop ();
 
           (this->*f) (t, tt);
           continue;
@@ -321,8 +333,10 @@ namespace build2
             {
               // Directory scope.
               //
-              if (ha_)
-                fail (al_) << "attributes before directory scope";
+              if (a)
+                fail (a.loc) << "attributes before directory scope";
+              else
+                attributes_pop ();
 
               // Can contain anything that a top level can.
               //
@@ -331,8 +345,10 @@ namespace build2
             }
             else
             {
-              if (ha_)
-                fail (al_) << "attributes before target scope";
+              if (a)
+                fail (a.loc) << "attributes before target scope";
+              else
+                attributes_pop ();
 
               // @@ TODO: target scope.
             }
@@ -359,13 +375,12 @@ namespace build2
         // assignment.
         //
 
-        // Will have to stash them if later support attributes on
-        // target/scope.
-        //
-        if (ha_)
-          fail (al_) << "attributes before target/scope";
+        if (a)
+          fail (a.loc) << "attributes before target/scope";
+        else
+          attributes_pop ();
 
-        attributes (t, tt);
+        attributes& a (attributes_push (t, tt));
 
         if (tt == type::name    ||
             tt == type::lcbrace ||
@@ -392,8 +407,7 @@ namespace build2
 
             // Handle variable attributes.
             //
-            if (ha_)
-              variable_attributes (var);
+            variable_attributes (var);
 
             // If we have multiple targets/scopes, then we save the value
             // tokens when parsing the first one and then replay them for
@@ -462,10 +476,10 @@ namespace build2
 
                   // Note: expanding the value in the context of the scope.
                   //
-                  pair<names_type, bool> p (variable_value (t, tt));
-                  value& v (scope_->target_vars[*ti][move (n.value)].assign (
-                              var).first);
-                  value_attributes (var, v, move (p), type::assign);
+                  value rhs (variable_value (t, tt));
+                  value& lhs (scope_->target_vars[*ti][move (n.value)].assign (
+                                var).first);
+                  value_attributes (&var, lhs, move (rhs), type::assign);
                 }
               }
 
@@ -476,8 +490,10 @@ namespace build2
           //
           else
           {
-            if (ha_)
-              fail (al_) << "attributes before prerequisites";
+            if (a)
+              fail (a.loc) << "attributes before prerequisites";
+            else
+              attributes_pop ();
 
             // Prepare the prerequisite list.
             //
@@ -549,8 +565,7 @@ namespace build2
 
         // Handle variable attributes.
         //
-        if (ha_)
-          variable_attributes (var);
+        variable_attributes (var);
 
         variable (t, tt, var, tt);
 
@@ -566,8 +581,10 @@ namespace build2
       //
       if (tt == type::newline && ns.empty ())
       {
-        if (ha_)
-          fail (al_) << "standalone attributes";
+        if (a)
+          fail (a.loc) << "standalone attributes";
+        else
+          attributes_pop ();
 
         next (t, tt);
         continue;
@@ -829,7 +846,7 @@ namespace build2
     // Get variable attributes, if any (note that here we will go into a
     // nested pairs mode).
     //
-    attributes (t, tt);
+    attributes& a (attributes_push (t, tt));
 
     if (tt == type::name)
     {
@@ -900,15 +917,19 @@ namespace build2
     {
       // Handle variable attributes.
       //
-      if (ha_)
-        variable_attributes (*var);
+      variable_attributes (*var);
 
       val = at == type::assign
         ? &scope_->assign (*var)
         : &scope_->append (*var);
     }
-    else if (ha_)
-      fail (al_) << "attributes without variable";
+    else
+    {
+      if (a)
+        fail (a.loc) << "attributes without variable";
+      else
+        attributes_pop ();
+    }
 
     // The rest should be a list of projects and/or targets. Parse
     // them as names to get variable expansion and directory prefixes.
@@ -930,11 +951,11 @@ namespace build2
       if (val != nullptr)
       {
         if (at == type::assign)
-          val->assign (move (r), *var);
+          val->assign (move (r), var);
         else if (at == type::prepend)
-          val->prepend (move (r), *var);
+          val->prepend (move (r), var);
         else
-          val->append (move (r), *var);
+          val->append (move (r), var);
       }
     }
 
@@ -959,24 +980,26 @@ 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.
     //
-    pair<names_type, bool> p (variable_value (t, tt));
+    value rhs (variable_value (t, tt));
 
     // While it may seem like supporting attributes is a good idea here,
     // there is actually little benefit in being able to type them or to
     // return NULL.
     //
     // export_value_ = value (); // Reset to untyped NULL value.
-    // value_attributes (var_pool.find ("__export"), // Well, this is a hack.
+    // value_attributes (nullptr,
     //                   export_value_,
-    //                   move (p),
+    //                   move (rhs),
     //                   type::assign);
-    if (ha_)
-      fail (al_) << "attributes in export";
+    if (attributes& a = attributes_top ())
+      fail (a.loc) << "attributes in export";
+    else
+      attributes_pop ();
 
-    if (!p.second)
+    if (rhs.null ())
       fail (t) << "null value in export";
 
-    export_value_ = move (p.first);
+    export_value_ = move (rhs).as<names_type> ();
 
     if (tt == type::newline)
       next (t, tt);
@@ -1253,32 +1276,31 @@ namespace build2
   {
     // Parse the rest as a variable value to get expansion, attributes, etc.
     //
-    pair<names_type, bool> p (variable_value (t, tt));
+    value v (variable_value (t, tt));
 
-    if (ha_)
+    if (attributes_top ())
     {
-      // Round-trip it through value.
+      // Round-trip it through (a potentially typed) value.
       //
-      value v;
-      value_attributes (var_pool.find ("__print"), // Well, this is a hack.
-                        v,
-                        move (p),
-                        type::assign);
+      value tv;
+      value_attributes (nullptr, tv, move (v), type::assign);
 
-      if (v.null ())
+      if (tv.null ())
         cout << "[null]" << endl;
       else
       {
-        p.first.clear ();
-        cout << reverse (v, p.first) << endl;
+        names_type storage;
+        cout << reverse (tv, storage) << endl;
       }
     }
     else
     {
-      if (!p.second)
+      attributes_pop ();
+
+      if (v.null ())
         cout << "[null]" << endl;
       else
-        cout << p.first << endl;
+        cout << v.as<names_type> () << endl;
     }
 
     if (tt != type::eos)
@@ -1305,17 +1327,17 @@ namespace build2
   void parser::
   variable (token& t, type& tt, const variable_type& var, type kind)
   {
-    pair<names_type, bool> p (variable_value (t, tt));
+    value rhs (variable_value (t, tt));
 
-    value& v (
+    value& lhs (
       kind == type::assign
       ? target_ != nullptr ? target_->assign (var) : scope_->assign (var)
       : target_ != nullptr ? target_->append (var) : scope_->append (var));
 
-    value_attributes (var, v, move (p), kind);
+    value_attributes (&var, lhs, move (rhs), kind);
   }
 
-  pair<parser::names_type, bool> parser::
+  value parser::
   variable_value (token& t, type& tt)
   {
     mode (lexer_mode::pairs, '@');
@@ -1324,11 +1346,11 @@ namespace build2
     // Parse value attributes if any. Note that it's ok not to have anything
     // after the attributes (e.g., foo=[null]).
     //
-    attributes (t, tt, true);
+    attributes_push (t, tt, true);
 
     return tt != type::newline && tt != type::eos
-      ? names_null (t, tt)
-      : make_pair (names_type (), true);
+      ? names_value (t, tt)
+      : value (names_type ());
   }
 
   static const value_type*
@@ -1352,9 +1374,15 @@ namespace build2
   void parser::
   variable_attributes (const variable_type& var)
   {
+    attributes a (attributes_pop ());
+
+    if (!a)
+      return;
+
+    const location& l (a.loc);
     const value_type* type (nullptr);
 
-    for (auto& p: as_)
+    for (auto& p: a.ats)
     {
       string& k (p.first);
       string& v (p.second);
@@ -1362,16 +1390,16 @@ namespace build2
       if (const value_type* t = map_type (k))
       {
         if (type != nullptr && t != type)
-          fail (al_) << "multiple variable types: " << k << ", " << type->name;
+          fail (l) << "multiple variable types: " << k << ", " << type->name;
 
         type = t;
         // Fall through.
       }
       else
-        fail (al_) << "unknown variable attribute " << k;
+        fail (l) << "unknown variable attribute " << k;
 
       if (!v.empty ())
-        fail (al_) << "unexpected value for attribute " << k << ": " << v;
+        fail (l) << "unexpected value for attribute " << k << ": " << v;
     }
 
     if (type != nullptr)
@@ -1379,31 +1407,34 @@ namespace build2
       if (var.type == nullptr)
         var.type = type;
       else if (var.type != type)
-        fail (al_) << "changing variable " << var.name << " type from "
-                   << var.type->name << " to " << type->name;
+        fail (l) << "changing variable " << var.name << " type from "
+                 << var.type->name << " to " << type->name;
     }
   }
 
   void parser::
-  value_attributes (const variable_type& var,
+  value_attributes (const variable_type* var,
                     value& v,
-                    pair<names_type, bool>&& rhs,
+                    value&& rhs,
                     token_type kind)
   {
+    attributes a (attributes_pop ());
+    const location& l (a.loc);
+
     // Essentially this is an attribute-augmented assign/append/prepend.
     //
     bool null (false);
     const value_type* type (nullptr);
 
-    for (auto& p: as_)
+    for (auto& p: a.ats)
     {
       string& k (p.first);
       string& v (p.second);
 
       if (k == "null")
       {
-        if (!rhs.first.empty () || !rhs.second)
-          fail (al_) << "value with null attribute";
+        if (!rhs.empty ()) // Note: null means we had an expansion.
+          fail (l) << "value with null attribute";
 
         null = true;
         // Fall through.
@@ -1411,16 +1442,16 @@ namespace build2
       else if (const value_type* t = map_type (k))
       {
         if (type != nullptr && t != type)
-          fail (al_) << "multiple value types: " << k << ", " << type->name;
+          fail (l) << "multiple value types: " << k << ", " << type->name;
 
         type = t;
         // Fall through.
       }
       else
-        fail (al_) << "unknown value attribute " << k;
+        fail (l) << "unknown value attribute " << k;
 
       if (!v.empty ())
-        fail (al_) << "unexpected value for attribute " << k << ": " << v;
+        fail (l) << "unexpected value for attribute " << k << ": " << v;
     }
 
     // When do we set the type and when do we keep the original? This gets
@@ -1435,9 +1466,9 @@ namespace build2
     //
     if (type != nullptr)
     {
-      if (var.type != nullptr && var.type != type)
-        fail (al_) << "confliction variable " << var.name << " type "
-                   << var.type->name << " and value type " << type->name;
+      if (var != nullptr && var->type != nullptr && var->type != type)
+        fail (l) << "confliction variable " << var->name << " type "
+                 << var->type->name << " and value type " << type->name;
 
       if (kind == token_type::assign)
       {
@@ -1454,8 +1485,8 @@ namespace build2
         else if (v.type == nullptr)
           typify (v, *type, var);
         else if (v.type != type)
-          fail (al_) << "confliction original value type " << v.type->name
-                     << " and append/prepend value type " << type->name;
+          fail (l) << "confliction original value type " << v.type->name
+                   << " and append/prepend value type " << type->name;
       }
     }
 
@@ -1465,107 +1496,152 @@ namespace build2
     {
       if (kind == token_type::assign)
       {
-        if (rhs.second)
-          v.assign (move (rhs.first), var);
+        if (rhs)
+          v.assign (move (rhs).as<names_type> (), var);
         else
           v = nullptr;
       }
-      else if (rhs.second) // Don't append/prepent NULL.
+      else if (rhs) // Don't append/prepent NULL.
       {
         if (kind == token_type::prepend)
-          v.prepend (move (rhs.first), var);
+          v.prepend (move (rhs).as<names_type> (), var);
         else
-          v.append (move (rhs.first), var);
+          v.append (move (rhs).as<names_type> (), var);
       }
     }
   }
 
-  pair<parser::names_type, bool> parser::
+  value parser::
   eval (token& t, type& tt)
   {
     mode (lexer_mode::eval);
     next (t, tt);
-
-    names_type ns;
-    bool n (eval_trailer (t, tt, ns));
-    return make_pair (move (ns), n);
+    return eval_trailer (t, tt);
   }
 
-  bool parser::
-  eval_trailer (token& t, type& tt, names_type& ns)
+  value parser::
+  eval_trailer (token& t, type& tt)
   {
-    bool null (false);
-
-    // Note that names() will handle cases like ( == foo) or (:foo) since if
-    // it gets called, it expects to see a name.
+    // Parse the next value (if any) handling its attributes.
     //
-    if (tt != type::rparen)
+    auto parse_value = [&t, &tt, this] () -> value
     {
-      if (!names (t, tt, ns) && tt == type::rparen)
-        null = true;
-    }
+      // Parse value attributes if any. Note that it's ok not to have anything
+      // after the attributes, as in, ($foo == [null]), or even ([null])
+      //
+      attributes& a (attributes_push (t, tt, true));
 
-    switch (tt)
-    {
-    case type::colon:
+      // Note that if names() gets called, it expects to see a name.
+      //
+      value r (tt != type::rparen &&
+               tt != type::colon &&
+               tt != type::equal &&
+               tt != type::not_equal
+               ? names_value (t, tt)
+               : value (names_type ()));
+
+      // Process attributes if any.
+      //
+      if (!a)
       {
-        // Later, when we support '?:', we will need to decide which case this
-        // is. But for now ':' is always a scope/target qualified name which
-        // we represent as a special ':'-style pair.
-        //
-        size_t n (ns.size ());
-        ns.back ().pair = ':';
+        attributes_pop ();
+        return r;
+      }
 
-        next (t, tt);
-        eval_trailer (t, tt, ns);
+      value v;
+      value_attributes (nullptr, v, move (r), type::assign);
+      return v;
+    };
 
-        if (ns.size () == n)
-          fail (t) << "scope/target expected after ':'";
+    value lhs (parse_value ());
 
-        break;
-      }
-    case type::equal:
-    case type::not_equal:
+    // We continue evaluating from left to right until we reach ')', storing
+    // the result in lhs (think 'a == b == false').
+    //
+    while (tt != type::rparen)
+    {
+      const location l (get_location (t, &path_));
+
+      // Remember to update parse_value above if adding any new name
+      // separators here.
+      //
+      switch (tt)
       {
-        type op (tt);
+      case type::colon:
+        {
+          // Later, when we support '?:', we will need to decide which case
+          // this is. But for now ':' is always a scope/target qualified name
+          // which we represent as a special ':'-style pair.
+          //
+          if (lhs.type != nullptr || lhs.null () || lhs.empty ())
+            fail (l) << "scope/target expected before ':'";
 
-        // ==, != are left-associative, so get the rhs name and evaluate.
-        //
-        next (t, tt);
-        names_type rhs (names (t, tt));
+          names_type& ns (lhs.as<names_type> ());
+          ns.back ().pair = ':';
 
-        bool r;
-        switch (op)
-        {
-        case type::equal:     r = ns == rhs; break;
-        case type::not_equal: r = ns != rhs; break;
-        default:              r = false;     assert (false);
+          next (t, tt);
+          value rhs (eval_trailer (t, tt));
+
+          if (tt != type::rparen ||
+              rhs.type != nullptr || rhs.null () || rhs.empty ())
+            fail (l) << "variable name expected after ':'";
+
+          ns.insert (ns.end (),
+                     make_move_iterator (rhs.as<names_type> ().begin ()),
+                     make_move_iterator (rhs.as<names_type> ().end ()));
+
+          break;
         }
+      case type::equal:
+      case type::not_equal:
+        {
+          type op (tt);
 
-        ns.resize (1);
-        ns[0] = name (r ? "true" : "false");
+          // ==, != are left-associative, so get the rhs value and evaluate.
+          //
+          next (t, tt);
+          value rhs (parse_value ());
 
-        eval_trailer (t, tt, ns);
-        break;
+          // Use (potentially typed) comparison via value.
+          //
+          // @@ Should we detect type mismatch here?
+          //
+          bool r;
+          switch (op)
+          {
+          case type::equal:     r = lhs == rhs; break;
+          case type::not_equal: r = lhs != rhs; break;
+          default:              r = false;     assert (false);
+          }
+
+          // While storing the result as a bool value might seem like a good
+          // idea, this would mean the user won't be able to compare it to
+          // (untyped) true/false names.
+          //
+          names_type ns;
+          ns.push_back (name (r ? "true" : "false"));
+          lhs = value (move (ns));
+          break;
+        }
+      default:
+        fail (l) << "expected ')' instead of " << t;
       }
-    case type::rparen:
-      break;
-    default:
-      fail (t) << "expected ')' instead of " << t;
     }
 
-    return !null;
+    return lhs;
   }
 
-  bool parser::
-  attributes (token& t, token_type& tt, bool standalone)
+  parser::attributes& parser::
+  attributes_push (token& t, token_type& tt, bool standalone)
   {
-    as_.clear ();
+    attributes_.push (
+      attributes {tt == type::lsbrace, get_location (t, &path_), {}});
 
-    if (tt != type::lsbrace)
-      return (ha_ = false);
+    attributes& a (attributes_.top ());
+    const location& l (a.loc);
 
-    al_ = get_location (t, &path_);
+    if (!a.has)
+      return a;
 
     // Using '@' for key-value pairs would be just too ugly. Seeing that we
     // control what goes into keys/values, let's use a much nicer '='.
@@ -1587,13 +1663,13 @@ namespace build2
         }
         catch (const invalid_argument&)
         {
-          fail (al_) << "invalid attribute key '" << *i << "'";
+          fail (l) << "invalid attribute key '" << *i << "'";
         }
 
         if (i->pair)
         {
           if (i->pair != '=')
-            fail << "unexpected pair style in attributes";
+            fail (l) << "unexpected pair style in attributes";
 
           try
           {
@@ -1601,11 +1677,11 @@ namespace build2
           }
           catch (const invalid_argument&)
           {
-            fail (al_) << "invalid attribute value '" << *i << "'";
+            fail (l) << "invalid attribute value '" << *i << "'";
           }
         }
 
-        as_.emplace_back (move (k), move (v));
+        a.ats.emplace_back (move (k), move (v));
       }
     }
 
@@ -1623,7 +1699,7 @@ namespace build2
     if (!standalone && (tt == type::newline || tt == type::eos))
       fail (t) << "standalone attributes";
 
-    return (ha_ = true);
+    return a;
   }
 
   // Parse names inside {} and handle the following "crosses" (i.e.,
@@ -1960,9 +2036,9 @@ namespace build2
         names_type lv_storage;
         names_view lv;
 
-        // Check if we should set/propagate NULL. We only do this is 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 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.
         //
         auto set_null = [first, &tt] ()
         {
@@ -1976,6 +2052,7 @@ namespace build2
 
         location loc;
         const char* what; // Variable or evaluation context.
+        value result;     // Holds function call/eval context result.
 
         if (tt == type::dollar)
         {
@@ -1996,7 +2073,13 @@ namespace build2
           else if (tt == type::lparen)
           {
             expire_mode ();
-            names_type ns (eval (t, tt).first);
+            value v (eval (t, tt));
+
+            if (v.null ())
+              fail (loc) << "null variable/function name";
+
+            names_type storage;
+            vector_view<build2::name> ns (reverse (v, storage)); // Movable.
             size_t n (ns.size ());
 
             // Make sure the result of evaluation is a potentially-qualified
@@ -2035,26 +2118,37 @@ namespace build2
 
             // Just a stub for now.
             //
-            // Should we use qualification as the context in which to call
-            // the function?
+            // Should we use (target/scope) qualification (of name) as the
+            // context in which to call the function?
             //
-            std::pair<names_type, bool> a (eval (t, tt));
-            cout << name << "(" << a.first << ")" << endl;
+            value a (eval (t, tt));
+            cout << name << "(";
+
+            if (a.null ())
+              cout << "[null]";
+            else if (!a.empty ())
+              cout << reverse (a, lv_storage);
+
+            cout << ")" << endl;
 
-            std::pair<names_type, bool> r (names_type (), false); // NULL.
-            lv_storage.swap (r.first);
+            result = value (); // NULL for now.
 
             tt = peek ();
 
             // See if we should propagate the NULL indicator.
             //
-            if (set_null ())
-              null = !r.second;
+            if (result.null ())
+            {
+              if (set_null ())
+                null = true;
+
+              continue;
+            }
 
-            if (lv_storage.empty ())
+            if (result.empty ())
               continue;
 
-            lv = lv_storage;
+            lv = reverse (result, lv_storage);
             what = "function call";
           }
           else
@@ -2108,25 +2202,28 @@ namespace build2
         else
         {
           loc = get_location (t, &path_);
-
-          std::pair<names_type, bool> r (eval (t, tt));
-          lv_storage.swap (r.first);
+          result = eval (t, tt);
 
           tt = peek ();
 
           // See if we should propagate the NULL indicator.
           //
-          if (set_null ())
-            null = !r.second;
+          if (result.null ())
+          {
+            if (set_null ())
+              null = true;
+
+            continue;
+          }
 
-          if (lv_storage.empty ())
+          if (result.empty ())
             continue;
 
-          lv = lv_storage;
+          lv = reverse (result, lv_storage);
           what = "context evaluation";
         }
 
-        // @@ Could move if lv is lv_storage.
+        // @@ Could move if lv is lv_storage (or even result).
         //
 
         // Should we accumulate? If the buffer is not empty, then
diff --git a/build2/scope.cxx b/build2/scope.cxx
index b22cb0c..1459687 100644
--- a/build2/scope.cxx
+++ b/build2/scope.cxx
@@ -326,7 +326,7 @@ namespace build2
     // while we still need to apply prefixes/suffixes in the type-aware way.
     //
     if (cache.value.type == nullptr && var.type != nullptr)
-      typify (cache.value, *var.type, var);
+      typify (cache.value, *var.type, &var);
 
     // Now apply override prefixes and suffixes. Also calculate the vars and
     // depth of the result, which will be those of the stem or prefix/suffix
@@ -362,11 +362,11 @@ namespace build2
 
         if (l) // No sense to prepend/append if NULL.
         {
-          cache.value.prepend (names (cast<names> (l)), var);
+          cache.value.prepend (names (cast<names> (l)), &var);
         }
         else if ((l = find (o, ".__suffix")))
         {
-          cache.value.append (names (cast<names> (l)), var);
+          cache.value.append (names (cast<names> (l)), &var);
         }
 
         if (l.defined ())
diff --git a/build2/variable b/build2/variable
index ea60832..a5f2ea5 100644
--- a/build2/variable
+++ b/build2/variable
@@ -50,13 +50,13 @@ namespace build2
     void (*const copy_assign) (value&, const value&, bool move);
 
     // While assign cannot be NULL, if append or prepend is NULL, then this
-    // means this type doesn't support this operation. Variable is provided
-    // primarily for diagnostics. Return true if the resulting value is not
-    // empty.
+    // means this type doesn't support this operation. Variable is optional
+    // and is provided only for diagnostics. Return true if the resulting
+    // value is not empty.
     //
-    bool (*const assign) (value&, names&&, const variable&);
-    bool (*const append) (value&, names&&, const variable&);
-    bool (*const prepend) (value&, names&&, const variable&);
+    bool (*const assign) (value&, names&&, const variable*);
+    bool (*const append) (value&, names&&, const variable*);
+    bool (*const prepend) (value&, names&&, const variable*);
 
     // Reverse the value back to a vector of names. Storage can be used by the
     // implementation if necessary. Cannot be NULL.
@@ -141,10 +141,10 @@ namespace build2
     bool operator== (nullptr_t) const {return null ();}
     bool operator!= (nullptr_t) const {return !null ();}
 
-    // Creation. A newly created value is NULL and can be reset back to NULL
-    // by assigning nullptr. Values can be copied and copy-assigned. Note that
-    // for assignment, the values' types should be the same or LHS should be
-    // untyped.
+    // Creation. A default-initialzied value is NULL and can be reset back to
+    // NULL by assigning nullptr. Values can be copied and copy-assigned. Note
+    // that for assignment, the values' types should be the same or LHS should
+    // be untyped.
     //
     //
   public:
@@ -154,6 +154,9 @@ namespace build2
     value (const value_type* t = nullptr)
         : type (t), state (value_state::null), extra (0) {}
 
+    explicit
+    value (names&&); // Create untyped value.
+
     // Note: preserves type.
     //
     value&
@@ -182,17 +185,17 @@ namespace build2
     value& operator= (const char* v) {return *this = string (v);}
     value& operator+= (const char* v) {return *this += string (v);}
 
-    // Assign/append/prepend raw data. Variable is normally only used for
-    // diagnostics.
+    // Assign/append/prepend raw data. Variable is optional and is only used
+    // for diagnostics.
     //
     void
-    assign (names&&, const variable&);
+    assign (names&&, const variable*);
 
     void
-    append (names&&, const variable&);
+    append (names&&, const variable*);
 
     void
-    prepend (names&&, const variable&);
+    prepend (names&&, const variable*);
 
     // Implementation details, don't use directly except in representation
     // type implementations.
@@ -224,8 +227,8 @@ namespace build2
     reset ();
   };
 
-  // The values should be of the same type (or both be untyped). NULL values
-  // compare equal.
+  // The values should be of the same type (or both be untyped) except NULL
+  // values can also be untyped. NULL values compare equal.
   //
   bool operator== (const value&, const value&);
   bool operator!= (const value&, const value&);
@@ -249,12 +252,12 @@ namespace build2
   template <typename T> const T* cast_null (const value&);
   template <typename T> const T* cast_null (const lookup&);
 
-  // Assign value type to the value. Variable is normally only used for
-  // diagnostics.
+  // Assign value type to the value. In the second version the variable is
+  // optional and is only used for diagnostics.
   //
   template <typename T>
   void typify (value&, const variable&);
-  void typify (value&, const value_type&, const variable&);
+  void typify (value&, const value_type&, const variable*);
 
   // Reverse the value back to names. The value should no be NULL and storage
   // should be empty.
@@ -262,6 +265,9 @@ namespace build2
   names_view
   reverse (const value&, names& storage);
 
+  vector_view<name>
+  reverse (value&, names& storage);
+
   // lookup
   //
   // A variable can be undefined, NULL, or contain a (potentially empty)
@@ -395,15 +401,15 @@ namespace build2
   //
   template <typename T, bool empty>
   static bool
-  simple_assign (value&, names&&, const variable&);
+  simple_assign (value&, names&&, const variable*);
 
   template <typename T, bool empty>
   static bool
-  simple_append (value&, names&&, const variable&);
+  simple_append (value&, names&&, const variable*);
 
   template <typename T, bool empty>
   static bool
-  simple_prepend (value&, names&&, const variable&);
+  simple_prepend (value&, names&&, const variable*);
 
   // Default implementations of the reverse callback for simple types that
   // calls value_traits<T>::reverse() and adds the result to the vector. As a
diff --git a/build2/variable.cxx b/build2/variable.cxx
index d6a42b3..fdee0dd 100644
--- a/build2/variable.cxx
+++ b/build2/variable.cxx
@@ -138,7 +138,7 @@ namespace build2
   }
 
   void value::
-  assign (names&& ns, const variable& var)
+  assign (names&& ns, const variable* var)
   {
     assert (type == nullptr || type->assign != nullptr);
 
@@ -165,7 +165,7 @@ namespace build2
   }
 
   void value::
-  append (names&& ns, const variable& var)
+  append (names&& ns, const variable* var)
   {
     bool r;
 
@@ -194,8 +194,14 @@ namespace build2
     else
     {
       if (type->append == nullptr)
-        fail << type->name << " value in variable " << var.name
-             << " cannot be appended to";
+      {
+        diag_record dr (fail);
+
+        dr << "cannot append to " << type->name << " value";
+
+        if (var != nullptr)
+          dr << " in variable " << var->name;
+      }
 
       r = type->append (*this, move (ns), var);
     }
@@ -204,7 +210,7 @@ namespace build2
   }
 
   void value::
-  prepend (names&& ns, const variable& var)
+  prepend (names&& ns, const variable* var)
   {
     bool r;
 
@@ -234,8 +240,14 @@ namespace build2
     else
     {
       if (type->prepend == nullptr)
-        fail << type->name << " value in variable " << var.name
-             << " cannot be prepended to";
+      {
+        diag_record dr (fail);
+
+        dr << "cannot prepend to " << type->name << " value";
+
+        if (var != nullptr)
+          dr << " in variable " << var->name;
+      }
 
       r = type->prepend (*this, move (ns), var);
     }
@@ -246,13 +258,18 @@ namespace build2
   bool
   operator== (const value& x, const value& y)
   {
-    assert (x.type == y.type);
+    bool xn (x.null ());
+    bool yn (y.null ());
+
+    assert (x.type == y.type ||
+            (xn && x.type == nullptr) ||
+            (yn && y.type == nullptr));
 
     if (x.state != y.state)
       return false;
 
-    if (x.null ())
-      return true;
+    if (xn)
+      return true; // Both are NULL since same state.
 
     if (x.type == nullptr)
       return x.as<names> () == y.as<names> ();
@@ -264,7 +281,7 @@ namespace build2
   }
 
   void
-  typify (value& v, const value_type& t, const variable& var)
+  typify (value& v, const value_type& t, const variable* var)
   {
     if (v.type == nullptr)
     {
@@ -282,9 +299,16 @@ namespace build2
     }
     else if (v.type != &t)
     {
-      fail << "variable " << var.name << " type mismatch" <<
-        info << "value type is " << v.type->name <<
-        info << (&t == var.type ? "variable" : "new") << " type is " << t.name;
+      diag_record dr (fail);
+
+      dr << "type mismatch";
+
+      if (var != nullptr)
+        dr << " in variable " << var->name;
+
+      dr << info << "value type is " << v.type->name;
+      dr << info << (var != nullptr && &t == var->type ? "variable" : "new")
+         << " type is " << t.name;
     }
   }
 
@@ -650,7 +674,7 @@ namespace build2
     // First access after being assigned a type?
     //
     if (r != nullptr && var.type != nullptr && r->type != var.type)
-      typify (const_cast<value&> (*r), *var.type, var);
+      typify (const_cast<value&> (*r), *var.type, &var);
 
     return  r;
   }
@@ -664,7 +688,7 @@ namespace build2
     // First access after being assigned a type?
     //
     if (r != nullptr && var.type != nullptr && r->type != var.type)
-      typify (*r, *var.type, var);
+      typify (*r, *var.type, &var);
 
     return  r;
   }
@@ -678,7 +702,7 @@ namespace build2
     // First access after being assigned a type?
     //
     if (!r.second && var.type != nullptr && v.type != var.type)
-      typify (v, *var.type, var);
+      typify (v, *var.type, &var);
 
     return make_pair (reference_wrapper<value> (v), r.second);
   }
diff --git a/build2/variable.ixx b/build2/variable.ixx
index 9b29608..3184108 100644
--- a/build2/variable.ixx
+++ b/build2/variable.ixx
@@ -8,6 +8,15 @@ namespace build2
 {
   // value
   //
+  inline value::
+  value (names&& ns)
+      : type (nullptr),
+        state (ns.empty () ? value_state::empty : value_state::filled),
+        extra (0)
+  {
+    new (&data_) names (move (ns));
+  }
+
   inline value& value::
   operator= (reference_wrapper<value> v)
   {
@@ -146,7 +155,7 @@ namespace build2
     value_type& t (value_traits<T>::value_type);
 
     if (v.type != &t)
-      typify (v, t, var);
+      typify (v, t, &var);
   }
 
   inline names_view
@@ -158,6 +167,13 @@ namespace build2
     return v.type == nullptr ? v.as<names> () : v.type->reverse (v, storage);
   }
 
+  inline vector_view<name>
+  reverse (value& v, names& storage)
+  {
+    names_view cv (reverse (static_cast<const value&> (v), storage));
+    return vector_view<name> (const_cast<name*> (cv.data ()), cv.size ());
+  }
+
   // value_traits
   //
   template <typename T>
@@ -610,7 +626,7 @@ namespace build2
     // First access after being assigned a type?
     //
     if (var.type != nullptr && val.type != var.type)
-      typify (const_cast<value&> (val), *var.type, var);
+      typify (const_cast<value&> (val), *var.type, &var);
 
     return r;
   }
@@ -626,7 +642,7 @@ namespace build2
     // First access after being assigned a type?
     //
     if (var.type != nullptr && val.type != var.type)
-      typify (const_cast<value&> (val), *var.type, var);
+      typify (const_cast<value&> (val), *var.type, &var);
 
     return p;
   }
diff --git a/build2/variable.txx b/build2/variable.txx
index 99ea79b..31d9284 100644
--- a/build2/variable.txx
+++ b/build2/variable.txx
@@ -37,7 +37,7 @@ namespace build2
 
   template <typename T, bool empty>
   bool
-  simple_assign (value& v, names&& ns, const variable& var)
+  simple_assign (value& v, names&& ns, const variable* var)
   {
     size_t n (ns.size ());
 
@@ -54,14 +54,20 @@ namespace build2
       catch (const invalid_argument&) {} // Fall through.
     }
 
-    error << "invalid " << value_traits<T>::value_type.name
-          << " value '" << ns << "' in variable " << var.name;
+    diag_record dr (error);
+
+    dr << "invalid " << value_traits<T>::value_type.name
+       << " value '" << ns << "'";
+
+    if (var != nullptr)
+      dr << " in variable " << var->name;
+
     throw failed ();
   }
 
   template <typename T, bool empty>
   bool
-  simple_append (value& v, names&& ns, const variable& var)
+  simple_append (value& v, names&& ns, const variable* var)
   {
     size_t n (ns.size ());
 
@@ -78,14 +84,20 @@ namespace build2
       catch (const invalid_argument&) {} // Fall through.
     }
 
-    error << "invalid " << value_traits<T>::value_type.name
-          << " value '" << ns << "' in variable " << var.name;
+    diag_record dr (error);
+
+    dr << "invalid " << value_traits<T>::value_type.name
+       << " value '" << ns << "'";
+
+    if (var != nullptr)
+      dr << " in variable " << var->name;
+
     throw failed ();
   }
 
   template <typename T, bool empty>
   bool
-  simple_prepend (value& v, names&& ns, const variable& var)
+  simple_prepend (value& v, names&& ns, const variable* var)
   {
     size_t n (ns.size ());
 
@@ -102,8 +114,14 @@ namespace build2
       catch (const invalid_argument&) {} // Fall through.
     }
 
-    error << "invalid " << value_traits<T>::value_type.name
-          << " value '" << ns << "' in variable " << var.name;
+    diag_record dr (error);
+
+    dr << "invalid " << value_traits<T>::value_type.name
+       << " value '" << ns << "'";
+
+    if (var != nullptr)
+      dr << " in variable " << var->name;
+
     throw failed ();
   }
 
@@ -127,7 +145,7 @@ namespace build2
 
   template <typename T>
   bool
-  vector_append (value& v, names&& ns, const variable& var)
+  vector_append (value& v, names&& ns, const variable* var)
   {
     vector<T>* p (v.null ()
                   ? new (&v.data_) vector<T> ()
@@ -145,10 +163,16 @@ namespace build2
         r = &*++i;
 
         if (n.pair != '@')
-          fail << "unexpected pair style for "
-               << value_traits<T>::value_type.name << " value "
-               << "'" << n << "'" << n.pair << "'" << *r << "' "
-               << "in variable " << var.name;
+        {
+          diag_record dr (fail);
+
+          dr << "unexpected pair style for "
+             << value_traits<T>::value_type.name << " value "
+             << "'" << n << "'" << n.pair << "'" << *r << "'";
+
+          if (var != nullptr)
+            dr << " in variable " << var->name;
+        }
       }
 
       try
@@ -166,7 +190,8 @@ namespace build2
         else
           dr << " element '" << n << "'";
 
-        dr << " in variable " << var.name;
+        if (var != nullptr)
+          dr << " in variable " << var->name;
       }
     }
 
@@ -175,7 +200,7 @@ namespace build2
 
   template <typename T>
   bool
-  vector_assign (value& v, names&& ns, const variable& var)
+  vector_assign (value& v, names&& ns, const variable* var)
   {
     if (!v.null ())
       v.as<vector<T>> ().clear ();
@@ -185,7 +210,7 @@ namespace build2
 
   template <typename T>
   bool
-  vector_prepend (value& v, names&& ns, const variable& var)
+  vector_prepend (value& v, names&& ns, const variable* var)
   {
     // Reduce to append.
     //
@@ -270,7 +295,7 @@ namespace build2
   //
   template <typename K, typename V>
   bool
-  map_append (value& v, names&& ns, const variable& var)
+  map_append (value& v, names&& ns, const variable* var)
   {
     using std::map;
 
@@ -285,17 +310,29 @@ namespace build2
       name& l (*i);
 
       if (!l.pair)
-        fail << value_traits<map<K, V>>::value_type.name << " key-value "
-             << "pair expected instead of '" << l << "' "
-             << "in variable " << var.name;
+      {
+        diag_record dr (fail);
+
+        dr << value_traits<map<K, V>>::value_type.name << " key-value "
+           << "pair expected instead of '" << l << "'";
+
+        if (var != nullptr)
+          dr << " in variable " << var->name;
+      }
 
       name& r (*++i); // Got to have the second half of the pair.
 
       if (l.pair != '@')
-        fail << "unexpected pair style for "
-             << value_traits<map<K, V>>::value_type.name << " key-value "
-             << "'" << l << "'" << l.pair << "'" << r << "' "
-             << "in variable " << var.name;
+      {
+        diag_record dr (fail);
+
+        dr << "unexpected pair style for "
+           << value_traits<map<K, V>>::value_type.name << " key-value "
+           << "'" << l << "'" << l.pair << "'" << r << "'";
+
+        if (var != nullptr)
+          dr << " in variable " << var->name;
+      }
 
       try
       {
@@ -309,14 +346,24 @@ namespace build2
         }
         catch (const invalid_argument&)
         {
-          fail << "invalid " << value_traits<V>::value_type.name
-               << " element value '" << r << "' in variable " << var.name;
+          diag_record dr (fail);
+
+          dr << "invalid " << value_traits<V>::value_type.name
+             << " element value '" << r << "'";
+
+          if (var != nullptr)
+            dr << " in variable " << var->name;
         }
       }
       catch (const invalid_argument&)
       {
-        fail << "invalid " << value_traits<K>::value_type.name
-             << " element key '" << l << "' in variable " << var.name;
+        diag_record dr (fail);
+
+        dr << "invalid " << value_traits<K>::value_type.name
+           << " element key '" << l << "'";
+
+        if (var != nullptr)
+          dr << " in variable " << var->name;
       }
     }
 
@@ -325,7 +372,7 @@ namespace build2
 
   template <typename K, typename V>
   bool
-  map_assign (value& v, names&& ns, const variable& var)
+  map_assign (value& v, names&& ns, const variable* var)
   {
     using std::map;
 
diff --git a/tests/eval/buildfile b/tests/eval/buildfile
index 06fb5fb..b1e5350 100644
--- a/tests/eval/buildfile
+++ b/tests/eval/buildfile
@@ -42,3 +42,19 @@ print "foo equals bar is (foo == bar)"
 foo = foo
 print ($foo == foo)
 print (bar != $foo)
+
+print ([null])
+print (([null]))
+print ([uint64] 01)
+
+n = [null]
+print ($n == [null])
+print ($N == [null])
+print ([null] == [null])
+
+print ($n == $N == true)
+
+n =
+print ($n == )
+n = {}
+print ($n == "")
diff --git a/tests/eval/test.out b/tests/eval/test.out
index 599f928..bad003e 100644
--- a/tests/eval/test.out
+++ b/tests/eval/test.out
@@ -18,3 +18,12 @@ true baz
 foo equals bar is false
 true
 true
+[null]
+[null]
+1
+true
+true
+true
+true
+true
+true
diff --git a/tests/variable/override/buildfile b/tests/variable/override/buildfile
index 18c9831..c090e81 100644
--- a/tests/variable/override/buildfile
+++ b/tests/variable/override/buildfile
@@ -1,4 +1,4 @@
-if ("$t" != "")
+if ($t != [null])
 {
   [$t] v = [null]
 }
diff --git a/tests/variable/qualified/buildfile b/tests/variable/qualified/buildfile
index 90e75c7..870b808 100644
--- a/tests/variable/qualified/buildfile
+++ b/tests/variable/qualified/buildfile
@@ -1,5 +1,5 @@
-#v = (:bar) # error: expected name instead of ':'
-#v = (foo:) # error: scope/target expected after ':'
+#v = (foo:) # error: variable name expected before ':'
+#v = (:bar) # error: scope/target expected after ':'
 
 print (foo:bar)
 print (foo :bar)
-- 
cgit v1.1