Implement value typing, null support via value attributes

For example:

v = [null]
v = [string] abc
v += ABC # abcABC

12 files changed, 429 insertions, 217 deletions

diff --git a/build2/b.cxx b/build2/b.cxx
index 44fa032..f9c3683 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -819,7 +819,7 @@ main (int argc, char* argv[])
             }
 
             value& v (p.first);
-            v.assign (names (o.val), o.var); // Original var for diagnostics.
+            v = o.val;
             first = false;
           }
 
diff --git a/build2/context b/build2/context
index bd56dbb..ed2b0ab 100644
--- a/build2/context
+++ b/build2/context
@@ -10,6 +10,7 @@
 #include <build2/types>
 #include <build2/utility>
 
+#include <build2/variable>
 #include <build2/operation>
 #include <build2/b-options>
 
@@ -17,7 +18,6 @@ namespace build2
 {
   class scope;
   class file;
-  struct variable;
 
   extern dir_path work;
   extern dir_path home;
@@ -49,7 +49,7 @@ namespace build2
   {
     const variable& var; // Original variable.
     const variable& ovr; // Override variable.
-    names val;
+    value val;
   };
 
   using variable_overrides = vector<variable_override>;
diff --git a/build2/context.cxx b/build2/context.cxx
index 26fc41a..941f98a 100644
--- a/build2/context.cxx
+++ b/build2/context.cxx
@@ -136,14 +136,19 @@ namespace build2
       o = o->override.get ();
 
       // Currently we expand project overrides in the global scope to keep
-      // things simple.
+      // things simple. Pass original variable for diagnostics.
       //
       parser p;
-      names val;
-      t = p.parse_variable_value (l, gs, val);
+      pair<value, token> r (p.parse_variable_value (l, gs, var));
 
-      if (t.type != token_type::eos)
-        fail << "unexpected " << t << " in variable assignment '" << s << "'";
+      if (r.second.type != token_type::eos)
+        fail << "unexpected " << r.second << " in variable assignment "
+             << "'" << s << "'";
+
+      // Make sure the value is not typed.
+      //
+      if (r.first.type != nullptr)
+        fail << "typed override of variable " << var.name;
 
       if (c == '!')
       {
@@ -153,10 +158,10 @@ namespace build2
           fail << "multiple global overrides of variable " << var.name;
 
         value& v (p.first);
-        v.assign (move (val), var); // Original var for diagnostics.
+        v = move (r.first);
       }
       else
-        vos.emplace_back (variable_override {var, *o, move (val)});
+        vos.emplace_back (variable_override {var, *o, move (r.first)});
     }
 
     // Enter builtin variables.
diff --git a/build2/parser b/build2/parser
index 23f90cc..89ce9b8 100644
--- a/build2/parser
+++ b/build2/parser
@@ -11,20 +11,19 @@
 #include <build2/spec>
 #include <build2/lexer>
 #include <build2/token>
+#include <build2/variable>
 #include <build2/diagnostics>
 
 namespace build2
 {
   class scope;
   class target;
-  struct variable;
 
   class parser
   {
   public:
     using names_type = build2::names;
     using variable_type = build2::variable;
-    using attributes_type = vector<pair<string, string>>;
 
     // If boot is true, then we are parsing bootstrap.build and modules
     // should only be bootstrapped.
@@ -42,8 +41,8 @@ namespace build2
     token
     parse_variable (lexer&, scope&, const variable_type&, token_type kind);
 
-    token
-    parse_variable_value (lexer&, scope&, names_type& result);
+    pair<value, token>
+    parse_variable_value (lexer&, scope&, const variable_type&);
 
     names_type
     parse_export_stub (istream& is, const path& p, scope& r, scope& b)
@@ -88,27 +87,31 @@ namespace build2
     string
     variable_name (names_type&&, const location&);
 
-    names_type
+    pair<names_type, bool> // See names_null() return type.
     variable_value (token&, token_type&);
 
     void
-    variable_attribute (const variable_type&,
-                        attributes_type&,
-                        const location&);
+    variable_attributes (const variable_type&);
 
-    names_type
+    void
+    value_attributes (const variable_type&,
+                      value& lhs,
+                      pair<names_type, bool>&& rhs,
+                      token_type kind);
+
+    pair<names_type, bool> // See names_null() return type.
     eval (token&, token_type&);
 
-    void
+    bool
     eval_trailer (token&, token_type&, names_type&);
 
-    // If the next token is '[' parse the attribute sequence until ']', get
-    // the next token, verify it is not newline/eos, and return the pointer to
-    // the extracted attributes (which is only valid until the next call).
-    // Otherwise return NULL.
+    // 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_type*
-    attributes (token&, token_type&);
+    bool
+    attributes (token&, token_type&, bool standalone = false);
 
     // 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
@@ -122,13 +125,25 @@ namespace build2
       return ns;
     }
 
-    void
+    // 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.
+    //
+    pair<names_type, bool>
+    names_null (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);
+    }
+
+    bool
     names (token& t, token_type& tt, names_type& ns, bool chunk = false)
     {
-      names (t, tt, ns, chunk, 0, nullptr, nullptr, nullptr);
+      return names (t, tt, ns, chunk, 0, nullptr, nullptr, nullptr);
     }
 
