From b232a3b0d1b658862a29d250f9d7f1786204b14d Mon Sep 17 00:00:00 2001
From: crthpl <56052645+crthpl@users.noreply.github.com>
Date: Thu, 7 Apr 2022 08:05:11 -0700
Subject: [PATCH] cgen: fix `.filter()` and `.map()` on shared arrays (#13954)

---
 vlib/v/checker/assign.v             |  3 +-
 vlib/v/checker/checker.v            | 17 +++++++++++
 vlib/v/checker/fn.v                 | 12 +++++++-
 vlib/v/gen/c/array.v                | 44 ++++++++++++++++++++---------
 vlib/v/gen/c/assign.v               |  3 +-
 vlib/v/gen/c/cgen.v                 |  5 +++-
 vlib/v/tests/filter_test.v          | 22 +++++++++++++++
 vlib/v/tests/map_fn_test.v          |  8 ++++++
 vlib/v/tests/shared_autolock_test.v |  8 ++++--
 vlib/v/tests/shared_in_test.v       | 12 ++++----
 vlib/vweb/tests/vweb_test_server.v  |  4 ++-
 11 files changed, 112 insertions(+), 26 deletions(-)
 create mode 100644 vlib/v/tests/filter_test.v
 create mode 100644 vlib/v/tests/map_fn_test.v

diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v
index f95c0e09a3..470d81430a 100644
--- a/vlib/v/checker/assign.v
+++ b/vlib/v/checker/assign.v
@@ -23,6 +23,7 @@ pub fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) {
 				c.expected_type = c.expr(node.left[i])
 			}
 			right_type := c.expr(right)
