checker, cgen: checks for shared/lock, first autolock (#5815)

vlib/v/ast/ast.v

@@ -396,6 +396,7 @@ pub:
 pub mut:
 	left_type    table.Type
 	right_type   table.Type
+	auto_locked  string
 }
 
 pub struct PostfixExpr {

vlib/v/checker/checker.v

@@ -31,6 +31,8 @@ pub mut:
 	const_decl       string
 	const_deps       []string
 	const_names      []string
+	locked_names     []string // vars that are currently locked
+	rlocked_names    []string // vars that are currently read-locked
 	pref             &pref.Preferences // Preferences shared from V struct
 	in_for_count     int // if checker is currently in an for loop
 	// checked_ident  string // to avoid infinit checker loops
@@ -571,7 +573,7 @@ pub fn (mut c Checker) infix_expr(mut infix_expr ast.InfixExpr) table.Type {
 		.left_shift {
 			if left.kind == .array {
 				// `array << elm`
-				c.fail_if_immutable(infix_expr.left)
+				infix_expr.auto_locked, _ = c.fail_if_immutable(infix_expr.left)
 				left_value_type := c.table.value_type(left_type)
 				left_value_sym := c.table.get_type_symbol(left_value_type)
 				if left_value_sym.kind == .interface_ {
@@ -656,11 +658,15 @@ pub fn (mut c Checker) infix_expr(mut infix_expr ast.InfixExpr) table.Type {
 	}
 }
 
-fn (mut c Checker) fail_if_immutable(expr ast.Expr) {
+// returns name and position of variable that needs write lock
+fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) {
+	mut to_lock := '' // name of variable that needs lock
+	mut pos := token.Position{} // and its position
+	mut explicit_lock_needed := false
 	match expr {
 		ast.CastExpr {
 			// TODO
-			return
+			return '', pos
 		}
 		ast.Ident {
 			scope := c.file.scope.innermost(expr.pos.pos)
@@ -670,24 +676,30 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) {
 						expr.pos)
 				}
 				v.is_changed = true
+				if v.typ.share() == .shared_t {
+					if expr.name !in c.locked_names {
+						to_lock = expr.name
+						pos = expr.pos
+					}
+				}
 			} else if expr.name in c.const_names {
 				c.error('cannot modify constant `$expr.name`', expr.pos)
 			}
 		}
 		ast.IndexExpr {
-			c.fail_if_immutable(expr.left)
+			to_lock, pos = c.fail_if_immutable(expr.left)
 		}
 		ast.ParExpr {
-			c.fail_if_immutable(expr.expr)
+			to_lock, pos = c.fail_if_immutable(expr.expr)
 		}
 		ast.PrefixExpr {
-			c.fail_if_immutable(expr.right)
+			to_lock, pos = c.fail_if_immutable(expr.right)
 		}
 		ast.SelectorExpr {
 			// retrieve table.Field
 			if expr.expr_type == 0 {
 				c.error('0 type in SelectorExpr', expr.pos)
-				return
+				return '', pos
 			}
 			typ_sym := c.table.get_type_symbol(c.unwrap_generic(expr.expr_type))
 			match typ_sym.kind {
@@ -696,14 +708,18 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) {
 					field_info := struct_info.find_field(expr.field_name) or {
 						type_str := c.table.type_to_str(expr.expr_type)
 						c.error('unknown field `${type_str}.$expr.field_name`', expr.pos)
-						return
+						return '', pos
 					}
 					if !field_info.is_mut {
 						type_str := c.table.type_to_str(expr.expr_type)
 						c.error('field `$expr.field_name` of struct `$type_str` is immutable',
 							expr.pos)
 					}
-					c.fail_if_immutable(expr.expr)
+					to_lock, pos = c.fail_if_immutable(expr.expr)
+					if to_lock != '' {
+						// No automatic lock for struct access
+						explicit_lock_needed = true
+					}
 				}
 				.array, .string {
 					// This should only happen in `builtin`
@@ -721,18 +737,27 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) {
 		ast.CallExpr {
 			// TODO: should only work for builtin method
 			if expr.name == 'slice' {
-				return
+				to_lock, pos = c.fail_if_immutable(expr.left)
+				if to_lock != '' {
+					// No automatic lock for array slicing (yet(?))
+					explicit_lock_needed = true
+				}
 			} else {
 				c.error('cannot use function call as mut', expr.pos)
 			}
 		}
 		ast.ArrayInit {
-			return
+			return '', pos
 		}
 		else {
 			c.error('unexpected expression `${typeof(expr)}`', expr.position())
 		}
 	}
+	if explicit_lock_needed {
+		c.error('`$to_lock` is `shared` and needs explicit lock for `${typeof(expr)}`', pos)
+		to_lock = ''
+	}
+	return to_lock, pos
 }
 
 pub fn (mut c Checker) call_expr(mut call_expr ast.CallExpr) table.Type {
@@ -2626,10 +2651,33 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, type_sym table.TypeSymbol
 }
 
 pub fn (mut c Checker) lock_expr(mut node ast.LockExpr) table.Type {
+	scope := c.file.scope.innermost(node.pos.pos)
 	for id in node.lockeds {
 		c.ident(mut id)
+		if v := scope.find_var(id.name) {
+			if v.typ.share() != .shared_t {
+				c.error('`$id.name` must be declared `shared` to be locked', id.pos)
+			}
+		} else {
+			c.error('`$id.name` is not a variable and cannot be locked', id.pos)
+		}
+		if id.name in c.locked_names {
+			c.error('`$id.name` is already locked', id.pos)
+		} else if id.name in c.rlocked_names {
+			c.error('`$id.name` is already read-locked', id.pos)
+		}
+		if node.is_rlock {
+			c.rlocked_names << id.name
+		} else {
+			c.locked_names << id.name
+		}
 	}
 	c.stmts(node.stmts)
+	if node.is_rlock {
+		c.rlocked_names = c.rlocked_names[.. c.rlocked_names.len - node.lockeds.len]
+	} else {
+		c.locked_names = c.locked_names[.. c.locked_names.len - node.lockeds.len]
+	}
 	// void for now... maybe sometime `x := lock a { a.getval() }`
 	return table.void_type
 }

vlib/v/checker/tests/lock_already_locked.out

@@ -0,0 +1,7 @@
+vlib/v/checker/tests/lock_already_locked.v:11:9: error: `a` is already locked 
+    9 |     }
+   10 |     lock a {
+   11 |         rlock a {
+      |               ^
+   12 |             a.x++
+   13 |         }

vlib/v/checker/tests/lock_already_locked.vv

@@ -0,0 +1,16 @@
+struct St {
+mut:
+	x int
+}
+
+fn main() {
+	shared a := &St{
+		x: 5
+	}
+	lock a {
+		rlock a {
+			a.x++
+		}
+	}
+	println(a.x)
+}

vlib/v/checker/tests/lock_already_rlocked.out

@@ -0,0 +1,7 @@
+vlib/v/checker/tests/lock_already_rlocked.v:11:8: error: `a` is already read-locked 
+    9 |     }
+   10 |     rlock a {
+   11 |         lock a {
+      |              ^
+   12 |             a.x++
+   13 |         }

vlib/v/checker/tests/lock_already_rlocked.vv

@@ -0,0 +1,16 @@
+struct St {
+mut:
+	x int
+}
+
+fn main() {
+	shared a := &St{
+		x: 5
+	}
+	rlock a {
+		lock a {
+			a.x++
+		}
+	}
+	println(a.x)
+}

vlib/v/checker/tests/lock_const.out

@@ -0,0 +1,7 @@
+vlib/v/checker/tests/lock_const.v:7:8: error: `a` is not a variable and cannot be locked 
+    5 | fn main() {
+    6 |     mut c := 0
+    7 |     rlock a {
+      |           ^
+    8 |         c = a
+    9 |     }

vlib/v/checker/tests/lock_const.vv

@@ -0,0 +1,11 @@
+const (
+	a = 5
+)
+
+fn main() {
+	mut c := 0
+	rlock a {
+		c = a
+	}
+	println(c)
+}

vlib/v/checker/tests/lock_needed.out

@@ -0,0 +1,7 @@
+vlib/v/checker/tests/lock_needed.v:10:2: error: `abc` is `shared` and needs explicit lock for `v.ast.SelectorExpr` 
+    8 |         x: 5
+    9 |     }
+   10 |     abc.x++
+      |     ~~~
+   11 |     println(abc.x)
+   12 | }

vlib/v/checker/tests/lock_needed.vv

@@ -0,0 +1,12 @@
+struct St {
+mut:
+	x int
+}
+
+fn main() {
+	shared abc := &St{
+		x: 5
+	}
+	abc.x++
+	println(abc.x)
+}

vlib/v/checker/tests/lock_nonshared.out

@@ -0,0 +1,7 @@
+vlib/v/checker/tests/lock_nonshared.v:10:7: error: `a` must be declared `shared` to be locked 
+    8 |         x: 5
+    9 |     }
+   10 |     lock a {
+      |          ^
+   11 |         a.x++
+   12 |     }

vlib/v/checker/tests/lock_nonshared.vv

@@ -0,0 +1,14 @@
+struct St {
+mut:
+	x int
+}
+
+fn main() {
+	mut a := &St{
+		x: 5
+	}
+	lock a {
+		a.x++
+	}
+	println(a)
+}

vlib/v/gen/cgen.v

@@ -1892,6 +1892,9 @@ fn (mut g Gen) infix_expr(node ast.InfixExpr) {
 	// }
 	// string + string, string == string etc
 	// g.infix_op = node.op
+	if node.auto_locked != '' {
+		g.writeln('sync__RwMutex_w_lock($node.auto_locked->mtx);')
+	}
 	left_type := g.unwrap_generic(node.left_type)
 	left_sym := g.table.get_type_symbol(left_type)
 	unaliased_left := if left_sym.kind == .alias { (left_sym.info as table.Alias).parent_type } else { left_type }
@@ -2121,6 +2124,10 @@ fn (mut g Gen) infix_expr(node ast.InfixExpr) {
 			}
 		}
 	}
