3663 - fix a refcounting bug: '(type)' != 'type'

This was a large commit, and most of it is a follow-up to commit 3309,
undoing what is probably the final ill-considered optimization I added
to s-expressions in Mu: I was always representing (a b c) as (a b . c),
etc. That is now gone.

Why did I need to take it out? The key problem was the error silently
ignored in layer 30. That was causing size_of("(type)") to silently
return garbage rather than loudly complain (assuming 'type' was a simple
type).

But to take it out I had to modify types_strictly_match (layer 21) to
actually strictly match and not just do a prefix match.

In the process of removing the prefix match, I had to make extracting
recipe types from recipe headers more robust. So far it only matched the
first element of each ingredient's type; these matched:

  (recipe address:number -> address:number)
  (recipe address -> address)

I didn't notice because the dotted notation optimization was actually
representing this as:

  (recipe address:number -> address number)

---

One final little thing in this commit: I added an alias for 'assert'
called 'assert_for_now', to indicate that I'm not sure something's
really an invariant, that it might be triggered by (invalid) user
programs, and so require more thought on error handling down the road.

But this may well be an ill-posed distinction. It may be overwhelmingly
uneconomic to continually distinguish between model invariants and error
states for input. I'm starting to grow sympathetic to Google Analytics's
recent approach of just banning assertions altogether. We'll see..

003trace.cc

@@ -176,6 +176,9 @@ int Trace_errors = 0;  // used only when Trace_stream is NULL
 
 // Errors are a special layer.
 #define raise  (!Trace_stream ? (tb_shutdown(),++Trace_errors,cerr) /*do print*/ : Trace_stream->stream(Error_depth, "error"))
+// If we aren't yet sure how to deal with some corner case, use assert_for_now
+// to indicate that it isn't an inviolable invariant.
+#define assert_for_now assert
 
 // Inside tests, fail any tests that displayed (unexpected) errors.
 // Expected errors in tests should always be hidden and silently checked for.

010vm.cc

@@ -301,12 +301,34 @@ void slurp_properties(istream& in, vector<pair<string, string_tree*> >& out) {
 string_tree* parse_property_list(istream& in) {
   skip_whitespace_but_not_newline(in);
   if (!has_data(in)) return NULL;
-  string_tree* left = new string_tree(slurp_until(in, ':'));
-  if (!has_data(in)) return left;
-  string_tree* right = parse_property_list(in);
-  return new string_tree(left, right);
+  string_tree* first = new string_tree(slurp_until(in, ':'));
+  if (!has_data(in)) return first;
+  string_tree* rest = parse_property_list(in);
+  if (!has_data(in) && rest->atom)
+    return new string_tree(first, new string_tree(rest, NULL));
+  return new string_tree(first, rest);
+}
+:(before "End Unit Tests")
+void test_parse_property_list_atom() {
+  istringstream in("a");
+  string_tree* x = parse_property_list(in);
+  CHECK(x->atom);
+  delete x;
+}
+void test_parse_property_list_list() {
+  istringstream in("a:b");
+  string_tree* x = parse_property_list(in);
+  CHECK(!x->atom);
+  CHECK(x->left->atom);
+  CHECK_EQ(x->left->value, "a");
+  CHECK(!x->right->atom);
+  CHECK(x->right->left->atom);
+  CHECK_EQ(x->right->left->value, "b");
+  CHECK(x->right->right == NULL);
+  delete x;
 }
 
+:(code)
 type_tree* new_type_tree(const string_tree* properties) {
   if (!properties) return NULL;
   if (properties->atom) {
@@ -375,9 +397,10 @@ bool type_tree::operator<(const type_tree& other) const {
   if (!left && other.left) return true;
   if (right && !other.right) return false;
   if (!right && other.right) return true;
+  // now if either pointer is unequal neither side can be null
   // if one side is equal that's easy
-  if (left == other.left || *left == *other.left) return *right < *other.right;
-  if (right == other.right || *right == *other.right) return *left < *other.left;
+  if (left == other.left || *left == *other.left) return right && *right < *other.right;
+  if (right == other.right || *right == *other.right) return left && *left < *other.left;
   // if the two sides criss-cross, pick the side with the smaller lhs
   if ((left == other.right || *left == *other.right)
       && (right == other.left || *right == *other.left))
@@ -423,15 +446,11 @@ void test_compare_list_with_extra_element() {
 }
 void test_compare_list_with_smaller_left_but_larger_right() {
   reagent a("a:address:number"), b("b:character:array");
-  assert(a.type->left->atom && a.type->right->atom);
-  assert(b.type->left->atom && b.type->right->atom);
   CHECK(*a.type < *b.type);
   CHECK(!(*b.type < *a.type));
 }
 void test_compare_list_with_smaller_left_but_larger_right_identical_types() {
   reagent a("a:address:boolean"), b("b:boolean:address");
-  assert(a.type->left->atom && a.type->right->atom);
-  assert(b.type->left->atom && b.type->right->atom);
   CHECK(*a.type < *b.type);
   CHECK(!(*b.type < *a.type));
 }
@@ -658,8 +677,11 @@ void dump(const string_tree* x, ostream& out) {
     if (curr->right) out << ' ';
     curr = curr->right;
   }
-  // final right
-  dump(curr, out);
+  // check for dotted list; should never happen
+  if (curr) {
+    out << ". ";
+    dump(curr, out);
+  }
   out << ')';
 }
 
@@ -684,8 +706,11 @@ void dump(const type_tree* x, ostream& out) {
     if (curr->right) out << ' ';
     curr = curr->right;
   }
-  // final right
-  dump(curr, out);
+  // check for dotted list; should never happen
+  if (curr) {
+    out << ". ";
+    dump(curr, out);
+  }
   out << ')';
 }
 
@@ -717,8 +742,11 @@ void dump_names(const type_tree* x, ostream& out) {
     if (curr->right) out << ' ';
     curr = curr->right;
   }
-  // final right
-  dump_names(curr, out);
+  // check for dotted list; should never happen
+  if (curr) {
+    out << ". ";
+    dump_names(curr, out);
+  }
   out << ')';
 }
 
@@ -743,8 +771,11 @@ void dump_names_without_quotes(const type_tree* x, ostream& out) {
     if (curr->right) out << ' ';
     curr = curr->right;
   }
-  // final right
-  dump_names_without_quotes(curr, out);
+  // check for dotted list; should never happen
+  if (curr) {
+    out << ". ";
+    dump_names_without_quotes(curr, out);
+  }
   out << ')';
 }
 

