From de15b95d09d00821aa23e96a0c3e827689c27a58 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Wed, 20 Jul 2016 16:18:29 +0200
Subject: Switch to dynamic empty() implementation in variable value

The current model fell apart when we modified values directly.
---
 build2/cxx/link.cxx |   2 +-
 build2/dump.cxx     |   6 +-
 build2/file.cxx     |   2 +-
 build2/parser.cxx   |  36 ++---
 build2/variable     | 121 ++++++++++-------
 build2/variable.cxx | 169 ++++++++++--------------
 build2/variable.ixx | 370 +++++++++++++++++++++-------------------------------
 build2/variable.txx |  77 ++++++-----
 8 files changed, 358 insertions(+), 425 deletions(-)

diff --git a/build2/cxx/link.cxx b/build2/cxx/link.cxx
index 88181aa..ba08119 100644
--- a/build2/cxx/link.cxx
+++ b/build2/cxx/link.cxx
@@ -1210,7 +1210,7 @@ namespace build2
       {
         ranlib = rs["config.bin.ranlib"];
 
-        if (ranlib->empty ()) // @@ BC LT [null].
+        if (ranlib && ranlib->empty ()) // @@ BC LT [null].
           ranlib = lookup ();
 
         const char* rl (
diff --git a/build2/dump.cxx b/build2/dump.cxx
index 08700fc..7cb97ef 100644
--- a/build2/dump.cxx
+++ b/build2/dump.cxx
@@ -22,7 +22,7 @@ namespace build2
   {
     // First print attributes if any.
     //
-    bool a (v.null () || (type && v.type != nullptr));
+    bool a (!v || (type && v.type != nullptr));
 
     if (a)
       os << '[';
@@ -35,7 +35,7 @@ namespace build2
       s = " ";
     }
 
-    if (v.null ())
+    if (!v)
     {
       os << s << "null";
       s = " ";
@@ -46,7 +46,7 @@ namespace build2
 
     // Now the value if there is one.
     //
-    if (!v.null ())
+    if (v)
     {
       names storage;
       os << (a ? " " : "") << reverse (v, storage);
diff --git a/build2/file.cxx b/build2/file.cxx
index 9b3be0c..9ab01ce 100644
--- a/build2/file.cxx
+++ b/build2/file.cxx
@@ -659,7 +659,7 @@ namespace build2
     // never set it to NULL).
     //
     auto l (root.vars["subprojects"]);
-    return l.defined () && (l->null () || l->type != nullptr);
+    return l.defined () && (l->null || l->type != nullptr);
   }
 
   void
diff --git a/build2/parser.cxx b/build2/parser.cxx
index 6b6c000..1987945 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -1093,7 +1093,7 @@ namespace build2
     else
       attributes_pop ();
 
-    if (rhs.null ())
+    if (!rhs)
       fail (t) << "null value in export";
 
     export_value_ = move (rhs).as<names_type> ();
@@ -1342,22 +1342,22 @@ namespace build2
       value tv;
       value_attributes (nullptr, tv, move (v), type::assign);
 
-      if (tv.null ())
-        cout << "[null]" << endl;
-      else
+      if (tv)
       {
         names_type storage;
         cout << reverse (tv, storage) << endl;
       }
+      else
+        cout << "[null]" << endl;
     }
     else
     {
       attributes_pop ();
 
-      if (v.null ())
-        cout << "[null]" << endl;
-      else
+      if (v)
         cout << v.as<names_type> () << endl;
+      else
+        cout << "[null]" << endl;
     }
 
     if (tt != type::eos)
@@ -1490,7 +1490,7 @@ namespace build2
 
       if (k == "null")
       {
-        if (!rhs.empty ()) // Note: null means we had an expansion.
+        if (rhs && !rhs.empty ()) // Note: null means we had an expansion.
           fail (l) << "value with null attribute";
 
         null = true;
@@ -1637,7 +1637,7 @@ namespace build2
           // 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 ())
+          if (lhs.type != nullptr || !lhs || lhs.empty ())
             fail (l) << "scope/target expected before ':'";
 
           names_type& ns (lhs.as<names_type> ());
@@ -1647,7 +1647,7 @@ namespace build2
           value rhs (eval_trailer (t, tt));
 
           if (tt != type::rparen ||
-              rhs.type != nullptr || rhs.null () || rhs.empty ())
+              rhs.type != nullptr || !rhs || rhs.empty ())
             fail (l) << "variable name expected after ':'";
 
           ns.insert (ns.end (),
@@ -2202,7 +2202,7 @@ namespace build2
             expire_mode ();
             value v (eval (t, tt));
 
-            if (v.null ())
+            if (!v)
               fail (loc) << "null variable/function name";
 
             names_type storage;
@@ -2251,10 +2251,14 @@ namespace build2
             value a (eval (t, tt));
             cout << name << "(";
 
-            if (a.null ())
+            if (a)
+            {
+              if (!a.empty ())
+                cout << reverse (a, lv_storage);
+            }
+            else
               cout << "[null]";
-            else if (!a.empty ())
-              cout << reverse (a, lv_storage);
+
 
             cout << ")" << endl;
 
@@ -2264,7 +2268,7 @@ namespace build2
 
             // See if we should propagate the NULL indicator.
             //
-            if (result.null ())
+            if (!result)
             {
               if (set_null ())
                 null = true;
@@ -2335,7 +2339,7 @@ namespace build2
 
           // See if we should propagate the NULL indicator.
           //
-          if (result.null ())
+          if (!result)
           {
             if (set_null ())
               null = true;
diff --git a/build2/variable b/build2/variable
index 9473674..08d0020 100644
--- a/build2/variable
+++ b/build2/variable
@@ -54,9 +54,9 @@ namespace build2
     // 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*);
+    void (*const assign) (value&, names&&, const variable*);
+    void (*const append) (value&, names&&, const variable*);
+    void (*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.
@@ -72,6 +72,10 @@ namespace build2
     // If NULL, then the types are compared as PODs using memcmp().
     //
     int (*const compare) (const value&, const value&);
+
+    // If NULL, then the value is never empty.
+    //
+    bool (*const empty) (const value&);
   };
 
   enum class variable_visibility
@@ -117,29 +121,33 @@ namespace build2
 
   typedef reference_wrapper<const variable> variable_cref;
 
-  // value
   //
-  enum class value_state: uint8_t {null, empty, filled}; // Order is important.
-
+  //
   class value
   {
   public:
     const value_type* type; // NULL means this value is not (yet) typed.
-    value_state state;
+    bool null;
 
     // Extra data that is associated with the value that can be used to store
     // flags, etc. It is initialized to 0 and copied (but not assigned) from
     // one value to another but is otherwise untouched (not even when the
     // value is reset to NULL).
     //
+    // Note: if deciding to use for something make sure it is not overlapping
+    // with an existing usage.
+    //
     uint16_t extra;
 
-    bool null () const {return state == value_state::null;}
-    bool empty () const {return state == value_state::empty;}
+    explicit operator bool () const {return !null;}
+    bool operator== (nullptr_t) const {return null;}
+    bool operator!= (nullptr_t) const {return !null;}
 
-    explicit operator bool () const {return !null ();}
-    bool operator== (nullptr_t) const {return null ();}
-    bool operator!= (nullptr_t) const {return !null ();}
+    // Check in a type-independent way if the value is empty. The value must
+    // not be NULL.
+    //
+    bool
+    empty () const;
 
     // 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
@@ -151,8 +159,7 @@ namespace build2
     ~value () {*this = nullptr;}
 
     explicit
-    value (const value_type* t = nullptr)
-        : type (t), state (value_state::null), extra (0) {}
+    value (const value_type* t = nullptr): type (t), null (true), extra (0) {}
 
     explicit
     value (names&&); // Create untyped value.
@@ -160,7 +167,7 @@ namespace build2
     // Note: preserves type.
     //
     value&
-    operator= (nullptr_t) {if (!null ()) reset (); return *this;}
+    operator= (nullptr_t) {if (!null) reset (); return *this;}
 
     value (value&&);
     explicit value (const value&);
@@ -302,7 +309,7 @@ namespace build2
 
     // Note: returns true if defined and not NULL.
     //
-    explicit operator bool () const {return defined () && !value->null ();}
+    explicit operator bool () const {return defined () && !value->null;}
 
     const value_type& operator*  () const {return *value;}
     const value_type* operator-> () const {return value;}
@@ -364,12 +371,12 @@ namespace build2
   //   //
   //   static T convert (name&&, name* rhs);
   //
-  //   // Assign/append/prepend T to value and return true if the result is
-  //   // not empty. Value is already of type T but can be NULL.
+  //   // Assign/append/prepend T to value which is already of type T but can
+  //   // be NULL.
   //   //
-  //   static bool assign (value&, T&&);
-  //   static bool append (value&, T&&);
-  //   static bool prepend (value&, T&&);
+  //   static void assign (value&, T&&);
+  //   static void append (value&, T&&);
+  //   static void prepend (value&, T&&);
   //
   //   // Reverse a value back to name. Only needs to be provided by simple
   //   // types.
@@ -380,6 +387,10 @@ namespace build2
   //   //
   //   static int compare (const T&, const T&);
   //
+  //   // Return true if the value is empty.
+  //   //
+  //   static bool empty (const T&);
+  //
   //   // For simple types (those that can be used as elements of containers),
   //   // type_name must be constexpr in order to sidestep the static init
   //   // order issue (in fact, that's the only reason we have it both here
@@ -414,6 +425,13 @@ namespace build2
   static void
   default_copy_assign (value&, const value&, bool);
 
+  // Default implementations of the empty callback that calls
+  // value_traits<T>::empty().
+  //
+  template <typename T>
+  static bool
+  default_empty (const value&);
+
   // Default implementations of the assign/append/prepend callbacks for simple
   // types. They call value_traits<T>::convert() and then pass the result to
   // value_traits<T>::assign()/append()/prepend(). As a result, it may not be
@@ -422,15 +440,15 @@ namespace build2
   // if false, then an empty value is not allowed.
   //
   template <typename T, bool empty>
-  static bool
+  static void
   simple_assign (value&, names&&, const variable*);
 
   template <typename T, bool empty>
-  static bool
+  static void
   simple_append (value&, names&&, const variable*);
 
   template <typename T, bool empty>
-  static bool
+  static void
   simple_prepend (value&, names&&, const variable*);
 
   // Default implementations of the reverse callback for simple types that
@@ -456,8 +474,8 @@ namespace build2
     static_assert (sizeof (bool) <= value::size_, "insufficient space");
 
     static bool convert (name&&, name*);
-    static bool assign (value&, bool);
-    static bool append (value&, bool); // OR.
+    static void assign (value&, bool);
+    static void append (value&, bool); // OR.
     static name reverse (bool x) {return name (x ? "true" : "false");}
     static int compare (bool, bool);
 
@@ -471,8 +489,8 @@ namespace build2
     static_assert (sizeof (uint64_t) <= value::size_, "insufficient space");
 
     static uint64_t convert (name&&, name*);
-    static bool assign (value&, uint64_t);
-    static bool append (value&, uint64_t); // ADD.
+    static void assign (value&, uint64_t);
+    static void append (value&, uint64_t); // ADD.
     static name reverse (uint64_t x) {return name (to_string (x));}
     static int compare (uint64_t, uint64_t);
 
@@ -488,11 +506,12 @@ namespace build2
     static_assert (sizeof (string) <= value::size_, "insufficient space");
 
     static string convert (name&&, name*);
-    static bool assign (value&, string&&);
-    static bool append (value&, string&&);
-    static bool prepend (value&, string&&);
+    static void assign (value&, string&&);
+    static void append (value&, string&&);
+    static void prepend (value&, string&&);
     static name reverse (const string& x) {return name (x);}
     static int compare (const string&, const string&);
+    static bool empty (const string& x) {return x.empty ();}
 
     static const char* const type_name;
     static const build2::value_type value_type;
@@ -506,11 +525,12 @@ namespace build2
     static_assert (sizeof (path) <= value::size_, "insufficient space");
 
     static path convert (name&&, name*);
-    static bool assign (value&, path&&);
-    static bool append (value&, path&&);  // operator/
-    static bool prepend (value&, path&&); // operator/
+    static void assign (value&, path&&);
+    static void append (value&, path&&);  // operator/
+    static void prepend (value&, path&&); // operator/
     static name reverse (const path& x) {return name (x.string ());}
     static int compare (const path&, const path&);
+    static bool empty (const path& x) {return x.empty ();}
 
     static const char* const type_name;
     static const build2::value_type value_type;
@@ -524,11 +544,12 @@ namespace build2
     static_assert (sizeof (dir_path) <= value::size_, "insufficient space");
 
     static dir_path convert (name&&, name*);
-    static bool assign (value&, dir_path&&);
-    static bool append (value&, dir_path&&);  // operator/
-    static bool prepend (value&, dir_path&&); // operator/
+    static void assign (value&, dir_path&&);
+    static void append (value&, dir_path&&);  // operator/
+    static void prepend (value&, dir_path&&); // operator/
     static name reverse (const dir_path& x) {return name (x);}
     static int compare (const dir_path&, const dir_path&);
+    static bool empty (const dir_path& x) {return x.empty ();}
 
     static const char* const type_name;
     static const build2::value_type value_type;
@@ -543,10 +564,11 @@ namespace build2
                    "insufficient space");
 
     static abs_dir_path convert (name&&, name*);
-    static bool assign (value&, abs_dir_path&&);
-    static bool append (value&, abs_dir_path&&);  // operator/
+    static void assign (value&, abs_dir_path&&);
+    static void append (value&, abs_dir_path&&);  // operator/
     static name reverse (const abs_dir_path& x) {return name (x);}
     static int compare (const abs_dir_path&, const abs_dir_path&);
+    static bool empty (const abs_dir_path& x) {return x.empty ();}
 
     static const char* const type_name;
     static const build2::value_type value_type;
@@ -560,11 +582,12 @@ namespace build2
     static_assert (sizeof (name) <= value::size_, "insufficient space");
 
     static name convert (name&&, name*);
-    static bool assign (value&, name&&);
-    static bool append (value&, name&&);
-    static bool prepend (value&, name&&);
+    static void assign (value&, name&&);
+    static void append (value&, name&&);
+    static void prepend (value&, name&&);
     static name reverse (const name& x) {return x;}
     static int compare (const name&, const name&);
+    static bool empty (const name& x) {return x.empty ();}
 
     static const char* const type_name;
     static const build2::value_type value_type;
@@ -577,9 +600,10 @@ namespace build2
   {
     static_assert (sizeof (vector<T>) <= value::size_, "insufficient space");
 
-    static bool assign (value&, vector<T>&&);
-    static bool append (value&, vector<T>&&);
-    static bool prepend (value&, vector<T>&&);
+    static void assign (value&, vector<T>&&);
+    static void append (value&, vector<T>&&);
+    static void prepend (value&, vector<T>&&);
+    static bool empty (const vector<T>& x) {return x.empty ();}
 
     static const string type_name;
     static const build2::value_type value_type;
@@ -594,10 +618,11 @@ namespace build2
 
     static_assert (sizeof (map<K, V>) <= value::size_, "insufficient space");
 
-    static bool assign (value&, map<K, V>&&);
-    static bool append (value&, map<K, V>&&);
-    static bool prepend (value& v, map<K, V>&& x) {
+    static void assign (value&, map<K, V>&&);
+    static void append (value&, map<K, V>&&);
+    static void prepend (value& v, map<K, V>&& x) {
       return append (v, move (x));}
+    static bool empty (const map<K, V>& x) {return x.empty ();}
 
     static const string type_name;
     static const build2::value_type value_type;
diff --git a/build2/variable.cxx b/build2/variable.cxx
index 78f9df8..7a333bb 100644
--- a/build2/variable.cxx
+++ b/build2/variable.cxx
@@ -22,14 +22,14 @@ namespace build2
     else if (type->dtor != nullptr)
       type->dtor (*this);
 
-    state = value_state::null;
+    null = true;
   }
 
   value::
   value (value&& v)
-      : type (v.type), state (v.state), extra (v.extra)
+      : type (v.type), null (v.null), extra (v.extra)
   {
-    if (!null ())
+    if (!null)
     {
       if (type == nullptr)
         new (&data_) names (move (v).as<names> ());
@@ -42,9 +42,9 @@ namespace build2
 
   value::
   value (const value& v)
-      : type (v.type), state (v.state), extra (v.extra)
+      : type (v.type), null (v.null), extra (v.extra)
   {
-    if (!null ())
+    if (!null)
     {
       if (type == nullptr)
         new (&data_) names (v.as<names> ());
@@ -73,21 +73,21 @@ namespace build2
       // Now our types are the same. If the receiving value is NULL, then call
       // copy_ctor() instead of copy_assign().
       //
-      if (!v.null ())
+      if (v)
       {
         if (type == nullptr)
         {
-          if (null ())
+          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)
+        else if (auto f = null ? type->copy_ctor : type->copy_assign)
           f (*this, v, true);
         else
           data_ = v.data_; // Assign as POD.
 
-        state = v.state;
+        null = v.null;
       }
       else
         *this = nullptr;
@@ -114,21 +114,21 @@ namespace build2
       // Now our types are the same. If the receiving value is NULL, then call
       // copy_ctor() instead of copy_assign().
       //
-      if (!v.null ())
+      if (v)
       {
         if (type == nullptr)
         {
-          if (null ())
+          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)
+        else if (auto f = null ? type->copy_ctor : type->copy_assign)
           f (*this, v, false);
         else
           data_ = v.data_; // Assign as POD.
 
-        state = v.state;
+        null = v.null;
       }
       else
         *this = nullptr;
@@ -142,54 +142,39 @@ namespace build2
   {
     assert (type == nullptr || type->assign != nullptr);
 
-    bool r;
-
     if (type == nullptr)
     {
-      names* p;
-
-      if (null ())
-        p = new (&data_) names (move (ns));
+      if (null)
+        new (&data_) names (move (ns));
       else
-      {
-        p = &as<names> ();
-        *p = move (ns);
-      }
-
-      r = !p->empty ();
+        as<names> () = move (ns);
     }
     else
-      r = type->assign (*this, move (ns), var);
+      type->assign (*this, move (ns), var);
 
-    state = r ? value_state::filled : value_state::empty;
+    null = false;
   }
 
   void value::
   append (names&& ns, const variable* var)
   {
-    bool r;
-
     if (type == nullptr)
     {
-      names* p;
-
-      if (null ())
-        p = new (&data_) names (move (ns));
+      if (null)
+        new (&data_) names (move (ns));
       else
       {
-        p = &as<names> ();
+        names& p (as<names> ());
 
-        if (p->empty ())
-          *p = move (ns);
+        if (p.empty ())
+          p = move (ns);
         else if (!ns.empty ())
         {
-          p->insert (p->end (),
-                     make_move_iterator (ns.begin ()),
-                     make_move_iterator (ns.end ()));
+          p.insert (p.end (),
+                    make_move_iterator (ns.begin ()),
+                    make_move_iterator (ns.end ()));
         }
       }
-
-      r = !p->empty ();
     }
     else
     {
@@ -203,39 +188,33 @@ namespace build2
           dr << " in variable " << var->name;
       }
 
-      r = type->append (*this, move (ns), var);
+      type->append (*this, move (ns), var);
     }
 
-    state = r ? value_state::filled : value_state::empty;
+    null = false;
   }
 
   void value::
   prepend (names&& ns, const variable* var)
   {
-    bool r;
-
     if (type == nullptr)
     {
-      names* p;
-
-      if (null ())
-        p = new (&data_) names (move (ns));
+      if (null)
+        new (&data_) names (move (ns));
       else
       {
-        p = &as<names> ();
+        names& p (as<names> ());
 
-        if (p->empty ())
-          *p = move (ns);
+        if (p.empty ())
+          p = move (ns);
         else if (!ns.empty ())
         {
           ns.insert (ns.end (),
-                     make_move_iterator (p->begin ()),
-                     make_move_iterator (p->end ()));
-          p->swap (ns);
+                     make_move_iterator (p.begin ()),
+                     make_move_iterator (p.end ()));
+          p.swap (ns);
         }
       }
-
-      r = !p->empty ();
     }
     else
     {
@@ -249,27 +228,24 @@ namespace build2
           dr << " in variable " << var->name;
       }
 
-      r = type->prepend (*this, move (ns), var);
+      type->prepend (*this, move (ns), var);
     }
 
-    state = r ? value_state::filled : value_state::empty;
+    null = false;
   }
 
   bool
   operator== (const value& x, const value& y)
   {
-    bool xn (x.null ());
-    bool yn (y.null ());
+    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 (xn)
-      return true; // Both are NULL since same state.
+    if (xn || yn)
+      return xn == yn;
 
     if (x.type == nullptr)
       return x.as<names> () == y.as<names> ();
@@ -283,25 +259,18 @@ namespace build2
   bool
   operator< (const value& x, const value& y)
   {
-    bool xn (x.null ());
-    bool yn (y.null ());
+    bool xn (x.null);
+    bool yn (y.null);
 
     assert (x.type == y.type ||
             (xn && x.type == nullptr) ||
             (yn && y.type == nullptr));
 
-    // NULL value is always less than non-NULL and we assume that empty
-    // value is always less than non-empty.
-    //
-    if (x.state < y.state)
-      return true;
-    else if (x.state > y.state)
-      return false;
-    else if (x.state != value_state::filled) // Both are NULL or empty.
-      return false;
-
-    // Both are filled.
+    // NULL value is always less than non-NULL.
     //
+    if (xn || yn)
+      return xn > yn; // !xn < !yn
+
     if (x.type == nullptr)
       return x.as<names> () < y.as<names> ();
 
@@ -314,25 +283,18 @@ namespace build2
   bool
   operator> (const value& x, const value& y)
   {
-    bool xn (x.null ());
-    bool yn (y.null ());
+    bool xn (x.null);
+    bool yn (y.null);
 
     assert (x.type == y.type ||
             (xn && x.type == nullptr) ||
             (yn && y.type == nullptr));
 
-    // NULL value is always less than non-NULL and we assume that empty
-    // value is always less than non-empty.
-    //
-    if (x.state > y.state)
-      return true;
-    else if (x.state < y.state)
-      return false;
-    else if (x.state != value_state::filled) // Both are NULL or empty.
-      return false;
-
-    // Both are filled.
+    // NULL value is always less than non-NULL.
     //
+    if (xn || yn)
+      return xn < yn; // !xn > !yn
+
     if (x.type == nullptr)
       return x.as<names> () > y.as<names> ();
 
@@ -347,7 +309,7 @@ namespace build2
   {
     if (v.type == nullptr)
     {
-      if (!v.null ())
+      if (v)
       {
         // Note: the order in which we do things here is important.
         //
@@ -410,7 +372,8 @@ namespace build2
     &simple_append<bool, false>,  // Prepend same as append.
     &simple_reverse<bool>,
     nullptr,              // No cast (cast data_ directly).
-    nullptr               // No compare (compare as POD).
+    nullptr,              // No compare (compare as POD).
+    nullptr               // Never empty.
   };
 
   // uint64_t value
@@ -450,7 +413,8 @@ namespace build2
     &simple_append<uint64_t, false>,  // Prepend same as append.
     &simple_reverse<uint64_t>,
     nullptr,                          // No cast (cast data_ directly).
-    nullptr                           // No compare (compare as POD).
+    nullptr,                          // No compare (compare as POD).
+    nullptr                           // Never empty.
   };
 
   // string value
@@ -536,7 +500,8 @@ namespace build2
     &simple_prepend<string, true>,
     &simple_reverse<string>,
     nullptr,                      // No cast (cast data_ directly).
-    &simple_compare<string>
+    &simple_compare<string>,
+    &default_empty<string>
   };
 
   // path value
@@ -581,7 +546,8 @@ namespace build2
     &simple_prepend<path, true>,
     &simple_reverse<path>,
     nullptr,                    // No cast (cast data_ directly).
-    &simple_compare<path>
+    &simple_compare<path>,
+    &default_empty<path>
   };
 
   // dir_path value
@@ -624,7 +590,8 @@ namespace build2
     &simple_prepend<dir_path, true>,
     &simple_reverse<dir_path>,
     nullptr,                        // No cast (cast data_ directly).
-    &simple_compare<dir_path>
+    &simple_compare<dir_path>,
+    &default_empty<dir_path>
   };
 
   // abs_dir_path value
@@ -657,7 +624,8 @@ namespace build2
     nullptr,                             // No prepend.
     &simple_reverse<abs_dir_path>,
     nullptr,                             // No cast (cast data_ directly).
-    &simple_compare<abs_dir_path>
+    &simple_compare<abs_dir_path>,
+    &default_empty<abs_dir_path>
   };
 
   // name value
@@ -692,7 +660,8 @@ namespace build2
     nullptr,                    // Prepend not supported.
     &name_reverse,
     nullptr,                    // No cast (cast data_ directly).
-    &simple_compare<name>
+    &simple_compare<name>,
+    &default_empty<name>
   };
 
   // variable_pool
diff --git a/build2/variable.ixx b/build2/variable.ixx
index e7d5654..6b59c72 100644
--- a/build2/variable.ixx
+++ b/build2/variable.ixx
@@ -8,11 +8,17 @@ namespace build2
 {
   // value
   //
+  inline bool value::
+  empty () const
+  {
+    assert (!null);
+    return type == nullptr ? as<names> ().empty () :
+      type->empty == nullptr ? false : type->empty (*this);
+  }
+
   inline value::
   value (names&& ns)
-      : type (nullptr),
-        state (ns.empty () ? value_state::empty : value_state::filled),
-        extra (0)
+      : type (nullptr), null (false), extra (0)
   {
     new (&data_) names (move (ns));
   }
@@ -43,10 +49,8 @@ namespace build2
       type = &value_traits<T>::value_type;
     }
 
-    state = value_traits<T>::assign (*this, move (v))
-      ? value_state::filled
-      : value_state::empty;
-
+    value_traits<T>::assign (*this, move (v));
+    null = false;
     return *this;
   }
 
@@ -54,18 +58,15 @@ namespace build2
   inline value& value::
   operator+= (T v)
   {
-    assert (type == &value_traits<T>::value_type ||
-            (type == nullptr && null ()));
+    assert (type == &value_traits<T>::value_type || (type == nullptr && null));
 
     // Prepare the receiving value.
     //
     if (type == nullptr)
       type = &value_traits<T>::value_type;
 
-    state = value_traits<T>::append (*this, move (v))
-      ? value_state::filled
-      : value_state::empty;
-
+    value_traits<T>::append (*this, move (v));
+    null = false;
     return *this;
   }
 
@@ -94,7 +95,7 @@ namespace build2
   {
     // Note that it can still be a typed vector<names>.
     //
-    assert (!v.null () &&
+    assert (v &&
             (v.type == nullptr || v.type == &value_traits<names>::value_type));
     return v.as<names> ();
   }
@@ -103,7 +104,7 @@ namespace build2
   inline names&
   cast (value& v)
   {
-    assert (!v.null () &&
+    assert (v &&
             (v.type == nullptr || v.type == &value_traits<names>::value_type));
     return v.as<names> ();
   }
@@ -112,7 +113,7 @@ namespace build2
   inline const T&
   cast (const value& v)
   {
-    assert (!v.null ());
+    assert (v);
 
     // Find base if any.
     //
@@ -129,7 +130,7 @@ namespace build2
   inline T&
   cast (value& v)
   {
-    assert (!v.null () && v.type == &value_traits<T>::value_type);
+    assert (v && v.type == &value_traits<T>::value_type);
     return *static_cast<T*> (v.type->cast == nullptr
                              ? static_cast<void*> (&v.data_)
                              : const_cast<void*> (v.type->cast (v, v.type)));
@@ -191,7 +192,7 @@ namespace build2
   inline names_view
   reverse (const value& v, names& storage)
   {
-    assert (!v.null () &&
+    assert (v &&
             storage.empty () &&
             (v.type == nullptr || v.type->reverse != nullptr));
     return v.type == nullptr ? v.as<names> () : v.type->reverse (v, storage);
@@ -222,28 +223,25 @@ namespace build2
 
   // bool value
   //
-  inline bool value_traits<bool>::
+  inline void value_traits<bool>::
   assign (value& v, bool x)
   {
-    if (v.null ())
-      new (&v.data_) bool (x);
-    else
+    if (v)
       v.as<bool> () = x;
+    else
+      new (&v.data_) bool (x);
 
-    return true;
   }
 
-  inline bool value_traits<bool>::
+  inline void value_traits<bool>::
   append (value& v, bool x)
   {
     // Logical OR.
     //
-    if (v.null ())
-      new (&v.data_) bool (x);
-    else
+    if (v)
       v.as<bool> () = v.as<bool> () || x;
-
-    return true;
+    else
+      new (&v.data_) bool (x);
   }
 
   inline int value_traits<bool>::
@@ -254,28 +252,24 @@ namespace build2
 
   // uint64_t value
   //
-  inline bool value_traits<uint64_t>::
+  inline void value_traits<uint64_t>::
   assign (value& v, uint64_t x)
   {
-    if (v.null ())
-      new (&v.data_) uint64_t (x);
-    else
+    if (v)
       v.as<uint64_t> () = x;
-
-    return true;
+    else
+      new (&v.data_) uint64_t (x);
   }
 
-  inline bool value_traits<uint64_t>::
+  inline void value_traits<uint64_t>::
   append (value& v, uint64_t x)
   {
     // ADD.
     //
-    if (v.null ())
-      new (&v.data_) uint64_t (x);
-    else
+    if (v)
       v.as<uint64_t> () += x;
-
-    return true;
+    else
+      new (&v.data_) uint64_t (x);
   }
 
   inline int value_traits<uint64_t>::
@@ -286,57 +280,45 @@ namespace build2
 
   // string value
   //
-  inline bool value_traits<string>::
+  inline void value_traits<string>::
   assign (value& v, string&& x)
   {
-    string* p;
-
-    if (v.null ())
-      p = new (&v.data_) string (move (x));
+    if (v)
+      v.as<string> () = move (x);
     else
-      p = &(v.as<string> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) string (move (x));
   }
 
-  inline bool value_traits<string>::
+  inline void value_traits<string>::
   append (value& v, string&& x)
   {
-    string* p;
-
-    if (v.null ())
-      p = new (&v.data_) string (move (x));
-    else
+    if (v)
     {
-      p = &v.as<string> ();
+      string& s (v.as<string> ());
 
-      if (p->empty ())
-        p->swap (x);
+      if (s.empty ())
+        s.swap (x);
       else
-        *p += x;
+        s += x;
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) string (move (x));
   }
 
-  inline bool value_traits<string>::
+  inline void value_traits<string>::
   prepend (value& v, string&& x)
   {
-    string* p;
-
-    if (v.null ())
-      p = new (&v.data_) string (move (x));
-    else
+    if (v)
     {
-      p = &v.as<string> ();
+      string& s (v.as<string> ());
 
-      if (!p->empty ())
-        x += *p;
+      if (!s.empty ())
+        x += s;
 
-      p->swap (x);
+      s.swap (x);
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) string (move (x));
   }
 
   inline int value_traits<string>::
@@ -347,57 +329,45 @@ namespace build2
 
   // path value
   //
-  inline bool value_traits<path>::
+  inline void value_traits<path>::
   assign (value& v, path&& x)
   {
-    path* p;
-
-    if (v.null ())
-      p = new (&v.data_) path (move (x));
+    if (v)
+      v.as<path> () = move (x);
     else
-      p = &(v.as<path> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) path (move (x));
   }
 
-  inline bool value_traits<path>::
+  inline void value_traits<path>::
   append (value& v, path&& x)
   {
-    path* p;
-
-    if (v.null ())
-      p = new (&v.data_) path (move (x));
-    else
+    if (v)
     {
-      p = &v.as<path> ();
+      path& p (v.as<path> ());
 
-      if (p->empty ())
-        p->swap (x);
+      if (p.empty ())
+        p.swap (x);
       else
-        *p /= x;
+        p /= x;
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) path (move (x));
   }
 
-  inline bool value_traits<path>::
+  inline void value_traits<path>::
   prepend (value& v, path&& x)
   {
-    path* p;
-
-    if (v.null ())
-      p = new (&v.data_) path (move (x));
-    else
+    if (v)
     {
-      p = &v.as<path> ();
+      path& p (v.as<path> ());
 
-      if (!p->empty ())
-        x /= *p;
+      if (!p.empty ())
+        x /= p;
 
-      p->swap (x);
+      p.swap (x);
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) path (move (x));
   }
 
   inline int value_traits<path>::
@@ -408,57 +378,45 @@ namespace build2
 
   // dir_path value
   //
-  inline bool value_traits<dir_path>::
+  inline void value_traits<dir_path>::
   assign (value& v, dir_path&& x)
   {
-    dir_path* p;
-
-    if (v.null ())
-      p = new (&v.data_) dir_path (move (x));
+    if (v)
+      v.as<dir_path> () = move (x);
     else
-      p = &(v.as<dir_path> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) dir_path (move (x));
   }
 
-  inline bool value_traits<dir_path>::
+  inline void value_traits<dir_path>::
   append (value& v, dir_path&& x)
   {
-    dir_path* p;
-
-    if (v.null ())
-      p = new (&v.data_) dir_path (move (x));
-    else
+    if (v)
     {
-      p = &v.as<dir_path> ();
+      dir_path& p (v.as<dir_path> ());
 
-      if (p->empty ())
-        p->swap (x);
+      if (p.empty ())
+        p.swap (x);
       else
-        *p /= x;
+        p /= x;
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) dir_path (move (x));
   }
 
-  inline bool value_traits<dir_path>::
+  inline void value_traits<dir_path>::
   prepend (value& v, dir_path&& x)
   {
-    dir_path* p;
-
-    if (v.null ())
-      p = new (&v.data_) dir_path (move (x));
-    else
+    if (v)
     {
-      p = &v.as<dir_path> ();
+      dir_path& p (v.as<dir_path> ());
 
-      if (!p->empty ())
-        x /= *p;
+      if (!p.empty ())
+        x /= p;
 
-      p->swap (x);
+      p.swap (x);
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) dir_path (move (x));
   }
 
   inline int value_traits<dir_path>::
@@ -469,37 +427,29 @@ namespace build2
 
   // abs_dir_path value
   //
-  inline bool value_traits<abs_dir_path>::
+  inline void value_traits<abs_dir_path>::
   assign (value& v, abs_dir_path&& x)
   {
-    abs_dir_path* p;
-
-    if (v.null ())
-      p = new (&v.data_) abs_dir_path (move (x));
+    if (v)
+      v.as<abs_dir_path> () = move (x);
     else
-      p = &(v.as<abs_dir_path> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) abs_dir_path (move (x));
   }
 
-  inline bool value_traits<abs_dir_path>::
+  inline void value_traits<abs_dir_path>::
   append (value& v, abs_dir_path&& x)
   {
-    abs_dir_path* p;
-
-    if (v.null ())
-      p = new (&v.data_) abs_dir_path (move (x));
-    else
+    if (v)
     {
-      p = &v.as<abs_dir_path> ();
+      abs_dir_path& p (v.as<abs_dir_path> ());
 
-      if (p->empty ())
-        p->swap (x);
+      if (p.empty ())
+        p.swap (x);
       else
-        *p /= x;
+        p /= x;
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) abs_dir_path (move (x));
   }
 
   inline int value_traits<abs_dir_path>::
@@ -510,17 +460,13 @@ namespace build2
 
   // name value
   //
-  inline bool value_traits<name>::
+  inline void value_traits<name>::
   assign (value& v, name&& x)
   {
-    name* p;
-
-    if (v.null ())
-      p = new (&v.data_) name (move (x));
+    if (v)
+      v.as<name> () = move (x);
     else
-      p = &(v.as<name> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) name (move (x));
   }
 
   inline int value_traits<name>::
@@ -532,105 +478,85 @@ namespace build2
   // vector<T> value
   //
   template <typename T>
-  inline bool value_traits<vector<T>>::
+  inline void value_traits<vector<T>>::
   assign (value& v, vector<T>&& x)
   {
-    vector<T>* p;
-
-    if (v.null ())
-      p = new (&v.data_) vector<T> (move (x));
+    if (v)
+      v.as<vector<T>> () = move (x);
     else
-      p = &(v.as<vector<T>> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) vector<T> (move (x));
   }
 
   template <typename T>
-  inline bool value_traits<vector<T>>::
+  inline void value_traits<vector<T>>::
   append (value& v, vector<T>&& x)
   {
-    vector<T>* p;
-
-    if (v.null ())
-      p = new (&v.data_) vector<T> (move (x));
-    else
+    if (v)
     {
-      p = &v.as<vector<T>> ();
+      vector<T>& p (v.as<vector<T>> ());
 
-      if (p->empty ())
-        p->swap (x);
+      if (p.empty ())
+        p.swap (x);
       else
-        p->insert (p->end (),
-                   make_move_iterator (x.begin ()),
-                   make_move_iterator (x.end ()));
+        p.insert (p.end (),
+                  make_move_iterator (x.begin ()),
+                  make_move_iterator (x.end ()));
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) vector<T> (move (x));
   }
 
   template <typename T>
-  inline bool value_traits<vector<T>>::
+  inline void value_traits<vector<T>>::
   prepend (value& v, vector<T>&& x)
   {
-    vector<T>* p;
-
-    if (v.null ())
-      p = new (&v.data_) vector<T> (move (x));
-    else
+    if (v)
     {
-      p = &v.as<vector<T>> ();
+      vector<T>& p (v.as<vector<T>> ());
 
-      if (!p->empty ())
+      if (!p.empty ())
         x.insert (x.end (),
-                  make_move_iterator (p->begin ()),
-                  make_move_iterator (p->end ()));
+                  make_move_iterator (p.begin ()),
+                  make_move_iterator (p.end ()));
 
-      p->swap (x);
+      p.swap (x);
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) vector<T> (move (x));
   }
 
   // map<K, V> value
   //
   template <typename K, typename V>
-  inline bool value_traits<std::map<K, V>>::
+  inline void value_traits<std::map<K, V>>::
   assign (value& v, map<K, V>&& x)
   {
-    map<K, V>* p;
-
-    if (v.null ())
-      p = new (&v.data_) map<K, V> (move (x));
+    if (v)
+      v.as<map<K, V>> () = move (x);
     else
-      p = &(v.as<map<K, V>> () = move (x));
-
-    return !p->empty ();
+      new (&v.data_) map<K, V> (move (x));
   }
 
   template <typename K, typename V>
-  inline bool value_traits<std::map<K, V>>::
+  inline void value_traits<std::map<K, V>>::
   append (value& v, map<K, V>&& x)
   {
-    map<K, V>* p;
-
-    if (v.null ())
-      p = new (&v.data_) map<K, V> (move (x));
-    else
+    if (v)
     {
-      p = &v.as<map<K, V>> ();
+      map<K, V>& m (v.as<map<K, V>> ());
 
-      if (p->empty ())
-        p->swap (x);
+      if (m.empty ())
+        m.swap (x);
       else
         // Note that this will only move values. Keys (being const) are still
         // copied.
         //
-        p->insert (p->end (),
-                   make_move_iterator (x.begin ()),
-                   make_move_iterator (x.end ()));
+        m.insert (m.end (),
+                  make_move_iterator (x.begin ()),
+                  make_move_iterator (x.end ()));
     }
-
-    return !p->empty ();
+    else
+      new (&v.data_) map<K, V> (move (x));
   }
 
   // variable_pool
diff --git a/build2/variable.txx b/build2/variable.txx
index a86a936..f1335b4 100644
--- a/build2/variable.txx
+++ b/build2/variable.txx
@@ -35,8 +35,15 @@ namespace build2
       l.as<T> () = r.as<T> ();
   }
 
-  template <typename T, bool empty>
+  template <typename T>
   bool
+  default_empty (const value& v)
+  {
+    return value_traits<T>::empty (v.as<T> ());
+  }
+
+  template <typename T, bool empty>
+  void
   simple_assign (value& v, names&& ns, const variable* var)
   {
     size_t n (ns.size ());
@@ -45,11 +52,13 @@ namespace build2
     {
       try
       {
-        return value_traits<T>::assign (
+        value_traits<T>::assign (
           v,
           (n == 0
            ? T ()
            : value_traits<T>::convert (move (ns.front ()), nullptr)));
+
+        return;
       }
       catch (const invalid_argument&) {} // Fall through.
     }
@@ -68,7 +77,7 @@ namespace build2
   }
 
   template <typename T, bool empty>
-  bool
+  void
   simple_append (value& v, names&& ns, const variable* var)
   {
     size_t n (ns.size ());
@@ -77,11 +86,13 @@ namespace build2
     {
       try
       {
-        return value_traits<T>::append (
+        value_traits<T>::append (
           v,
           (n == 0
            ? T ()
            : value_traits<T>::convert (move (ns.front ()), nullptr)));
+
+        return;
       }
       catch (const invalid_argument&) {} // Fall through.
     }
@@ -100,7 +111,7 @@ namespace build2
   }
 
   template <typename T, bool empty>
-  bool
+  void
   simple_prepend (value& v, names&& ns, const variable* var)
   {
     size_t n (ns.size ());
@@ -109,11 +120,13 @@ namespace build2
     {
       try
       {
-        return value_traits<T>::prepend (
+        value_traits<T>::prepend (
           v,
           (n == 0
            ? T ()
            : value_traits<T>::convert (move (ns.front ()), nullptr)));
+
+        return;
       }
       catch (const invalid_argument&) {} // Fall through.
     }
@@ -150,12 +163,12 @@ namespace build2
   //
 
   template <typename T>
-  bool
+  void
   vector_append (value& v, names&& ns, const variable* var)
   {
-    vector<T>* p (v.null ()
-                  ? new (&v.data_) vector<T> ()
-                  : &v.as<vector<T>> ());
+    vector<T>& p (v
+                  ? v.as<vector<T>> ()
+                  : *new (&v.data_) vector<T> ());
 
     // Convert each element to T while merging pairs.
     //
@@ -183,7 +196,7 @@ namespace build2
 
       try
       {
-        p->push_back (value_traits<T>::convert (move (n), r));
+        p.push_back (value_traits<T>::convert (move (n), r));
       }
       catch (const invalid_argument&)
       {
@@ -200,22 +213,20 @@ namespace build2
           dr << " in variable " << var->name;
       }
     }
-
-    return !p->empty ();
   }
 
   template <typename T>
-  bool
+  void
   vector_assign (value& v, names&& ns, const variable* var)
   {
-    if (!v.null ())
+    if (v)
       v.as<vector<T>> ().clear ();
 
-    return vector_append<T> (v, move (ns), var);
+    vector_append<T> (v, move (ns), var);
   }
 
   template <typename T>
-  bool
+  void
   vector_prepend (value& v, names&& ns, const variable* var)
   {
     // Reduce to append.
@@ -223,21 +234,19 @@ namespace build2
     vector<T> t;
     vector<T>* p;
 
-    if (v.null ())
-      p = new (&v.data_) vector<T> ();
-    else
+    if (v)
     {
       p = &v.as<vector<T>> ();
       p->swap (t);
     }
+    else
+      p = new (&v.data_) vector<T> ();
 
     vector_append<T> (v, move (ns), var);
 
     p->insert (p->end (),
                make_move_iterator (t.begin ()),
                make_move_iterator (t.end ()));
-
-    return !p->empty ();
   }
 
   template <typename T>
@@ -294,20 +303,21 @@ namespace build2
     &vector_prepend<T>,
     &vector_reverse<T>,
     nullptr,                          // No cast (cast data_ directly).
-    &vector_compare<T>
+    &vector_compare<T>,
+    &default_empty<vector<T>>
   };
 
   // map<K, V> value
   //
   template <typename K, typename V>
-  bool
+  void
   map_append (value& v, names&& ns, const variable* var)
   {
     using std::map;
 
-    map<K, V>* p (v.null ()
-                  ? new (&v.data_) map<K, V> ()
-                  : &v.as<map<K, V>> ());
+    map<K, V>& p (v
+                  ? v.as<map<K, V>> ()
+                  : *new (&v.data_) map<K, V> ());
 
     // Verify we have a sequence of pairs and convert each lhs/rhs to K/V.
     //
@@ -348,7 +358,7 @@ namespace build2
         {
           V v (value_traits<V>::convert (move (r), nullptr));
 
-          p->emplace (move (k), move (v));
+          p.emplace (move (k), move (v));
         }
         catch (const invalid_argument&)
         {
@@ -372,20 +382,18 @@ namespace build2
           dr << " in variable " << var->name;
       }
     }
-
-    return !p->empty ();
   }
 
   template <typename K, typename V>
-  bool
+  void
   map_assign (value& v, names&& ns, const variable* var)
   {
     using std::map;
 
-    if (!v.null ())
+    if (v)
       v.as<map<K, V>> ().clear ();
 
-    return map_append<K, V> (v, move (ns), var);
+    map_append<K, V> (v, move (ns), var);
   }
 
   template <typename K, typename V>
@@ -455,6 +463,7 @@ namespace build2
     &map_append<K, V>,   // Prepend is the same as append.
     &map_reverse<K, V>,
     nullptr,             // No cast (cast data_ directly).
-    &map_compare<K, V>
+    &map_compare<K, V>,
+    &default_empty<map<K, V>>
   };
 }
-- 
cgit v1.1