-    void
+    bool
     names (token&, token_type&,
            names_type&,
            bool chunk,
@@ -305,7 +320,9 @@ namespace build2
     scope* scope_;   // Current base scope (out_base).
     scope* root_;    // Current root scope (out_root).
 
-    attributes_type attrs_; // Current attributes, if any.
+    bool ha_;                         // Has attributes flag.
+    location al_;                     // Current attributes location.
+    vector<pair<string, string>> as_; // Current attributes.
 
     target* default_target_;
     names_type export_value_;
diff --git a/build2/parser.cxx b/build2/parser.cxx
index c516cef..ce5b968 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -68,8 +68,8 @@ namespace build2
     return t;
   }
 
-  token parser::
-  parse_variable_value (lexer& l, scope& s, names_type& result)
+  pair<value, token> parser::
+  parse_variable_value (lexer& l, scope& s, const variable_type& var)
   {
     path_ = &l.name ();
     lexer_ = &l;
@@ -78,8 +78,12 @@ namespace build2
 
     type tt;
     token t (type::eos, false, 0, 0);
-    result = variable_value (t, tt);
-    return t;
+    pair<names_type, bool> p (variable_value (t, tt));
+
+    value v;
+    value_attributes (var, v, move (p), type::assign);
+
+    return make_pair (move (v), move (t));
   }
 
   void parser::
