improve move checker to detect pointer leaksHEADmastergnc

+ Currently requires a lot of unnecessary `forget` statements,
  but at least some can likely be eliminated by doing some
  basic origin analysis, kind of like with groups

6 files changed, 218 insertions, 113 deletions

diff --git a/examples/vec.fwd b/examples/vec.fwd
index f7838f1..22049e0 100644
--- a/examples/vec.fwd
+++ b/examples/vec.fwd
@@ -45,9 +45,17 @@ set_vec(vec v, i64 i, i64 e, (vec) ok)
 {
 	v => [n => n, s => s, buf => buf];
 	buf + (n - 1 as u64) => nbuf;
-	nil nbuf {fwdpanic("should never happen");} => dst;
+	nil nbuf {
+		nil nbuf;
+		fwdfree(buf);
+		fwdpanic("should never happen");
+	} => dst;
+
+	/* perform actual store */
 	e => dst*;
-	ok([n => n, s => s, nbuf => buf] vec);
+
+	nil nbuf;
+	ok([n => n, s => s, buf => buf] vec);
 }
 
 n_vec(vec v, (vec, u64) ok)
@@ -67,9 +75,19 @@ append_vec(vec v, i64 e, (vec) ok)
 at_vec(vec v, u64 i, (vec, &i64) ok)
 {
 	v => [n => n, s => s, buf => buf];
-	guard(i < n) => {fwdpanic("bounds error")} => ;
-	fwdptradd(buf, (i * sizeof(i64)) as i64) => *i64 buf;
-	nil buf {fwdpanic("should never happen");} => &i64 bufr;
+	guard(i < n) => {
+		fwdfree(buf);
+		fwdpanic("bounds error");
+	} => ;
+
+	buf + i => *i64 nbuf;
+	nil nbuf {
+		nil nbuf;
+		fwdfree(buf);
+		fwdpanic("should never happen");
+	} => &i64 bufr;
+
+	nil nbuf;
 	ok([n => n, s => s, buf => buf] vec, bufr);
 }
 
diff --git a/include/fwd/ast.h b/include/fwd/ast.h
index 4c8d412..0fd1118 100644
--- a/include/fwd/ast.h
+++ b/include/fwd/ast.h
@@ -349,7 +349,7 @@ static inline bool is_trivially_copyable(struct type *type)
 	case TYPE_U32:
 	case TYPE_U64:
 	case TYPE_REF:
-	case TYPE_PTR:
+	case TYPE_BOOL:
 	case TYPE_FUNC_PTR:
 	case TYPE_PURE_CLOSURE:
 		return true;
@@ -590,6 +590,7 @@ static inline bool is_trivially_copyable(struct type *type)
 #define nil_check_expr(x) return_a0(x, AST_NIL_CHECK)
 #define nil_check_body(x) return_a1(x, AST_NIL_CHECK)
 #define nil_check_ref(x) return_a2(x, AST_NIL_CHECK)
+#define nil_check_rest(x) return_a3(x, AST_NIL_CHECK)
 #define gen_nil_check(expr, body, ref, loc) \
 	gen3(AST_NIL_CHECK, expr, body, ref, loc)
 
diff --git a/src/analyze.c b/src/analyze.c
index 2dc039a..04b7856 100644
--- a/src/analyze.c
+++ b/src/analyze.c
@@ -397,7 +397,6 @@ static int analyze_closure(struct state *state, struct scope *scope,
 	}
 
 	node->scope = closure_scope;
-	closure_scope->flags |= SCOPE_PROC;
 	scope_add_scope(scope, closure_scope);
 
 	if (analyze_list(state, closure_scope, closure_bindings(node)))
@@ -777,6 +776,9 @@ static int analyze_nil_check(struct state *state, struct scope *scope, struct as
 		return -1;
 	}
 
+	if (analyze(state, scope, nil_check_rest(node)))
+		return -1;
+
 	node->t = tgen_void(node->loc);
 	return 0;
 }
