Bugfix for accidental violation of immutable value guarantees due to

inner field members

7 files changed, 116 insertions, 14 deletions

diff --git a/src/compile/assignments.c b/src/compile/assignments.c
index c0f45f5b..74a00e0b 100644
--- a/src/compile/assignments.c
+++ b/src/compile/assignments.c
@@ -101,7 +101,7 @@ Text_t compile_assignment_statement(env_t *env, ast_t *ast) {
                           "variable's scope may outlive the scope of the "
                           "stack memory.");
         env_t *val_env = with_enum_scope(env, lhs_t);
-        Text_t val = compile_to_type(val_env, assign->values->ast, lhs_t);
+        Text_t val = compile_maybe_incref(val_env, assign->values->ast, lhs_t);
         return Texts(compile_assignment(env, assign->targets->ast, val), ";\n");
     }
 
@@ -120,7 +120,7 @@ Text_t compile_assignment_statement(env_t *env, ast_t *ast) {
                           "variable's scope may outlive the scope of the "
                           "stack memory.");
         env_t *val_env = with_enum_scope(env, lhs_t);
-        Text_t val = compile_to_type(val_env, value->ast, lhs_t);
+        Text_t val = compile_maybe_incref(val_env, value->ast, lhs_t);
         code = Texts(code, compile_type(lhs_t), " $", i, " = ", val, ";\n");
         i += 1;
     }