@@ -97,8 +101,7 @@ namespace build2
     {
       // Extract attributes if any.
       //
-      location al (get_location (t, &path_));
-      attributes_type* as (attributes (t, tt));
+      attributes (t, tt);
 
       // We always start with one or more names.
       //
@@ -164,8 +167,8 @@ namespace build2
 
         if (f != nullptr)
         {
-          if (as != nullptr)
-            fail (al) << "attributes before " << n;
+          if (ha_)
+            fail (al_) << "attributes before " << n;
 
           (this->*f) (t, tt);
           continue;
@@ -260,8 +263,8 @@ namespace build2
             {
               // Directory scope.
               //
-              if (as != nullptr)
-                fail (al) << "attributes before directory scope";
+              if (ha_)
+                fail (al_) << "attributes before directory scope";
 
               // Can contain anything that a top level can.
               //
@@ -271,8 +274,8 @@ namespace build2
             }
             else
             {
-              if (as != nullptr)
-                fail (al) << "attributes before target scope";
+              if (ha_)
+                fail (al_) << "attributes before target scope";
 
               // @@ TODO: target scope.
             }
@@ -302,11 +305,10 @@ namespace build2
         // Will have to stash them if later support attributes on
         // target/scope.
         //
-        if (as != nullptr)
-          fail (al) << "attributes before target/scope";
+        if (ha_)
+          fail (al_) << "attributes before target/scope";
 
-        al = get_location (t, &path_);
-        as = attributes (t, tt);
+        attributes (t, tt);
 
         if (tt == type::name    ||
             tt == type::lcbrace ||
@@ -361,8 +363,8 @@ namespace build2
 
             // Handle variable attributes.
             //
-            if (as != nullptr)
-              variable_attribute (var, *as, al);
+            if (ha_)
+              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
@@ -431,13 +433,12 @@ namespace build2
                     fail (at) << "append to target type/pattern-specific "
                               << "variable " << var.name;
 
-                  // Note: expanding variables in the value in the context of
-                  // the scope.
+                  // Note: expanding the value in the context of the scope.
                   //
-                  names_type vns (variable_value (t, tt));
-                  value& val (scope_->target_vars[*ti][move (n.value)].assign (
-                                var).first);
-                  val.assign (move (vns), var);
+                  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);
 
                   leave_scope ();
                 }
@@ -450,8 +451,8 @@ namespace build2
           //
           else
           {
-            if (as != nullptr)
-              fail (al) << "attributes before prerequisites";
+            if (ha_)
+              fail (al_) << "attributes before prerequisites";
 
             // Prepare the prerequisite list.
             //
@@ -524,8 +525,8 @@ namespace build2
 
         // Handle variable attributes.
         //
-        if (as != nullptr)
-          variable_attribute (var, *as, al);
+        if (ha_)
+          variable_attributes (var);
 
         variable (t, tt, var, tt);
 
@@ -541,8 +542,8 @@ namespace build2
       //
       if (tt == type::newline && ns.empty ())
       {
-        if (as != nullptr)
-          fail (al) << "standalone attributes";
+        if (ha_)
+          fail (al_) << "standalone attributes";
 
         next (t, tt);
         continue;
@@ -801,11 +802,10 @@ namespace build2
     mode (lexer_mode::pairs, '@');
     next (t, tt);
 
-    // Get attributes, if any (note that here we will go into a nested pairs
-    // mode).
+    // Get variable attributes, if any (note that here we will go into a
+    // nested pairs mode).
     //
-    location al (get_location (t, &path_));
-    attributes_type* as (attributes (t, tt));
+    attributes (t, tt);
 
     if (tt == type::name)
     {
@@ -876,15 +876,15 @@ namespace build2
     {
       // Handle variable attributes.
       //
-      if (as != nullptr)
-        variable_attribute (*var, *as, al);
+      if (ha_)
+        variable_attributes (*var);
 
       val = at == type::assign
         ? &scope_->assign (*var)
         : &scope_->append (*var);
     }
-    else if (as != nullptr)
-      fail (al) << "attributes without variable";
+    else if (ha_)
+      fail (al_) << "attributes without variable";
 
     // The rest should be a list of projects and/or targets. Parse
     // them as names to get variable expansion and directory prefixes.
@@ -932,14 +932,27 @@ namespace build2
     if (ps == nullptr || ps->out_path () != scope_->out_path ())
       fail (t) << "export outside export stub";
 
-    // The rest is a value. Parse it as names to get variable expansion.
-    // build2::import() will check the names, if required.
+    // The rest is a value. Parse it as a variable value to get expansion,
+    // attributes, etc. build2::import() will check the names, if required.
     //
-    mode (lexer_mode::pairs, '@');
-    next (t, tt);
+    pair<names_type, bool> p (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.
+    //                   export_value_,
+    //                   move (p),
+    //                   type::assign);
+    if (ha_)
+      fail (al_) << "attributes in export";
 
-    if (tt != type::newline && tt != type::eos)
-      export_value_ = names (t, tt);
+    if (!p.second)
+      fail (t) << "null value in export";
+
+    export_value_ = move (p.first);
 
     if (tt == type::newline)
       next (t, tt);
@@ -1211,17 +1224,35 @@ namespace build2
   void parser::
   print (token& t, type& tt)
   {
-    // Parse the rest as names to get variable expansion, etc. Switch to the
-    // variable lexing mode so that we don't treat special characters (e.g.,
-    // ':') as the end of the names.
+    // Parse the rest as a variable value to get expansion, attributes, etc.
     //
-    mode (lexer_mode::pairs, '@');
-    next (t, tt);
-    names_type ns (tt != type::newline && tt != type::eos
-                   ? names (t, tt)
-                   : names_type ());
+    pair<names_type, bool> p (variable_value (t, tt));
 
-    cout << ns << endl;
+    if (ha_)
+    {
+      // Round-trip it through value.
+      //
+      value v;
+      value_attributes (var_pool.find ("__print"), // Well, this is a hack.
+                        v,
+                        move (p),
+                        type::assign);
+
+      if (v.null ())
+        cout << "[null]" << endl;
+      else
+      {
+        p.first.clear ();
+        cout << reverse (v, p.first) << endl;
+      }
+    }
+    else
+    {
+      if (!p.second)
+        cout << "[null]" << endl;
+      else
+        cout << p.first << endl;
+    }
 
     if (tt != type::eos)
       next (t, tt); // Swallow newline.
@@ -1247,74 +1278,72 @@ namespace build2
   void parser::
   variable (token& t, type& tt, const variable_type& var, type kind)
   {
-    names_type vns (variable_value (t, tt));
+    pair<names_type, bool> p (variable_value (t, tt));
 
-    if (kind == type::assign)
-    {
-      value& v (target_ != nullptr
-                ? target_->assign (var)
-                : scope_->assign (var));
-      v.assign (move (vns), var);
-    }
-    else
-    {
-      value& v (target_ != nullptr
-                ? target_->append (var)
-                : scope_->append (var));
+    value& v (
+      kind == type::assign
+      ? target_ != nullptr ? target_->assign (var) : scope_->assign (var)
+      : target_ != nullptr ? target_->append (var) : scope_->append (var));
 
-      if (kind == type::prepend)
-        v.prepend (move (vns), var);
-      else
-        v.append (move (vns), var);
-    }
+    value_attributes (var, v, move (p), kind);
   }
 
-  names parser::
+  pair<parser::names_type, bool> parser::
   variable_value (token& t, type& tt)
   {
     mode (lexer_mode::pairs, '@');
     next (t, tt);
-    return (tt != type::newline && tt != type::eos
-            ? names (t, tt)
-            : names_type ());
+
+    // 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);
+
+    return tt != type::newline && tt != type::eos
+      ? names_null (t, tt)
+      : make_pair (names_type (), true);
+  }
+
+  static const value_type*
+  map_type (const string& n)
+  {
+    return
+      n == "bool"      ? &value_traits<bool>::value_type       :
+      n == "uint64"    ? &value_traits<uint64_t>::value_type   :
+      n == "string"    ? &value_traits<string>::value_type     :
+      n == "path"      ? &value_traits<path>::value_type       :
+      n == "dir_path"  ? &value_traits<dir_path>::value_type   :
+      n == "name"      ? &value_traits<name>::value_type       :
+      n == "strings"   ? &value_traits<strings>::value_type    :
+      n == "paths"     ? &value_traits<paths>::value_type      :
+      n == "dir_paths" ? &value_traits<dir_paths>::value_type  :
+      n == "names"     ? &value_traits<names>::value_type      :
+      nullptr;
   }
 
   void parser::
-  variable_attribute (const variable_type& var,
-                      attributes_type& as,
-                      const location& al)
+  variable_attributes (const variable_type& var)
   {
     const value_type* type (nullptr);
 
-    for (auto& p: as)
+    for (auto& p: as_)
     {
       string& k (p.first);
       string& v (p.second);
 
-      if (const value_type* t =
-          k == "bool"      ? &value_traits<bool>::value_type       :
-          k == "uint64"    ? &value_traits<uint64_t>::value_type   :
-          k == "string"    ? &value_traits<string>::value_type     :
-          k == "path"      ? &value_traits<path>::value_type       :
-          k == "dir_path"  ? &value_traits<dir_path>::value_type   :
-          k == "name"      ? &value_traits<name>::value_type       :
-          k == "strings"   ? &value_traits<strings>::value_type    :
-          k == "paths"     ? &value_traits<paths>::value_type      :
-          k == "dir_paths" ? &value_traits<dir_paths>::value_type  :
-          k == "names"     ? &value_traits<names_type>::value_type :
-          nullptr)
+      if (const value_type* t = map_type (k))
       {
-        if (!v.empty ())
-          fail (al) << "value in variable type " << k << ": " << v;
-
-        if (type != nullptr)
-          fail (al) << "multiple variable types: " << k << ", " << type->name;
+        if (type != nullptr && t != type)
+          fail (al_) << "multiple variable types: " << k << ", " << type->name;
 
         type = t;
-        continue;
+        // Fall through.
       }
+      else
+        fail (al_) << "unknown variable attribute " << k;
 
-      fail (al) << "unknown variable attribute " << k;
+      if (!v.empty ())
+        fail (al_) << "unexpected value for attribute " << k << ": " << v;
     }
 
     if (type != nullptr)
@@ -1322,30 +1351,131 @@ 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 (al_) << "changing variable " << var.name << " type from "
+                   << var.type->name << " to " << type->name;
+    }
+  }
+
+  void parser::
+  value_attributes (const variable_type& var,
+                    value& v,
+                    pair<names_type, bool>&& rhs,
+                    token_type kind)
+  {
+    // Essentially this is an attribute-augmented assign/append/prepend.
+    //
+    bool null (false);
+    const value_type* type (nullptr);
+
+    for (auto& p: as_)
+    {
+      string& k (p.first);
+      string& v (p.second);
+
+      if (k == "null")
+      {
+        if (!rhs.first.empty () || !rhs.second)
+          fail (al_) << "value with null attribute";
+
+        null = true;
+        // Fall through.
+      }
+      else if (const value_type* t = map_type (k))
+      {
+        if (type != nullptr && t != type)
+          fail (al_) << "multiple value types: " << k << ", " << type->name;
+
+        type = t;
+        // Fall through.
+      }
+      else
+        fail (al_) << "unknown value attribute " << k;
+
+      if (!v.empty ())
+        fail (al_) << "unexpected value for attribute " << k << ": " << v;
+    }
+
+    // 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
+    // expect the resulting value to be of that type. So for assign we always
+    // override the type since it's a new value. For append/prepend we
+    // override if the LHS value is NULL (which also covers undefined). We
+    // also override if LHS is untyped. Otherwise, we require that the types
+    // be the same. Also check that the requested value type doesn't conflict
+    // with the variable type.
+    //
+    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 (kind == token_type::assign)
+      {
+        if (type != v.type)
+        {
+          v = nullptr; // Clear old value.
+          v.type = type;
+        }
+      }
+      else
+      {
+        if (!v)
+          v.type = type;
+        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;
+      }
+    }
+
+    if (null)
+      v = nullptr;
+    else
+    {
+      if (kind == token_type::assign)
+      {
+        if (rhs.second)
+          v.assign (move (rhs.first), var);
+        else
+          v = nullptr;
+      }
+      else if (rhs.second) // Don't append/prepent NULL.
+      {
+        if (kind == token_type::prepend)
+          v.prepend (move (rhs.first), var);
+        else
+          v.append (move (rhs.first), var);
+      }
     }
   }
 
-  parser::names_type parser::
+  pair<parser::names_type, bool> parser::
   eval (token& t, type& tt)
   {
     mode (lexer_mode::eval);
     next (t, tt);
 
     names_type ns;
-    eval_trailer (t, tt, ns);
-    return ns;
+    bool n (eval_trailer (t, tt, ns));
+    return make_pair (move (ns), n);
   }
 
-  void parser::
+  bool parser::
   eval_trailer (token& t, type& tt, names_type& ns)
   {
+    bool null (false);
+
     // Note that names() will handle the ( == foo) case since if it gets
     // called, it expects to see a name.
     //
     if (tt != type::rparen)
-      names (t, tt, ns);
+    {
+      if (!names (t, tt, ns) && tt == type::rparen)
+        null = true;
+    }
 
     switch (tt)
     {
@@ -1378,15 +1508,19 @@ namespace build2
     default:
       fail (t) << "expected ')' instead of " << t;
     }
+
+    return !null;
   }
 
-  parser::attributes_type* parser::
-  attributes (token& t, token_type& tt)
+  bool parser::
+  attributes (token& t, token_type& tt, bool standalone)
   {
-    attrs_.clear ();
+    as_.clear ();
 
     if (tt != type::lsbrace)
-      return nullptr;
+      return (ha_ = false);
+
+    al_ = get_location (t, &path_);
 
     // 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 '='.
@@ -1396,7 +1530,6 @@ namespace build2
 
     if (tt != type::rsbrace && tt != type::newline && tt != type::eos)
     {
-      const location l (get_location (t, &path_));
       names_type ns (names (t, tt));
 
       for (auto i (ns.begin ()); i != ns.end (); ++i)
@@ -1409,7 +1542,7 @@ namespace build2
         }
         catch (const invalid_argument&)
         {
-          fail (l) << "invalid attribute key '" << *i << "'";
+          fail (al_) << "invalid attribute key '" << *i << "'";
         }
 
         if (i->pair)
@@ -1420,11 +1553,11 @@ namespace build2
           }
           catch (const invalid_argument&)
           {
-            fail (l) << "invalid attribute value '" << *i << "'";
+            fail (al_) << "invalid attribute value '" << *i << "'";
           }
         }
 
-        attrs_.emplace_back (move (k), move (v));
+        as_.emplace_back (move (k), move (v));
       }
     }
 
@@ -1439,10 +1572,10 @@ namespace build2
 
     next (t, tt);
 
-    if (tt == type::newline || tt == type::eos)
+    if (!standalone && (tt == type::newline || tt == type::eos))
       fail (t) << "standalone attributes";
 
-    return &attrs_;
+    return (ha_ = true);
   }
 
   // Parse names inside {} and handle the following "crosses" (i.e.,