+	if node.auto_locked != '' {
+		g.writeln(';')
+		g.write('sync__RwMutex_w_unlock($node.auto_locked->mtx)')
+	}
 }
 
 fn (mut g Gen) lock_expr(node ast.LockExpr) {

vlib/v/tests/autolock_array_1.v

@@ -0,0 +1,52 @@
+import sync
+import time
+
+const (
+	iterations_per_thread = 100000
+)
+
+fn add_elements(shared foo []int, n int) {
+	for _ in 0 .. iterations_per_thread {
+		foo << n
+	}
+	// automatic lock is not yet implemented for this...
+	lock foo {
+		foo[0]++
+	}
+}
+
+fn test_autolocked_array() {
+	shared abc := &[0]
+	go add_elements(shared abc, 1)
+	go add_elements(shared abc, 3)
+	for _ in 0 .. iterations_per_thread {
+		abc << 0
+	}
+	// wait for coroutines to finish - that should really be
+	// done by channels, yield, semaphore...
+	for {
+		mut finished_threads := 0
+		rlock abc {
+			finished_threads = abc[0]
+		}
+		if finished_threads == 2 {
+			break
+		}
+		time.sleep_ms(100)
+	}
+	// create histogram of results
+	mut result := [0, 0, 0, 0]
+	rlock abc {
+		// automatic rlock for iteration is also not implemented, yet
+		for v in abc {
+			if v > 3 {
+				panic('unexpected element on array')
+			}
+			result[v]++
+		}
+	}
+	assert result[0] == iterations_per_thread
+	assert result[1] == iterations_per_thread
+	assert result[2] == 1 // number of non-main threads
+	assert result[3] == iterations_per_thread
+}