diff --git a/src/ast.c b/src/ast.c
index 9515775..a39cb94 100644
--- a/src/ast.c
+++ b/src/ast.c
@@ -570,10 +570,24 @@ struct type *reverse_type_list(struct type *root)
 void fix_closures(struct ast *root)
 {
 	while (root) {
-		if (root->k != AST_CALL) {
+		if (root->k != AST_CALL && root->k != AST_NIL_CHECK) {
 			root = root->n;
 			continue;
 		}
+		if (root->k == AST_NIL_CHECK) {
+			struct ast *next = root->n;
+			if (!next)
+				next = gen_empty(root->loc);
+
+			struct ast *block = gen_block(next, root->loc);
+			nil_check_rest(root) = block;
+			root->n = NULL;
+
+			root = next;
+			continue;
+		}
+
+		/* call */
 		struct ast *arg = ast_last(call_args(root));
 		if (!arg) {
 			root = root->n;
diff --git a/src/move.c b/src/move.c
index 2a55d3d..5137719 100644
--- a/src/move.c
+++ b/src/move.c
@@ -138,9 +138,9 @@ static void forget_references(struct state *state)
 	state->referenced = moved_create();
 }
 
-static int mvcheck(struct state *state, struct ast *node);
-static int mvcheck_list(struct state *state, struct ast *nodes);
-static int mvcheck_statements(struct state *state, struct ast *nodes);
+static int mvcheck_expr(struct state *state, struct ast *node);
+static int mvcheck_expr_list(struct state *state, struct ast *nodes);
+static int mvcheck_statements(struct state *state, struct ast *nodes, bool last);
 
 static int refcheck_id(struct state *state, struct ast *node)
 {
@@ -192,20 +192,7 @@ static int mvcheck_deref(struct state *state, struct ast *node)
 {
 	/** @todo good enough for now but probably won't hold when we start
 	 * doing element lookups in structs etc. */
-	return mvcheck(state, deref_base(node));
-}
-
-static int mvcheck_proc(struct state *state, struct ast *node)
-{
-	/* extern, can't really do anything so just say it's fine */
-	if (!proc_body(node))
-		return 0;
-
-	struct state new_state = create_state(state);
-	/* we don't need to merge things into the parent state */
-	int ret = mvcheck(&new_state, proc_body(node));
-	destroy_state(&new_state);
-	return ret;
+	return mvcheck_expr(state, deref_base(node));
 }
 
 static int total_check_single(struct state *state, struct scope *scope)
@@ -245,16 +232,20 @@ static int total_check_proc(struct state *state, struct scope *scope)
 	return ret | total_check_proc(state, scope->parent);
 }
 
-static int mvcheck_block(struct state *state, struct ast *node)
+static int mvcheck_block(struct state *state, struct ast *node, bool last)
 {
+	assert(node->k == AST_BLOCK);
 	struct state new_state = create_state(state);
-	int ret = mvcheck_statements(&new_state, block_body(node));
+	int ret = mvcheck_statements(&new_state, block_body(node), last);
 	if (ret) {
 		destroy_state(&new_state);
 		return ret;
 	}
 
-	if (total_check_single(&new_state, node->scope))
+	if (last && total_check_proc(&new_state, node->scope))
+		semantic_info(node->scope, node, "at end of block");
+
+	else if (!last && total_check_single(&new_state, node->scope))
 		semantic_info(node->scope, node, "at end of block");
 
 	/** @todo add exit analysis and run total_check_proc on those blocks */
@@ -263,9 +254,20 @@ static int mvcheck_block(struct state *state, struct ast *node)
 	return 0;
 }
 
-static int mvcheck_call(struct state *state, struct ast *node)
+static int mvcheck_closure(struct state *state, struct ast *node, bool last)
 {
-	if (mvcheck(state, call_expr(node)))
+	struct state new_state = create_state(state);
+	new_state.pure = ast_flags(node, AST_FLAG_NOMOVES);
+
+	int ret = mvcheck_block(&new_state, closure_body(node), last);
+	push_up(&new_state);
+	destroy_state(&new_state);
+	return ret;
+}
+
+static int mvcheck_call(struct state *state, struct ast *node, bool last)
+{
+	if (mvcheck_expr(state, call_expr(node)))
 		return -1;
 
 	struct ast *args = call_args(node);
@@ -277,10 +279,35 @@ static int mvcheck_call(struct state *state, struct ast *node)
 		if (arg->k == AST_CLOSURE)
 			continue;
 
-		if (mvcheck(state, arg))
+		if (mvcheck_expr(state, arg))
 			return -1;
 	}
 
+	/* count how many closure groups call has. If the call is an exit point
+	 * (last == true), then a single closure group must also be an exit
+	 * point */
+	size_t groups = 0;
+	foreach_node(arg, call_args(node)) {
+		if (arg->k != AST_CLOSURE)
+			continue;
+
+		groups++;
+		struct ast *next = arg->n;
+		while (next && next->t->group == arg->t->group)
+			next = next->n;
+
+		if (!next)
+			break;
+
+		arg = next;
+	}
+
+	if (!last && (groups > 0)) {
+		semantic_error(node->scope, node,
+				"calls with closures must currently be exit points, sorry!");
+		return -1;
+	}
+
 	/* check into closures */
 	int ret = 0;
 	struct state buffer_state = create_state(state);
@@ -290,7 +317,7 @@ static int mvcheck_call(struct state *state, struct ast *node)
 			continue;
 
 		struct state arg_state = create_state(state);
-		ret = mvcheck(&arg_state, arg);
+		ret = mvcheck_closure(&arg_state, arg, last && (groups == 1));
 
 		merge(&group_state, &arg_state);
 		destroy_state(&arg_state);
@@ -316,21 +343,6 @@ static int mvcheck_call(struct state *state, struct ast *node)
 	return ret;
 }
 
-static int mvcheck_closure(struct state *state, struct ast *node)
-{
-	struct state new_state = create_state(state);
-	new_state.pure = ast_flags(node, AST_FLAG_NOMOVES);
-
-	int ret = mvcheck(&new_state, closure_body(node));
-	push_up(&new_state);
-	destroy_state(&new_state);
-
-	if (total_check_single(state, node->scope))
-		semantic_info(node->scope, node, "in closure");
-
-	return ret;
-}
-
 static void opt_group_left(struct state *state, struct ast *def,
                            struct ast *node)
 {
@@ -396,30 +408,36 @@ static int mvcheck_id(struct state *state, struct ast *node)
 
 static int mvcheck_let(struct state *state, struct ast *node)
 {
-	return mvcheck(state, let_expr(node));
+	return mvcheck_expr(state, let_expr(node));
 }
 
 static int mvcheck_init(struct state *state, struct ast *node)
 {
-	return mvcheck_list(state, init_body(node));
+	return mvcheck_expr_list(state, init_body(node));
 }
 
-static int mvcheck_if(struct state *state, struct ast *node)
+static int mvcheck_if(struct state *state, struct ast *node, bool last)
 {
-	if (mvcheck(state, if_cond(node)))
+	if (!last) {
+		semantic_error(node->scope, node,
+				"`if` statements must currently be exit points, sorry!");
 		return -1;
+	}
+
+	assert(if_else(node));
+
+	/* don't check cond since it can't take ownership of anything */
 
 	struct state body_state = create_state(state);
 	struct state else_state = create_state(state);
 
-	if (mvcheck(&body_state, if_body(node))) {
+	if (mvcheck_block(&body_state, if_body(node), last)) {
 		destroy_state(&body_state);
 		destroy_state(&else_state);
 		return -1;
 	}
 
-	if (if_else(node))
-	if (mvcheck(&else_state, if_else(node))) {
+	if (mvcheck_block(&else_state, if_else(node), last)) {
 		destroy_state(&body_state);
 		destroy_state(&else_state);
 		return -1;
@@ -433,49 +451,11 @@ static int mvcheck_if(struct state *state, struct ast *node)
 	return 0;
 }
 
-static int mvcheck_unop(struct state *state, struct ast *node)
-{
-	return mvcheck(state, unop_expr(node));
-}
-
-static int mvcheck_binop(struct state *state, struct ast *node)
-{
-	if (mvcheck(state, binop_left(node)))
-		return -1;
-
-	return mvcheck(state, binop_right(node));
-}
-
-static int mvcheck_comparison(struct state *state, struct ast *node)
-{
-	if (mvcheck(state, comparison_left(node)))
-		return -1;
-
-	return mvcheck(state, comparison_right(node));
-}
-
-static int mvcheck_list(struct state *state, struct ast *nodes)
+static int mvcheck_expr_list(struct state *state, struct ast *nodes)
 {
 	foreach_node(node, nodes) {
-		if (mvcheck(state, node))
-			return -1;
-	}
-
-	return 0;
-}
-
-static int mvcheck_statements(struct state *state, struct ast *nodes)
-{
-	foreach_node(node, nodes) {
-		struct state new_state = create_state(state);
-		if (mvcheck(&new_state, node)) {
-			destroy_state(&new_state);
+		if (mvcheck_expr(state, node))
 			return -1;
-		}
-
-		forget_references(&new_state);
-		push_up(&new_state);
-		destroy_state(&new_state);
 	}
 
 	return 0;
@@ -484,7 +464,7 @@ static int mvcheck_statements(struct state *state, struct ast *nodes)
 static int mvcheck_construct(struct state *state, struct ast *node)
 {
 	foreach_node(expr, construct_members(node)) {
-		if (mvcheck(state, expr))
+		if (mvcheck_expr(state, expr))
 			return -1;
 	}
 
@@ -493,12 +473,12 @@ static int mvcheck_construct(struct state *state, struct ast *node)
 
 static int mvcheck_construction(struct state *state, struct ast *node)
 {
-	return mvcheck(state, construction_expr(node));
+	return mvcheck_expr(state, construction_expr(node));
 }
 
 static int mvcheck_as(struct state *state, struct ast *as)
 {
-	return mvcheck(state, as_expr(as));
+	return mvcheck_expr(state, as_expr(as));
 }
 
 static int mvcheck_forget(struct state *state, struct ast *node)
@@ -513,34 +493,65 @@ static int mvcheck_forget(struct state *state, struct ast *node)
 
 static int mvcheck_explode(struct state *state, struct ast *node)
 {
-	return mvcheck(state, explode_expr(node));
+	return mvcheck_expr(state, explode_expr(node));
 }
 
 static int mvcheck_write(struct state *state, struct ast *node)
 {
-	return mvcheck(state, write_src(node));
+	return mvcheck_expr(state, write_src(node));
 }
 
-static int mvcheck(struct state *state, struct ast *node)
+static int mvcheck_nil_check(struct state *state, struct ast *node, bool last)
 {
+	if (!last) {
+		/** @todo would this be an internal error? */
+		semantic_error(node->scope, node,
+				"`nil check` must be exit point, sorry");
+		return -1;
+	}
+
+	assert(nil_check_rest(node));
+
+	struct state body_state = create_state(state);
+	struct state rest_state = create_state(state);
+
+	if (mvcheck_block(&body_state, nil_check_body(node), last)) {
+		destroy_state(&body_state);
+		destroy_state(&rest_state);
+		return -1;
+	}
+
+	if (mvcheck_block(&rest_state, nil_check_rest(node), last)) {
+		destroy_state(&body_state);
+		destroy_state(&rest_state);
+		return -1;
+	}
+
+	push_up(&body_state);
+	push_up(&rest_state);
+
+	destroy_state(&body_state);
+	destroy_state(&rest_state);
+	return 0;
+}
+
+static int mvcheck_expr(struct state *state, struct ast *node)
+{
+	/* unary, binary and comparison operators (a kind of binary operator,
+	 * fair enough) must operate on primitive types, and as such don't need
+	 * to be move checked as none of them can change ownership or a value */
 	if (is_unop(node))
-		return mvcheck_unop(state, node);
+		return 0;
 
 	if (is_binop(node))
-		return mvcheck_binop(state, node);
+		return 0;
 
 	if (is_comparison(node))
-		return mvcheck_comparison(state, node);
+		return 0;
 
 	switch (node->k) {
-	case AST_PROC_DEF:      return mvcheck_proc      (state, node);
-	case AST_BLOCK:         return mvcheck_block     (state, node);
-	case AST_CALL:          return mvcheck_call      (state, node);
-	case AST_CLOSURE:       return mvcheck_closure   (state, node);
-	case AST_LET:           return mvcheck_let       (state, node);
 	case AST_ID:            return mvcheck_id        (state, node);
 	case AST_INIT:          return mvcheck_init      (state, node);
-	case AST_IF:            return mvcheck_if        (state, node);
 	case AST_REF:           return mvcheck_ref       (state, node);
 	case AST_DEREF:         return mvcheck_deref     (state, node);
 	case AST_CONSTRUCT:     return mvcheck_construct (state, node);
@@ -549,7 +560,6 @@ static int mvcheck(struct state *state, struct ast *node)
 	case AST_EXPLODE:       return mvcheck_explode   (state, node);
 	case AST_WRITE:         return mvcheck_write     (state, node);
 	case AST_CONSTRUCTION:  return mvcheck_construction(state, node);
-	case AST_NIL_CHECK:
 	case AST_SIZEOF:
 	case AST_STRUCT_DEF:
 	case AST_EMPTY:
@@ -567,11 +577,72 @@ static int mvcheck(struct state *state, struct ast *node)
 	return -1;
 }
 
+static int mvcheck_statement(struct state *state, struct ast *node, bool last)
+{
+	switch (node->k) {
+	case AST_CALL:      return mvcheck_call     (state, node, last);
+	case AST_IF:        return mvcheck_if       (state, node, last);
+	case AST_NIL_CHECK: return mvcheck_nil_check(state, node, last);
+	case AST_EXPLODE:   return mvcheck_explode  (state, node);
+	case AST_LET:       return mvcheck_let      (state, node);
+	case AST_WRITE:     return mvcheck_write    (state, node);
+	case AST_FORGET:    return mvcheck_forget   (state, node);
+	case AST_EMPTY:     return 0;
+	default: break;
+	}
+
+	internal_error("unhandled move statement: %s", ast_str(node->k));
+	return -1;
+}
+
+static int mvcheck_statements(struct state *state, struct ast *nodes, bool last)
+{
+	foreach_node(node, nodes) {
+		struct state new_state = create_state(state);
+		if (mvcheck_statement(&new_state, node, last && !node->n)) {
+			destroy_state(&new_state);
+			return -1;
+		}
+
+		forget_references(&new_state);
+		push_up(&new_state);
+		destroy_state(&new_state);
+	}
+
+	return 0;
+}
+
+static int mvcheck_proc(struct state *state, struct ast *node)
+{
+	/* extern, can't really do anything so just say it's fine */
+	if (!proc_body(node))
+		return 0;
+
+	struct state new_state = create_state(state);
+	/* we don't need to merge things into the parent state */
+	int ret = mvcheck_block(&new_state, proc_body(node), true);
+	destroy_state(&new_state);
+	return ret;
+}
+
+static int mvcheck_top(struct state *state, struct ast *node)
+{
+	switch (node->k) {
+	case AST_PROC_DEF:   return mvcheck_proc(state, node);
+	case AST_IMPORT:     return 0;
+	case AST_STRUCT_DEF: return 0;
+	default: break;
+	}
+
+	internal_error("missing top move check for %s", ast_str(node->k));
+	return -1;
+}
+
 int mvcheck_root(struct ast *root)
 {
 	foreach_node(node, root) {
 		struct state state = create_state(NULL);
-		if (mvcheck(&state, node))
+		if (mvcheck_top(&state, node))
 			return -1;
 
 		destroy_state(&state);
diff --git a/src/parser.y b/src/parser.y
index 5f46895..dad3210 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -424,8 +424,7 @@ explode
 	}
 
 if
-	: "if" expr body { $$ = gen_if($2, $3, NULL, src_loc(@$)); }
-	| "if" expr body "else" body { $$ = gen_if($2, $3, $5, src_loc(@$)); }
+	: "if" expr body "else" body { $$ = gen_if($2, $3, $5, src_loc(@$)); }
 	| "if" expr body "else" if { $$ = gen_if($2, $3, $5, src_loc(@$)); }
 
 nil