@@ -1556,7 +1689,7 @@ namespace build2
     return count;
   }
 
-  void parser::
+  bool parser::
   names (token& t, type& tt,
          names_type& ns,
          bool chunk,
@@ -1565,6 +1698,8 @@ namespace build2
          const dir_path* dp,
          const string* tp)
   {
+    bool null (false);
+
     // 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.,
     // a@{b c d{e f} {}}.
@@ -1769,13 +1904,26 @@ namespace build2
       //
       if (tt == type::dollar || tt == type::lparen)
       {
-        // These two cases are pretty similar in that in both we
-        // pretty quickly end up with a list of names that we need
-        // to splice into the result.
+        // These two cases are pretty similar in that in both we quickly end
+        // up with a list of names that we need to splice into the result.
         //
         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.
+        //
+        auto set_null = [first, &tt] ()
+        {
+          return first &&
+            tt != type::name &&
+            tt != type::dollar &&
+            tt != type::lparen &&
+            tt != type::lcbrace &&
+            tt != type::pair_separator;
+        };
+
         location loc;
         const char* what; // Variable or evaluation context.
 
@@ -1797,7 +1945,7 @@ namespace build2
           else if (tt == type::lparen)
           {
             expire_mode ();
-            names_type ns (eval (t, tt));
+            names_type ns (eval (t, tt).first);
 
             // Make sure the result of evaluation is a single, simple name.
             //
@@ -1821,14 +1969,22 @@ namespace build2
           if (tt == type::lparen)
           {
             next (t, tt); // Get '('.
-            names_type ns (eval (t, tt));
 
             // Just a stub for now.
             //
-            cout << n << "(" << ns << ")" << endl;
+            std::pair<names_type, bool> a (eval (t, tt));
+            cout << n << "(" << a.first << ")" << endl;
+
+            std::pair<names_type, bool> r (names_type (), false); // NULL.
+            lv_storage.swap (r.first);
 
             tt = peek ();
 
+            // See if we should propagate the NULL indicator.
+            //
+            if (set_null ())
+              null = !r.second;
+
             if (lv_storage.empty ())
               continue;
 
@@ -1851,12 +2007,22 @@ namespace build2
             const auto& var (var_pool.find (move (n)));
             auto l (target_ != nullptr ? (*target_)[var] : (*scope_)[var]);
 
-            // Undefined/NULL namespace variables are not allowed.
-            //
-            if (!l && var.name.find ('.') != string::npos)
-              fail (loc) << "undefined/null namespace variable " << var.name;
+            if (!l)
+            {
+              // Undefined/NULL namespace variables are not allowed.
+              //
+              if (var.name.find ('.') != string::npos)
+                fail (loc) << "undefined/null namespace variable " << var.name;
+
+              // See if we should set the NULL indicator.
+              //
+              if (set_null ())
+                null = true;
+
+              continue;
+            }
 
-            if (!l || l->empty ())
+            if (l->empty ())
               continue;
 
             lv = reverse (*l, lv_storage);
@@ -1866,10 +2032,17 @@ namespace build2
         else
         {
           loc = get_location (t, &path_);
-          lv_storage = eval (t, tt);
+
+          std::pair<names_type, bool> r (eval (t, tt));
+          lv_storage.swap (r.first);
 
           tt = peek ();
 
+          // See if we should propagate the NULL indicator.
+          //
+          if (set_null ())
+            null = !r.second;
+
           if (lv_storage.empty ())
             continue;
 
@@ -2043,6 +2216,9 @@ namespace build2
         continue;
       }
 
+      // Note: remember to update set_null test if adding new recognized
+      // tokens.
+
       if (!first)
         break;
 
@@ -2075,6 +2251,8 @@ namespace build2
                        (tp != nullptr ? *tp : string ()),
                        string ());
     }
+
+    return !null;
   }
 
   void parser::
diff --git a/build2/scope b/build2/scope
index 0e1bae5..566466a 100644
--- a/build2/scope
+++ b/build2/scope
@@ -141,7 +141,7 @@ namespace build2
     pair<lookup, size_t>
     find_original (
       const variable&,
-      const target_type* tt, const string* tn,
+      const target_type* tt = nullptr, const string* tn = nullptr,
       const target_type* gt = nullptr, const string* gn = nullptr) const;
 
     pair<lookup, size_t>
diff --git a/build2/scope.cxx b/build2/scope.cxx
index b0bd109..b74d52a 100644
--- a/build2/scope.cxx
+++ b/build2/scope.cxx
@@ -307,15 +307,18 @@ namespace build2
   value& scope::
   append (const variable& var)
   {
-    auto l (operator[] (var));
+    // Note that here we want the original value without any overrides
+    // applied.
+    //
+    lookup l (find_original (var).first);
 
-    if (l && l.belongs (*this)) // Existing variable in this scope.
-      return const_cast<value&> (*l);
+    if (l.defined () && l.belongs (*this)) // Existing var in this scope.
+      return const_cast<value&> (*l); // Ok since this is original.
 
-    value& r (assign (var));
+    value& r (assign (var)); // NULL.
 
-    if (l)
-      r = *l; // Copy value from the outer scope.
+    if (l.defined ())
+      r = *l; // Copy value (and type) from the outer scope.
 
     return r;
   }
diff --git a/build2/target b/build2/target
index eb6154a..241f6ff 100644
--- a/build2/target
+++ b/build2/target
@@ -288,7 +288,7 @@ namespace build2
     }
 
     // As above but also return the depth at which the value is found. The