+			c.fail_if_unreadable(right, right_type, 'right-hand side of assignment')
 			if i == 0 {
 				right_type0 = right_type
 				node.right_types = [
@@ -185,7 +186,7 @@ pub fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) {
 		}
 		// Do not allow `a := 0; b := 0; a = &b`
 		if !is_decl && left is ast.Ident && !is_blank_ident && !left_type.is_real_pointer()
-			&& right_type.is_real_pointer() {
+			&& right_type.is_real_pointer() && !right_type.has_flag(.shared_f) {
 			left_sym := c.table.sym(left_type)
 			if left_sym.kind != .function {
 				c.warn(
diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 357e7b6c5e..0fc6104eab 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -2066,6 +2066,7 @@ fn (mut c Checker) assert_stmt(node ast.AssertStmt) {
 		c.error('assert can be used only with `bool` expressions, but found `$atype_name` instead',
 			node.pos)
 	}
+	c.fail_if_unreadable(node.expr, ast.bool_type_idx, 'assertion')
 	c.expected_type = cur_exp_typ
 }
 
@@ -4189,10 +4190,26 @@ pub fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what stri
 				c.fail_if_unreadable(expr.expr, expr.expr_type, what)
 			}
 		}
+		ast.CallExpr {
+			pos = expr.pos
+			if expr.is_method {
+				c.fail_if_unreadable(expr.left, expr.left_type, what)
+			}
+			return
+		}
+		ast.LockExpr {
+			// TODO: check expressions inside the lock by appending to c.(r)locked_names
+			return
+		}
 		ast.IndexExpr {
 			pos = expr.left.pos().extend(expr.pos)
 			c.fail_if_unreadable(expr.left, expr.left_type, what)
 		}
+		ast.InfixExpr {
+			pos = expr.left.pos().extend(expr.pos)
+			c.fail_if_unreadable(expr.left, expr.left_type, what)
+			c.fail_if_unreadable(expr.right, expr.right_type, what)
+		}
 		else {
 			pos = expr.pos()
 		}
diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v
index 08ba5a1282..c473e9fc90 100644
--- a/vlib/v/checker/fn.v
+++ b/vlib/v/checker/fn.v
@@ -1741,11 +1741,12 @@ fn (mut c Checker) map_builtin_method_call(mut node ast.CallExpr, left_type ast.
 			if method_name[0] == `m` {
 				c.fail_if_immutable(node.left)
 			}
-			if node.left.is_auto_deref_var() {
+			if node.left.is_auto_deref_var() || ret_type.has_flag(.shared_f) {
 				ret_type = left_type.deref()
 			} else {
 				ret_type = left_type
 			}
+			ret_type = ret_type.clear_flag(.shared_f)
 		}
 		'keys' {
 			info := left_sym.info as ast.Map
@@ -1855,12 +1856,18 @@ fn (mut c Checker) array_builtin_method_call(mut node ast.CallExpr, left_type as
 			else { arg_type }
 		}
 		node.return_type = c.table.find_or_register_array(c.unwrap_generic(ret_type))
+		if node.return_type.has_flag(.shared_f) {
+			node.return_type = node.return_type.clear_flag(.shared_f).deref()
+		}
 		ret_sym := c.table.sym(ret_type)
 		if ret_sym.kind == .multi_return {
 			c.error('returning multiple values is not supported in .map() calls', node.pos)
 		}
 	} else if method_name == 'filter' {
 		// check fn
+		if node.return_type.has_flag(.shared_f) {
+			node.return_type = node.return_type.clear_flag(.shared_f).deref()
+		}
 		c.check_map_and_filter(false, elem_typ, node)
 	} else if method_name in ['any', 'all'] {
 		c.check_map_and_filter(false, elem_typ, node)
@@ -1874,6 +1881,9 @@ fn (mut c Checker) array_builtin_method_call(mut node ast.CallExpr, left_type as
 		} else {
 			node.return_type = node.receiver_type.set_nr_muls(0)
 		}
+		if node.return_type.has_flag(.shared_f) {
+			node.return_type = node.return_type.clear_flag(.shared_f)
+		}
 	} else if method_name == 'sort' {
 		node.return_type = ast.void_type
 	} else if method_name == 'contains' {
diff --git a/vlib/v/gen/c/array.v b/vlib/v/gen/c/array.v
index 39eb7b524d..86dd3ea40b 100644
--- a/vlib/v/gen/c/array.v
+++ b/vlib/v/gen/c/array.v
@@ -305,15 +305,23 @@ fn (mut g Gen) gen_array_map(node ast.CallExpr) {
 	}
 	g.empty_line = true
 	noscan := g.check_noscan(ret_info.elem_type)
-	g.writeln('$ret_typ $tmp = __new_array${noscan}(0, 0, sizeof($ret_elem_type));')
+	g.writeln('$ret_typ $tmp = {0};')
 	has_infix_left_var_name := g.infix_left_var_name.len > 0
 	if has_infix_left_var_name {
 		g.writeln('if ($g.infix_left_var_name) {')
 		g.infix_left_var_name = ''
 		g.indent++
 	}
-	g.write('${g.typ(node.left_type)} ${tmp}_orig = ')
+	left_type := if node.left_type.has_flag(.shared_f) {
+		node.left_type.clear_flag(.shared_f).deref()
+	} else {
+		node.left_type
+	}
+	g.write('${g.typ(left_type)} ${tmp}_orig = ')
 	g.expr(node.left)
+	if node.left_type.has_flag(.shared_f) {
+		g.write('->val')
+	}
 	g.writeln(';')
 	g.writeln('int ${tmp}_len = ${tmp}_orig.len;')
 	g.writeln('$tmp = __new_array${noscan}(0, ${tmp}_len, sizeof($ret_elem_type));\n')
@@ -403,11 +411,11 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
 	// the only argument can only be an infix expression like `a < b` or `b.field > a.field`
 	if node.args.len == 0 {
 		comparison_type = g.unwrap(info.elem_type.set_nr_muls(0))
-		shared a := g.array_sort_fn
-		array_sort_fn := a.clone()
-		if compare_fn in array_sort_fn {
-			g.gen_array_sort_call(node, compare_fn)
-			return
+		rlock g.array_sort_fn {
+			if compare_fn in g.array_sort_fn {
+				g.gen_array_sort_call(node, compare_fn)
+				return
+			}
 		}
 		left_expr = '*a'
 		right_expr = '*b'
@@ -424,11 +432,11 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
 		if is_reverse {
 			compare_fn += '_reverse'
 		}
-		shared a := g.array_sort_fn
-		array_sort_fn := a.clone()
-		if compare_fn in array_sort_fn {
-			g.gen_array_sort_call(node, compare_fn)
-			return
+		rlock g.array_sort_fn {
+			if compare_fn in g.array_sort_fn {
+				g.gen_array_sort_call(node, compare_fn)
+				return
+			}
 		}
 		if left_name.starts_with('a') != is_reverse {
 			left_expr = g.expr_string(infix_expr.left)
@@ -503,15 +511,23 @@ fn (mut g Gen) gen_array_filter(node ast.CallExpr) {
 	elem_type_str := g.typ(info.elem_type)
 	g.empty_line = true
 	noscan := g.check_noscan(info.elem_type)
-	g.writeln('$styp $tmp = __new_array${noscan}(0, 0, sizeof($elem_type_str));')
+	g.writeln('$styp $tmp = {0};')
 	has_infix_left_var_name := g.infix_left_var_name.len > 0
 	if has_infix_left_var_name {
 		g.writeln('if ($g.infix_left_var_name) {')
 		g.infix_left_var_name = ''
 		g.indent++
 	}
-	g.write('${g.typ(node.left_type)} ${tmp}_orig = ')
+	left_type := if node.left_type.has_flag(.shared_f) {
+		node.left_type.clear_flag(.shared_f).deref()
+	} else {
+		node.left_type
+	}
+	g.write('${g.typ(left_type)} ${tmp}_orig = ')
 	g.expr(node.left)
+	if node.left_type.has_flag(.shared_f) {
+		g.write('->val')
+	}
 	g.writeln(';')
 	g.writeln('int ${tmp}_len = ${tmp}_orig.len;')
 	g.writeln('$tmp = __new_array${noscan}(0, ${tmp}_len, sizeof($elem_type_str));\n')
diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v
index 0f5db0083a..ff7825263e 100644
--- a/vlib/v/gen/c/assign.v
+++ b/vlib/v/gen/c/assign.v
@@ -379,7 +379,8 @@ fn (mut g Gen) gen_assign_stmt(node_ ast.AssignStmt) {
 				g.write(', ')
 			}
 			mut cloned := false
-			if g.is_autofree && right_sym.kind in [.array, .string] {
+			if g.is_autofree && right_sym.kind in [.array, .string]
+				&& !unwrapped_val_type.has_flag(.shared_f) {
 				if g.gen_clone_assignment(val, unwrapped_val_type, false) {
 					cloned = true
 				}
diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v
index 60afc89800..c77803429a 100644
--- a/vlib/v/gen/c/cgen.v
+++ b/vlib/v/gen/c/cgen.v
@@ -984,7 +984,10 @@ fn (mut g Gen) register_optional(t ast.Type) string {
 }
 
 fn (mut g Gen) write_optionals() {
-	mut done := g.done_optionals.clone()
+	mut done := []string{}
+	rlock g.done_optionals {
+		done = g.done_optionals
+	}
 	for base, styp in g.optionals {
 		if base in done {
 			continue
diff --git a/vlib/v/tests/filter_test.v b/vlib/v/tests/filter_test.v
new file mode 100644
index 0000000000..57461888c7
--- /dev/null
+++ b/vlib/v/tests/filter_test.v
@@ -0,0 +1,22 @@
+fn test_filter() {
+	arr := [3, 6, 1, 8, 2, 4, 5, 7]
+	filtered := arr.filter(it > 4)
+	assert filtered.len == 4
+	assert filtered.contains(6)
+	assert filtered.contains(8)
+	assert filtered.contains(5)
+	assert filtered.contains(7)
+}
+
+fn test_shared_filter() {
+	shared arr := [3, 6, 1, 8, 2, 4, 5, 7]
+
+	filtered := rlock arr {
+		arr.filter(it > 4)
+	}
+	assert filtered.len == 4
+	assert filtered.contains(6)
+	assert filtered.contains(8)
+	assert filtered.contains(5)
+	assert filtered.contains(7)
+}
diff --git a/vlib/v/tests/map_fn_test.v b/vlib/v/tests/map_fn_test.v
new file mode 100644
index 0000000000..08eb874ebe
--- /dev/null
+++ b/vlib/v/tests/map_fn_test.v
@@ -0,0 +1,8 @@
+fn test_map_fn() {
+	map1 := [3, 4, 6]
+	assert map1.map(it * 2) == [6, 8, 12]
+	shared map2 := [7, 9, 10]
+	rlock map2 {
+		assert map2.map(it * 2) == [14, 18, 20]
+	}
+}
diff --git a/vlib/v/tests/shared_autolock_test.v b/vlib/v/tests/shared_autolock_test.v
index cf9f6b6418..fc41047f90 100644
--- a/vlib/v/tests/shared_autolock_test.v
+++ b/vlib/v/tests/shared_autolock_test.v
@@ -13,7 +13,9 @@ fn test_autolock_array() {
 		a[2]++
 	}
 	t.wait()
-	assert a[2] == 2 * iterations + 7
+	rlock a {
+		assert a[2] == 2 * iterations + 7
+	}
 }
 
 fn inc_map_elem(shared b map[string]int, k string) {
@@ -34,5 +36,7 @@ fn test_autolock_map() {
 		m['asd']++
 	}
 	t.wait()
-	assert m['asd'] == 2 * iterations + 7
+	rlock m {
+		assert m['asd'] == 2 * iterations + 7
+	}
 }
diff --git a/vlib/v/tests/shared_in_test.v b/vlib/v/tests/shared_in_test.v
index 1fbe2c0975..0cd29cd90a 100644
--- a/vlib/v/tests/shared_in_test.v
+++ b/vlib/v/tests/shared_in_test.v
@@ -1,8 +1,10 @@
 fn test_shared_in() {
 	shared a := [1, 3, 7, 3]
-	assert 1 in a
-	assert 0 !in a
-	assert 7 in a
-	assert 3 in a
-	assert 1238941 !in a
+	rlock a {
+		assert 1 in a
+		assert 0 !in a
+		assert 7 in a
+		assert 3 in a
+		assert 1238941 !in a
+	}
 }
diff --git a/vlib/vweb/tests/vweb_test_server.v b/vlib/vweb/tests/vweb_test_server.v
index 5d4cbff1e5..6af589aff3 100644
--- a/vlib/vweb/tests/vweb_test_server.v
+++ b/vlib/vweb/tests/vweb_test_server.v
@@ -51,7 +51,9 @@ fn main() {
 //}
 
 pub fn (mut app App) index() vweb.Result {
-	assert app.global_config.max_ping == 50
+	rlock app.global_config {
+		assert app.global_config.max_ping == 50
+	}
 	return app.text('Welcome to VWeb')
 }