Fixed a memory bug due to a shallow copy in value_t::storage_t.

parsexp.cc

@@ -492,18 +492,16 @@ parser_t::parse_value_term(std::istream& in, scope_t& scope, const flags_t tflag
     break;
   }
 
+#if 0
   case token_t::DOLLAR:
     tok = next_token(in, tflags);
     if (tok.kind != token_t::IDENT)
       throw parse_error("$ symbol must be followed by variable name");
 
-#if 0
     node = new op_t(op_t::VAR_NAME);
     node->set_string(tok.value.as_string());
-#endif
     break;
 
-#if 0
   case token_t::DOT:
     node = new op_t(op_t::NODE_ID);
     node->set_name(document_t::CURRENT);

valexpr.cc

@@ -56,8 +56,11 @@ bool compute_amount(ptr_op_t expr, amount_t& amt,
   value_t result;
   try {
     expr->compute(result, xact ? details_t(*xact) : details_t(), context);
-    result.cast(value_t::AMOUNT);
+
-    amt = result.as_amount_lval();
+    // Most of the time when computing the amount of a transaction this cast
+    // will do nothing at all.
+    result.in_place_cast(value_t::AMOUNT);
+    amt = result.as_amount();
   }
   catch (error * err) {
     if (err->context.empty() ||

value.cc

@@ -36,6 +36,45 @@ namespace ledger {
 intrusive_ptr<value_t::storage_t> value_t::true_value;
 intrusive_ptr<value_t::storage_t> value_t::false_value;
 
+value_t::storage_t& value_t::storage_t::operator=(const value_t::storage_t& rhs)
+{
+  type = rhs.type;
+
+  switch (type) {
+  case DATETIME:
+    new((datetime_t *) data) datetime_t(*(datetime_t *) rhs.data);
+    break;
+
+  case AMOUNT:
+    new((amount_t *) data) amount_t(*(amount_t *) rhs.data);
+    break;
+
+  case BALANCE:
+    *(balance_t **) data = new balance_t(**(balance_t **) rhs.data);
+    break;
+
+  case BALANCE_PAIR:
+    *(balance_pair_t **) data =
+      new balance_pair_t(**(balance_pair_t **) rhs.data);
+    break;
+
+  case STRING:
+    new((string *) data) string(*(string *) rhs.data);
+    break;
+
+  case SEQUENCE:
+    *(sequence_t **) data = new sequence_t(**(sequence_t **) rhs.data);
+    break;
+
+  default:
+    // The rest are fundamental types, which can copy using std::memcpy
+    std::memcpy(data, rhs.data, sizeof(data));
+    break;
+  }
+
+  return *this;
+}
+
 void value_t::storage_t::destroy()
 {
   switch (type) {
@@ -119,28 +158,8 @@ void value_t::shutdown()
 void value_t::_dup()
 {
   assert(storage);
-  if (storage->refc > 1) {
+  if (storage->refc > 1)
     storage = new storage_t(*storage.get());
-
-    // If the data referenced by storage is an allocated pointer, we
-    // need to create a new object in order to achieve duplication.
-    switch (storage->type) {
-    case BALANCE:
-      *(balance_t **) storage->data =
-	new balance_t(**(balance_t **) storage->data);
-      break;
-    case BALANCE_PAIR:
-      *(balance_pair_t **) storage->data =
-	new balance_pair_t(**(balance_pair_t **) storage->data);
-      break;
-    case SEQUENCE:
-      *(sequence_t **) storage->data =
-	new sequence_t(**(sequence_t **) storage->data);
-      break;
-    default:
-      break;			// everything else has been duplicated
-    }
-  }
 }
 
 value_t::operator bool() const

value.h

@@ -164,13 +164,9 @@ private:
     explicit storage_t(const storage_t& rhs)
       : type(rhs.type), refc(0) {
       TRACE_CTOR(value_t::storage_t, "");
-      std::memcpy(data, rhs.data, sizeof(data));
+      *this = rhs;
-    }
-    storage_t& operator=(const storage_t& rhs) {
-      type = rhs.type;
-      std::memcpy(data, rhs.data, sizeof(data));
-      return *this;
     }
+    storage_t& operator=(const storage_t& rhs);
 
     /**
      * Reference counting methods.  The intrusive_ptr_* methods are