-    // depth is calculated by adding 1 for each test performed.  So a value
+    // depth is calculated by adding 1 for each test performed. So a value
     // that is from the target will have depth 1. That from the group -- 2.
     // From the innermost scope's target type/patter-specific variables --
     // 3. From the innermost scope's variables -- 4. And so on.  The idea is
@@ -296,14 +296,21 @@ namespace build2
     // earlier. If no value is found, then the depth is set to ~0.
     //
     pair<lookup, size_t>
-    find (const variable&) const;
+    find (const variable& var) const
+    {
+      auto p (find_original (var));
+      return var.override == nullptr
+        ? p
+        : base_scope ().find_override (var, move (p), true);
+    }
 
     pair<lookup, size_t>
     find (const string& name) const {return find (var_pool.find (name));}
 
+    pair<lookup, size_t>
+    find_original (const variable&) const;
 
-    // Return a value suitable for assignment. See class scope for
-    // details.
+    // Return a value suitable for assignment. See scope for details.
     //
     value&
     assign (const variable& var) {return vars.assign (var).first;}
diff --git a/build2/target.cxx b/build2/target.cxx
index c9e6fde..782a7dd 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -115,12 +115,10 @@ namespace build2
   }
 
   pair<lookup, size_t> target::
-  find (const variable& var) const
+  find_original (const variable& var) const
   {
     pair<lookup, size_t> r (lookup (), 0);
 
-    scope& s (base_scope ());
-
     ++r.second;
     if (auto p = vars.find (var))
       r.first = lookup (p, &vars);
@@ -135,37 +133,39 @@ namespace build2
       }
     }
 