@@ -178,13 +178,13 @@ Text_t compile_lvalue(env_t *env, ast_t *ast) {
                 type_t *value_type = get_type(env, table_type->default_value);
                 return Texts("*Table$get_or_setdefault(", compile_to_pointer_depth(env, index->indexed, 1, false), ", ",
                              compile_type(table_type->key_type), ", ", compile_type(value_type), ", ",
-                             compile_to_type(env, index->index, table_type->key_type), ", ",
-                             compile_to_type(env, table_type->default_value, table_type->value_type), ", ",
+                             compile_maybe_incref(env, index->index, table_type->key_type), ", ",
+                             compile_maybe_incref(env, table_type->default_value, table_type->value_type), ", ",
                              compile_type_info(container_t), ")");
             }
             return Texts("*(", compile_type(Type(PointerType, table_type->value_type)), ")Table$reserve(",
                          compile_to_pointer_depth(env, index->indexed, 1, false), ", ", "stack(",
-                         compile_to_type(env, index->index, table_type->key_type), ")", ", NULL,",
+                         compile_maybe_incref(env, index->index, table_type->key_type), ")", ", NULL,",
                          compile_type_info(container_t), ")");
         } else {
             code_err(ast, "I don't know how to assign to this target");
diff --git a/src/compile/enums.c b/src/compile/enums.c
index 31af96ad..56d6432a 100644
--- a/src/compile/enums.c
+++ b/src/compile/enums.c
@@ -157,10 +157,15 @@ Text_t compile_enum_field_access(env_t *env, ast_t *ast) {
         if (streq(f->field, tag->name)) {
             Text_t tag_name = namespace_name(e->env, e->env->namespace, Texts("tag$", tag->name));
             if (tag->type != NULL && Match(tag->type, StructType)->fields) {
+                Text_t member = compile_maybe_incref(
+                    env,
+                    WrapAST(ast, InlineCCode,
+                            .chunks = new (ast_list_t, WrapAST(ast, TextLiteral, Texts("_e.", tag->name))),
+                            .type = tag->type),
+                    tag->type);
                 return Texts("({ ", compile_declaration(value_t, Text("_e")), " = ",
                              compile_to_pointer_depth(env, f->fielded, 0, false), "; ", "_e.$tag == ", tag_name, " ? ",
-                             promote_to_optional(tag->type, Texts("_e.", tag->name)), " : ", compile_none(tag->type),
-                             "; })");
+                             promote_to_optional(tag->type, member), " : ", compile_none(tag->type), "; })");
             } else if (fielded_t->tag == PointerType) {
                 Text_t fielded = compile_to_pointer_depth(env, f->fielded, 1, false);
                 return Texts("((", fielded, ")->$tag == ", tag_name, " ? OPTIONAL_EMPTY_STRUCT : NONE_EMPTY_STRUCT)");
diff --git a/src/compile/expressions.c b/src/compile/expressions.c
index 130a267c..a69a7d56 100644
--- a/src/compile/expressions.c
+++ b/src/compile/expressions.c
@@ -11,11 +11,39 @@
 
 public
 Text_t compile_maybe_incref(env_t *env, ast_t *ast, type_t *t) {
-    if (is_idempotent(ast) && can_be_mutated(env, ast)) {
-        type_t *actual = get_type(with_enum_scope(env, t), ast);
-        if (t->tag == ListType && type_eq(t, actual)) return Texts("LIST_COPY(", compile_to_type(env, ast, t), ")");
-        else if (t->tag == TableType && type_eq(t, actual))
-            return Texts("TABLE_COPY(", compile_to_type(env, ast, t), ")");
+    if (!has_refcounts(t) || !can_be_mutated(env, ast)) {
+        return compile_to_type(env, ast, t);
+    }
+
+    // When using a struct as a value, we need to increment the refcounts of the inner fields as well:
+    if (t->tag == StructType) {
+        // If the struct is non-idempotent, we have to stash it in a local var first
+        if (is_idempotent(ast)) {
+            Text_t code = Texts("((", compile_type(t), "){");
+            for (arg_t *field = Match(t, StructType)->fields; field; field = field->next) {
+                Text_t val = compile_maybe_incref(env, WrapAST(ast, FieldAccess, .fielded = ast, .field = field->name),
+                                                  get_arg_type(env, field));
+                code = Texts(code, val);
+                if (field->next) code = Texts(code, ", ");
+            }
+            return Texts(code, "})");
+        } else {
+            Text_t code = Texts("({ ", compile_declaration(t, Text("_tmp")), " = ", compile_to_type(env, ast, t), "; ",
+                                "((", compile_type(t), "){");
+            ast_t *tmp = WrapAST(ast, InlineCCode,
+                                 .chunks = new (ast_list_t, .ast = WrapAST(ast, TextLiteral, Text("_tmp"))), .type = t);
+            for (arg_t *field = Match(t, StructType)->fields; field; field = field->next) {
+                Text_t val = compile_maybe_incref(env, WrapAST(ast, FieldAccess, .fielded = tmp, .field = field->name),
+                                                  get_arg_type(env, field));
+                code = Texts(code, val);
+                if (field->next) code = Texts(code, ", ");
+            }
+            return Texts(code, "}); })");
+        }
+    } else if (t->tag == ListType && ast->tag != List && can_be_mutated(env, ast) && type_eq(get_type(env, ast), t)) {
+        return Texts("LIST_COPY(", compile_to_type(env, ast, t), ")");
+    } else if (t->tag == TableType && ast->tag != Table && can_be_mutated(env, ast) && type_eq(get_type(env, ast), t)) {
+        return Texts("TABLE_COPY(", compile_to_type(env, ast, t), ")");
     }
     return compile_to_type(env, ast, t);
 }
diff --git a/src/compile/pointers.c b/src/compile/pointers.c
index 11348330..98274cc8 100644
--- a/src/compile/pointers.c
+++ b/src/compile/pointers.c
@@ -55,13 +55,13 @@ Text_t compile_typed_allocation(env_t *env, ast_t *ast, type_t *pointer_type) {
     type_t *pointed = Match(pointer_type, PointerType)->pointed;
     switch (ast->tag) {
     case HeapAllocate: {
-        return Texts("heap(", compile_to_type(env, Match(ast, HeapAllocate)->value, pointed), ")");
+        return Texts("heap(", compile_maybe_incref(env, Match(ast, HeapAllocate)->value, pointed), ")");
     }
     case StackReference: {
         ast_t *subject = Match(ast, StackReference)->value;
         if (can_be_mutated(env, subject) && type_eq(pointed, get_type(env, subject)))
             return Texts("(&", compile_lvalue(env, subject), ")");
-        else return Texts("stack(", compile_to_type(env, subject, pointed), ")");
+        else return Texts("stack(", compile_maybe_incref(env, subject, pointed), ")");
     }
     default: code_err(ast, "Not an allocation!");
     }
diff --git a/src/types.c b/src/types.c
index edfee27d..4d15d493 100644
--- a/src/types.c
+++ b/src/types.c
@@ -249,6 +249,27 @@ PUREFUNC bool has_heap_memory(type_t *t) {
     }
 }
 
+PUREFUNC bool has_refcounts(type_t *t) {
+    switch (t->tag) {
+    case ListType: return true;
+    case TableType: return true;
+    case OptionalType: return has_refcounts(Match(t, OptionalType)->type);
+    case StructType: {
+        for (arg_t *field = Match(t, StructType)->fields; field; field = field->next) {
+            if (has_refcounts(field->type)) return true;
+        }
+        return false;
+    }
+    case EnumType: {
+        for (tag_t *tag = Match(t, EnumType)->tags; tag; tag = tag->next) {
+            if (tag->type && has_refcounts(tag->type)) return true;
+        }
+        return false;
+    }
+    default: return false;
+    }
+}
+
 PUREFUNC bool has_stack_memory(type_t *t) {
     if (!t) return false;
     switch (t->tag) {
diff --git a/src/types.h b/src/types.h
index b8f9a42f..8a2175d9 100644
--- a/src/types.h
+++ b/src/types.h
@@ -150,6 +150,7 @@ typedef enum {
 } precision_cmp_e;
 PUREFUNC precision_cmp_e compare_precision(type_t *a, type_t *b);
 PUREFUNC bool has_heap_memory(type_t *t);
+PUREFUNC bool has_refcounts(type_t *t);
 PUREFUNC bool has_stack_memory(type_t *t);
 PUREFUNC bool can_promote(type_t *actual, type_t *needed);
 PUREFUNC const char *enum_single_value_tag(type_t *enum_type, type_t *t);
diff --git a/test/values.tm b/test/values.tm
new file mode 100644
index 00000000..86f34a89
--- /dev/null
+++ b/test/values.tm
@@ -0,0 +1,47 @@
+# Tests for ensuring immutable value nature in various contexts
+struct Inner(xs:[Int32])
+
+struct Outer(inner:Inner)
+
+func sneaky(outer:Outer)
+    (&outer.inner.xs)[1] = 99
+
+func sneaky2(outer:&Outer)
+    (&outer.inner.xs)[1] = 99
+
+func main()
+    do
+        xs := [10, 20, 30]
+        copy := xs
+        (&xs)[1] = 99
+        assert xs == [99, 20, 30]
+        assert copy == [10, 20, 30]
+
+    do
+        t := {"A":10, "B":20}
+        copy := t
+        (&t)["A"] = 99
+        assert t == {"A":99, "B":20}
+        assert copy == {"A":10, "B":20}
+
+    do
+        foo := Outer(Inner([10, 20, 30]))
+        copy := foo
+        (&foo.inner.xs)[1] = 99
+        assert foo.inner.xs == [99, 20, 30]
+        assert copy.inner.xs == [10, 20, 30]
+
+    do
+        foo := Outer(Inner([10, 20, 30]))
+        copy := foo
+        sneaky(foo)
+        assert foo.inner.xs == [10, 20, 30]
+        assert copy.inner.xs == [10, 20, 30]
+
+    do
+        foo := Outer(Inner([10, 20, 30]))
+        copy := foo
+        sneaky2(&foo)
+        assert foo.inner.xs == [99, 20, 30]
+        assert copy.inner.xs == [10, 20, 30]
+