017parse_tree.cc

@@ -4,9 +4,8 @@
 // (address to array of charaters) to (list of numbers)".
 //
 // Type trees aren't as general as s-expressions even if they look like them:
-// the first element of a type tree is always an atom, and left and right
-// pointers of non-atoms are never NULL. All type trees are 'dotted' in lisp
-// parlance.
+// the first element of a type tree is always an atom, and it can never be
+// dotted (right->right->right->...->right is always NULL).
 //
 // For now you can't use the simpler 'colon-based' representation inside type
 // trees. Once you start typing parens, keep on typing parens.
@@ -79,19 +78,6 @@ string_tree* parse_string_tree(istream& in) {
   }
   in.get();  // skip ')'
   assert(*curr == NULL);
-  if (result == NULL) return result;
-  if (result->right == NULL) return result;
-  // standardize the final element to always be on the right if it's an atom
-  // (a b c) => (a b . c) in s-expression parlance
-  string_tree* tmp = result;
-  while (tmp->right->right) tmp = tmp->right;
-  assert(!tmp->right->atom);
-  if (!tmp->right->left->atom) return result;
-  string_tree* tmp2 = tmp->right;
-  tmp->right = tmp2->left;
-  tmp2->left = NULL;
-  assert(tmp2->right == NULL);
-  delete tmp2;
   return result;
 }
 

021check_instruction.cc

@@ -102,6 +102,7 @@ bool types_coercible(const reagent& to, const reagent& from) {
   if (is_mu_address(from) && is_mu_number(to)) return true;
   if (is_mu_boolean(from) && is_mu_number(to)) return true;
   if (is_mu_number(from) && is_mu_boolean(to)) return true;
+  // End types_coercible Special-cases
   return false;
 }
 
@@ -136,14 +137,12 @@ bool types_strictly_match(reagent/*copy*/ to, reagent/*copy*/ from) {
   return types_strictly_match(to.type, from.type);
 }
 