-    // Delegate to scope's find().
+    // Delegate to scope's find_original().
     //
     if (!r.first)
     {
-      auto p (s.find_original (var,
-                               &type (),
-                               &name,
-                               group != nullptr ? &group->type () : nullptr,
-                               group != nullptr ? &group->name : nullptr));
+      auto p (base_scope ().find_original (
+                var,
+                &type (),
+                &name,
+                group != nullptr ? &group->type () : nullptr,
+                group != nullptr ? &group->name : nullptr));
 
       r.first = move (p.first);
       r.second = r.first ? r.second + p.second : p.second;
     }
 
-    return var.override == nullptr
-      ? r
-      : s.find_override (var, move (r), true);
+    return r;
   }
 
   value& target::
   append (const variable& var)
   {
-    auto l (operator[] (var));
+    // Note that here we want the original value without any overrides
+    // applied.
+    //
+    lookup l (find_original (var).first);
 
-    if (l && l.belongs (*this)) // Existing variable in this target.
-      return const_cast<value&> (*l);
+    if (l.defined () && l.belongs (*this)) // Existing var in this target.
+      return const_cast<value&> (*l); // Ok since this is original.
 
-    value& r (assign (var));
+    value& r (assign (var)); // NULL.
 
-    if (l)
-      r = *l; // Copy value from the outer scope.
+    if (l.defined ())
+      r = *l; // Copy value (and type) from the outer scope.
 
     return r;
   }