-// two types match if the second begins like the first
-// (trees perform the same check recursively on each subtree)
 bool types_strictly_match(const type_tree* to, const type_tree* from) {
   if (from == to) return true;
+  if (!to) return false;
   if (!from) return to->atom && to->value == 0;
-  if (to->atom && !from->atom) return from->left->atom && from->left->name == to->name;
+  if (from->atom != to->atom) return false;
   if (from->atom) {
-    if (!to->atom) return false;
     if (from->value == -1) return from->name == to->name;
     return from->value == to->value;
   }

027call_ingredient.cc

@@ -182,6 +182,7 @@ bool is_mu_text(reagent/*copy*/ x) {
       && x.type->right->left->atom
       && x.type->right->left->value == get(Type_ordinal, "array")
       && x.type->right->right
-      && x.type->right->right->atom
-      && x.type->right->right->value == get(Type_ordinal, "character");
+      && !x.type->right->right->atom
+      && x.type->right->right->left->value == get(Type_ordinal, "character")
+      && x.type->right->right->right == NULL;
 }

030container.cc

@@ -157,7 +157,11 @@ if (!contains_key(Type, base_type->value)) {
 type_info t = get(Type, base_type->value);
 if (t.kind == CONTAINER) {
   // Compute size_of Container
-  if (!contains_key(Container_metadata, type)) return 1;  // error raised elsewhere
+  if (!contains_key(Container_metadata, type)) {
+    raise << "unknown size for container type '" << to_string(type) << "'\n" << end();
+//?     DUMP("");
+    return 0;
+  }
   return get(Container_metadata, type).size;
 }
 
@@ -203,7 +207,7 @@ void compute_container_sizes(const type_tree* type, set<type_tree>& pending_meta
       return;
     }
     if (type->left->name == "address")
-      compute_container_sizes(type->right, pending_metadata, location_for_error_messages);
+      compute_container_sizes(payload_type(type), pending_metadata, location_for_error_messages);
     // End compute_container_sizes Non-atom Special-cases
     return;
   }
@@ -231,6 +235,14 @@ void compute_container_sizes(const type_info& container_info, const type_tree* f
   Container_metadata.push_back(pair<type_tree*, container_metadata>(new type_tree(*full_type), metadata));
 }
 
+const type_tree* payload_type(const type_tree* type) {
+  assert(!type->atom);
+  const type_tree* result = type->right;
+  assert(!result->atom);
+  if (!result->right) return result->left;
+  return result;
+}
+
 container_metadata& get(vector<pair<type_tree*, container_metadata> >& all, const type_tree* key) {
   for (int i = 0;  i < SIZE(all);  ++i) {
     if (matches(all.at(i).first, key))

032array.cc

@@ -98,6 +98,10 @@ def main [
 ]
 +app: foo: 3 14 15 16
 
+:(before "End types_coercible Special-cases")
+if (is_mu_array(from) && is_mu_array(to))
+  return types_strictly_match(array_element(from.type), array_element(to.type));
+
 :(before "End size_of(reagent r) Special-cases")
 if (!r.type->atom && r.type->left->atom && r.type->left->value == get(Type_ordinal, "array")) {
   if (!r.type->right) {
@@ -111,10 +115,10 @@ if (!r.type->atom && r.type->left->atom && r.type->left->value == get(Type_ordin
 if (type->left->value == get(Type_ordinal, "array")) return static_array_length(type);
 :(code)
 int static_array_length(const type_tree* type) {
-  if (!type->atom && !type->right->atom && type->right->right->atom  // exactly 3 types
-      && is_integer(type->right->right->name)) {  // third 'type' is a number
+  if (!type->atom && type->right && !type->right->atom && type->right->right && !type->right->right->atom && !type->right->right->right  // exactly 3 types
+      && type->right->right->left && type->right->right->left->atom && is_integer(type->right->right->left->name)) {  // third 'type' is a number
     // get size from type
-    return to_integer(type->right->right->name);
+    return to_integer(type->right->right->left->name);
   }
   cerr << to_string(type) << '\n';
   assert(false);
@@ -159,7 +163,7 @@ container foo [
       raise << "container '" << name << "' doesn't specify type of array elements for '" << info.elements.back().name << "'\n" << end();
       continue;
     }
-    if (type->right->atom) {  // array has no length
+    if (!type->right->right || !is_integer(type->right->right->left->name)) {  // array has no length
       raise << "container '" << name << "' cannot determine size of element '" << info.elements.back().name << "'\n" << end();
       continue;
     }
@@ -365,20 +369,29 @@ type_tree* copy_array_element(const type_tree* type) {
 
 type_tree* array_element(const type_tree* type) {
   assert(type->right);
-  // hack: don't require parens for either array:num:3 array:address:num
-  if (!type->right->atom && type->right->right && type->right->right->atom && is_integer(type->right->right->name))
+  if (type->right->atom) {
+    return type->right;
+  }
+  else if (!type->right->right) {
+    return type->right->left;
+  }
+  // hack: support array:num:3 without requiring extra parens
+  else if (type->right->right->left && type->right->right->left->atom && is_integer(type->right->right->left->name)) {
+    assert(!type->right->right->right);
     return type->right->left;
+  }
   return type->right;
 }
 
 int array_length(const reagent& x) {
   // x should already be canonized.
-  if (!x.type->atom && !x.type->right->atom && x.type->right->right->atom  // exactly 3 types
-      && is_integer(x.type->right->right->name)) {  // third 'type' is a number
+  // hack: look for length in type
+  if (!x.type->atom && x.type->right && !x.type->right->atom && x.type->right->right && !x.type->right->right->atom && !x.type->right->right->right  // exactly 3 types
+      && x.type->right->right->left && x.type->right->right->left->atom && is_integer(x.type->right->right->left->name)) {  // third 'type' is a number
     // get size from type
-    return to_integer(x.type->right->right->name);
+    return to_integer(x.type->right->right->left->name);
   }
-  // we should never get here at transform time
+  // this should never happen at transform time
   return get_or_insert(Memory, x.value);
 }
 
@@ -393,6 +406,11 @@ void test_array_length_compound() {
   CHECK_EQ(array_length(x), 3);
 }
 
+void test_array_length_static() {
+  reagent x("1:array:num:3");
+  CHECK_EQ(array_length(x), 3);
+}
+
 :(scenario index_truncates)
 def main [
   1:array:num:3 <- create-array

034address.cc

@@ -210,10 +210,18 @@ void drop_from_type(reagent& r, string expected_type) {
     raise << "can't drop2 " << expected_type << " from '" << to_string(r) << "'\n" << end();
     return;
   }
+  // r.type = r.type->right
   type_tree* tmp = r.type;
   r.type = tmp->right;
   tmp->right = NULL;
   delete tmp;
+  // if (!r.type->right) r.type = r.type->left
+  assert(!r.type->atom);
+  if (r.type->right) return;
+  tmp = r.type;
+  r.type = tmp->left;
+  tmp->left = NULL;
+  delete tmp;
 }
 
 :(scenario new_returns_incorrect_type)
@@ -223,6 +231,13 @@ def main [
 ]
 +error: main: product of 'new' has incorrect type: '1:bool <- new num:type'
 
+:(scenario new_discerns_singleton_list_from_atom_container)
+% Hide_errors = true;
+def main [
+  1:address:num/raw <- new {(num): type}  # should be '{num: type}'
+]
++error: main: product of 'new' has incorrect type: '1:address:num/raw <- new {(num): type}'
+
 :(scenario new_with_type_abbreviation)
 def main [
   1:address:num/raw <- new num:type

036refcount.cc

@@ -52,7 +52,7 @@ void decrement_any_refcounts(const reagent& canonized_x) {
   if (is_mu_address(canonized_x)) {
     assert(canonized_x.value);
     assert(!canonized_x.metadata.size);
-    decrement_refcount(get_or_insert(Memory, canonized_x.value), canonized_x.type->right, payload_size(canonized_x));
+    decrement_refcount(get_or_insert(Memory, canonized_x.value), payload_type(canonized_x.type), payload_size(canonized_x));
   }
   // End Decrement Refcounts(canonized_x)
 }
@@ -316,7 +316,7 @@ void compute_container_address_offsets(const type_tree* type, const string& loca
       return;
     }
     if (type->left->name == "address")
-      compute_container_address_offsets(type->right, location_for_error_messages);
+      compute_container_address_offsets(payload_type(type), location_for_error_messages);
     else if (type->left->name == "array")
       compute_container_address_offsets(array_element(type), location_for_error_messages);
     // End compute_container_address_offsets Non-atom Special-cases
@@ -352,7 +352,7 @@ void compute_exclusive_container_address_offsets(const type_info& exclusive_cont
 
 void append_addresses(int base_offset, const type_tree* type, map<set<tag_condition_info>, set<address_element_info> >& out, const set<tag_condition_info>& key, const string& location_for_error_messages) {
   if (is_mu_address(type)) {
-    get_or_insert(out, key).insert(address_element_info(base_offset, new type_tree(*type->right)));
+    get_or_insert(out, key).insert(address_element_info(base_offset, new type_tree(*payload_type(type))));
     return;
   }
   const type_tree* base_type = type;
@@ -365,7 +365,7 @@ void append_addresses(int base_offset, const type_tree* type, map<set<tag_condit
       // Compute Container Address Offset(element)
       if (is_mu_address(element)) {
         trace(9993, "transform") << "address at offset " << curr_offset << end();
-        get_or_insert(out, key).insert(address_element_info(curr_offset, new type_tree(*element.type->right)));
+        get_or_insert(out, key).insert(address_element_info(curr_offset, new type_tree(*payload_type(element.type))));
         ++curr_offset;
       }
       else if (is_mu_array(element)) {