diff --git a/build2/variable b/build2/variable
index 48acfea..10a0f59 100644
--- a/build2/variable
+++ b/build2/variable
@@ -120,14 +120,16 @@ namespace build2
     //
     //
   public:
-    ~value () {if (!null ()) *this = nullptr;}
+    ~value () {*this = nullptr;}
 
     explicit
     value (const value_type* t = nullptr)
         : type (t), state (value_state::null) {}
 
+    // Note: preserves type.
+    //
     value&
-    operator= (nullptr_t);
+    operator= (nullptr_t) {if (!null ()) reset (); return *this;}
 
     value (value&&);
     explicit value (const value&);
@@ -185,6 +187,10 @@ namespace build2
     // Make sure we have sufficient storage for untyped values.
     //
     static_assert (sizeof (names) <= size_, "insufficient space");
+
+  private:
+    void
+    reset ();
   };
 
   // The values should be of the same type (or both be untyped). NULL values
diff --git a/build2/variable.cxx b/build2/variable.cxx
index 2c2dbc2..a8e3c77 100644
--- a/build2/variable.cxx
+++ b/build2/variable.cxx
@@ -14,20 +14,15 @@ namespace build2
 {
   // value
   //
-  value& value::
-  operator= (nullptr_t)
+  void value::
+  reset ()
   {
-    if (!null ())
-    {
-      if (type == nullptr)
-        as<names> ().~names ();
-      else if (type->dtor != nullptr)
-        type->dtor (*this);
-
-      state = value_state::null;
-    }
+    if (type == nullptr)
+      as<names> ().~names ();
+    else if (type->dtor != nullptr)
+      type->dtor (*this);
 
-    return *this;
+    state = value_state::null;
   }
 
   value::
@@ -37,7 +32,7 @@ namespace build2
     if (!null ())
     {
       if (type == nullptr)
-        as<names> () = move (v).as<names> ();
+        new (&data_) names (move (v).as<names> ());
       else if (type->copy_ctor != nullptr)
         type->copy_ctor (*this, v, true);
       else
@@ -52,7 +47,7 @@ namespace build2
     if (!null ())
     {
       if (type == nullptr)
-        as<names> () = v.as<names> ();
+        new (&data_) names (v.as<names> ());
       else if (type->copy_ctor != nullptr)
         type->copy_ctor (*this, v, false);
       else
@@ -71,9 +66,7 @@ namespace build2
       //
       if (type == nullptr && v.type != nullptr)
       {
-        if (!null ())
-          *this = nullptr;
-
+        *this = nullptr;
         type = v.type;
       }
 
@@ -81,7 +74,12 @@ namespace build2
       // copy_ctor() instead of copy_assign().
       //
       if (type == nullptr)
-        as<names> () = move (v).as<names> ();
+      {
+        if (null ())
+          new (&data_) names (move (v).as<names> ());
+        else
+          as<names> () = move (v).as<names> ();
+      }
       else if (auto f = null () ? type->copy_ctor : type->copy_assign)
         f (*this, v, true);
       else
@@ -104,12 +102,7 @@ namespace build2
       //
       if (type == nullptr && v.type != nullptr)
       {
-        if (!null ())
-        {
-          reinterpret_cast<names&> (data_).~names ();
-          state = value_state::null;
-        }
-
+        *this = nullptr;
         type = v.type;
       }
 
@@ -117,7 +110,12 @@ namespace build2
       // copy_ctor() instead of copy_assign().
       //
       if (type == nullptr)
-        as<names> () = v.as<names> ();
+      {
+        if (null ())
+          new (&data_) names (v.as<names> ());
+        else
+          as<names> () = v.as<names> ();
+      }
       else if (auto f = null () ? type->copy_ctor : type->copy_assign)
         f (*this, v, false);
       else
diff --git a/build2/variable.ixx b/build2/variable.ixx
index ee0f647..821d224 100644
--- a/build2/variable.ixx
+++ b/build2/variable.ixx
@@ -30,9 +30,7 @@ namespace build2
     //
     if (type == nullptr)
     {
-      if (!null ())
-        *this = nullptr;
-
+      *this = nullptr;
       type = &value_traits<T>::value_type;
     }