shared: support `lock` on SelectorExpression (#10105)

vlib/v/ast/ast.v

@@ -740,10 +740,11 @@ pub:
 	is_rlock []bool
 	pos      token.Position
 pub mut:
-	lockeds []Ident // `x`, `y` in `lock x, y {`
+	lockeds  []Expr // `x`, `y.z` in `lock x, y.z {`
-	is_expr bool
+	comments []Comment
-	typ     Type
+	is_expr  bool
-	scope   &Scope
+	typ      Type
+	scope    &Scope
 }
 
 pub struct MatchExpr {
@@ -1596,6 +1597,21 @@ pub fn (expr Expr) is_auto_deref_var() bool {
 	return false
 }
 
+// returns if an expression can be used in `lock x, y.z {`
+pub fn (e &Expr) is_lockable() bool {
+	match e {
+		Ident {
+			return true
+		}
+		SelectorExpr {
+			return e.expr.is_lockable()
+		}
+		else {
+			return false
+		}
+	}
+}
+
 // check if stmt can be an expression in C
 pub fn (stmt Stmt) check_c_expr() ? {
 	match stmt {

vlib/v/checker/check_types.v

@@ -401,7 +401,7 @@ pub fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what stri
 			if typ.has_flag(.shared_f) {
 				if expr.name !in c.rlocked_names && expr.name !in c.locked_names {
 					action := if what == 'argument' { 'passed' } else { 'used' }
-					c.error('$expr.name is `shared` and must be `rlock`ed or `lock`ed to be $action as non-mut $what',
+					c.error('`$expr.name` is `shared` and must be `rlock`ed or `lock`ed to be $action as non-mut $what',
 						expr.pos)
 				}
 			}
@@ -409,7 +409,17 @@ pub fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what stri
 		}
 		ast.SelectorExpr {
 			pos = expr.pos
-			c.fail_if_unreadable(expr.expr, expr.expr_type, what)
+			if typ.has_flag(.shared_f) {
+				expr_name := '${expr.expr}.$expr.field_name'
+				if expr_name !in c.rlocked_names && expr_name !in c.locked_names {
+					action := if what == 'argument' { 'passed' } else { 'used' }
+					c.error('`$expr_name` is `shared` and must be `rlock`ed or `lock`ed to be $action as non-mut $what',
+						expr.pos)
+				}
+				return
+			} else {
+				c.fail_if_unreadable(expr.expr, expr.expr_type, what)
+			}
 		}
 		ast.IndexExpr {
 			pos = expr.left.position().extend(expr.pos)

vlib/v/checker/checker.v

@@ -1401,17 +1401,29 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) {
 						c.error('unknown field `${type_str}.$expr.field_name`', expr.pos)
 						return '', pos
 					}
-					if !field_info.is_mut && !c.pref.translated {
-						type_str := c.table.type_to_str(expr.expr_type)
-						c.error('field `$expr.field_name` of struct `$type_str` is immutable',
-							expr.pos)
-					}
 					if field_info.typ.has_flag(.shared_f) {
-						type_str := c.table.type_to_str(expr.expr_type)
+						expr_name := '${expr.expr}.$expr.field_name'
-						c.error('you have to create a handle and `lock` it to modify `shared` field `$expr.field_name` of struct `$type_str`',
+						if expr_name !in c.locked_names {
-							expr.pos)
+							if c.locked_names.len > 0 || c.rlocked_names.len > 0 {
+								if expr_name in c.rlocked_names {
+									c.error('$expr_name has an `rlock` but needs a `lock`',
+										expr.pos)
+								} else {
+									c.error('$expr_name must be added to the `lock` list above',
+										expr.pos)
+								}
+							}
+							to_lock = expr_name
+							pos = expr.pos
+						}
+					} else {
+						if !field_info.is_mut && !c.pref.translated {
+							type_str := c.table.type_to_str(expr.expr_type)
+							c.error('field `$expr.field_name` of struct `$type_str` is immutable',
+								expr.pos)
+						}
+						to_lock, 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
@@ -5679,24 +5691,22 @@ pub fn (mut c Checker) lock_expr(mut node ast.LockExpr) ast.Type {
 		c.error('nested `lock`/`rlock` not allowed', node.pos)
 	}
 	for i in 0 .. node.lockeds.len {
-		c.ident(mut node.lockeds[i])
+		e_typ := c.expr(node.lockeds[i])
-		id := node.lockeds[i]
+		id_name := node.lockeds[i].str()
-		if mut id.obj is ast.Var {
+		if !e_typ.has_flag(.shared_f) {
-			if id.obj.typ.share() != .shared_t {
+			obj_type := if node.lockeds[i] is ast.Ident { 'variable' } else { 'struct element' }
-				c.error('`$id.name` must be declared `shared` to be locked', id.pos)
+			c.error('`$id_name` must be declared as `shared` $obj_type to be locked',
-			}
+				node.lockeds[i].position())
-		} else {
-			c.error('`$id.name` is not a variable and cannot be locked', id.pos)
 		}
-		if id.name in c.locked_names {
+		if id_name in c.locked_names {
-			c.error('`$id.name` is already locked', id.pos)
+			c.error('`$id_name` is already locked', node.lockeds[i].position())
-		} else if id.name in c.rlocked_names {
+		} else if id_name in c.rlocked_names {
-			c.error('`$id.name` is already read-locked', id.pos)
+			c.error('`$id_name` is already read-locked', node.lockeds[i].position())
 		}
 		if node.is_rlock[i] {
-			c.rlocked_names << id.name
+			c.rlocked_names << id_name
 		} else {
-			c.locked_names << id.name
+			c.locked_names << id_name
 		}
 	}
 	c.stmts(node.stmts)

vlib/v/checker/tests/lock_already_locked.out

@@ -5,7 +5,7 @@ vlib/v/checker/tests/lock_already_locked.vv:11:3: error: nested `lock`/`rlock` n
       |         ~~~~~
    12 |             a.x++
    13 |         }
-vlib/v/checker/tests/lock_already_locked.vv:15:10: error: a is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
+vlib/v/checker/tests/lock_already_locked.vv:15:10: error: `a` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
    13 |         }
    14 |     }
    15 |     println(a.x)

vlib/v/checker/tests/lock_already_rlocked.out

@@ -5,7 +5,7 @@ vlib/v/checker/tests/lock_already_rlocked.vv:11:3: error: nested `lock`/`rlock`
       |         ~~~~
    12 |             a.x++
    13 |         }
-vlib/v/checker/tests/lock_already_rlocked.vv:15:10: error: a is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
+vlib/v/checker/tests/lock_already_rlocked.vv:15:10: error: `a` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
    13 |         }
    14 |     }
    15 |     println(a.x)

vlib/v/checker/tests/lock_const.out

@@ -1,4 +1,4 @@
-vlib/v/checker/tests/lock_const.vv:7:8: error: `a` is not a variable and cannot be locked 
+vlib/v/checker/tests/lock_const.vv:7:8: error: `a` must be declared as `shared` variable to be locked
     5 | fn main() {
     6 |     mut c := 0
     7 |     rlock a {

vlib/v/checker/tests/lock_needed.out

@@ -5,21 +5,21 @@ vlib/v/checker/tests/lock_needed.vv:10:2: error: `abc` is `shared` and needs exp
       |     ~~~
    11 |     println(abc.x)
    12 | }
-vlib/v/checker/tests/lock_needed.vv:11:10: error: abc is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
+vlib/v/checker/tests/lock_needed.vv:11:10: error: `abc` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
     9 |     }
    10 |     abc.x++
    11 |     println(abc.x)
       |             ~~~
    12 | }
    13 |
-vlib/v/checker/tests/lock_needed.vv:25:12: error: you have to create a handle and `rlock` it to use a `shared` element as non-mut argument to print
+vlib/v/checker/tests/lock_needed.vv:25:12: error: `a.st` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
    23 |         }
    24 |     }
    25 |     println(a.st.x)
       |               ~~
    26 | }
    27 |
-vlib/v/checker/tests/lock_needed.vv:30:10: error: a is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
+vlib/v/checker/tests/lock_needed.vv:30:10: error: `a` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
    28 | fn g() {
    29 |     shared a := []f64{len: 10, init: 7.5}
    30 |     println(a[3])

vlib/v/checker/tests/lock_nonshared.out

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

vlib/v/checker/tests/shared_bad_args.out

@@ -1,25 +1,25 @@
-vlib/v/checker/tests/shared_bad_args.vv:43:8: error: r is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut receiver
+vlib/v/checker/tests/shared_bad_args.vv:43:8: error: `r` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut receiver
    41 |     shared r := Qr{ a: 7 }
    42 |     lock s {
    43 |         u := r.s_val(s)
       |              ^
    44 |         println(u)
    45 |     }
-vlib/v/checker/tests/shared_bad_args.vv:47:16: error: s is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument
+vlib/v/checker/tests/shared_bad_args.vv:47:16: error: `s` is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument
    45 |     }
    46 |     lock r {
    47 |         v := r.s_val(s)
       |                      ^
    48 |         println(v)
    49 |     }
-vlib/v/checker/tests/shared_bad_args.vv:50:13: error: m is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument
+vlib/v/checker/tests/shared_bad_args.vv:50:13: error: `m` is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument
    48 |         println(v)
    49 |     }
    50 |     w := m_val(m)
       |                ^
    51 |     x := a_val(a)
    52 |     println('$w $x')
-vlib/v/checker/tests/shared_bad_args.vv:51:13: error: a is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument
+vlib/v/checker/tests/shared_bad_args.vv:51:13: error: `a` is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument
    49 |     }
    50 |     w := m_val(m)
    51 |     x := a_val(a)
@@ -54,28 +54,28 @@ vlib/v/checker/tests/shared_bad_args.vv:67:12: error: a is `shared` and must be
       |               ^
    68 | }
    69 |
-vlib/v/checker/tests/shared_bad_args.vv:76:10: error: y is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
+vlib/v/checker/tests/shared_bad_args.vv:76:10: error: `y` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
    74 | fn main() {
    75 |     shared y := St{ a: 5 }
    76 |     println(y)
       |             ^
    77 |     println('$y')
    78 |     a := Ab{ s: St{ a: 3 } }
-vlib/v/checker/tests/shared_bad_args.vv:77:12: error: y is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut interpolation object
+vlib/v/checker/tests/shared_bad_args.vv:77:12: error: `y` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut interpolation object
    75 |     shared y := St{ a: 5 }
    76 |     println(y)
    77 |     println('$y')
       |               ^
    78 |     a := Ab{ s: St{ a: 3 } }
    79 |     println(a.s)
-vlib/v/checker/tests/shared_bad_args.vv:79:12: error: you have to create a handle and `rlock` it to use a `shared` element as non-mut argument to print
+vlib/v/checker/tests/shared_bad_args.vv:79:12: error: `a.s` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut argument to print
    77 |     println('$y')
    78 |     a := Ab{ s: St{ a: 3 } }
    79 |     println(a.s)
       |               ^
    80 |     println('$a.s')
    81 | }
-vlib/v/checker/tests/shared_bad_args.vv:80:14: error: you have to create a handle and `rlock` it to use a `shared` element as non-mut interpolation object
+vlib/v/checker/tests/shared_bad_args.vv:80:14: error: `a.s` is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut interpolation object
    78 |     a := Ab{ s: St{ a: 3 } }
    79 |     println(a.s)
    80 |     println('$a.s')

vlib/v/checker/tests/shared_element_lock.out

@@ -1,4 +1,4 @@
-vlib/v/checker/tests/shared_element_lock.vv:36:5: error: you have to create a handle and `lock` it to modify `shared` field `pe` of struct `Programmer`
+vlib/v/checker/tests/shared_element_lock.vv:36:5: error: `pr.pe` is `shared` and needs explicit lock for `v.ast.SelectorExpr`
    34 |         }
    35 |     }
    36 |     pr.pe.color = 3

vlib/v/gen/c/cgen.v

@@ -3956,35 +3956,45 @@ fn (mut g Gen) lock_expr(node ast.LockExpr) {
 	if node.lockeds.len == 0 {
 		// this should not happen
 	} else if node.lockeds.len == 1 {
-		id := node.lockeds[0]
-		name := id.name
-		deref := if id.is_mut { '->' } else { '.' }
 		lock_prefix := if node.is_rlock[0] { 'r' } else { '' }
-		g.writeln('sync__RwMutex_${lock_prefix}lock(&$name${deref}mtx);')
+		g.write('sync__RwMutex_${lock_prefix}lock(&')
+		g.expr(node.lockeds[0])
+		g.writeln('->mtx);')
 	} else {
 		mtxs = g.new_tmp_var()
 		g.writeln('uintptr_t _arr_$mtxs[$node.lockeds.len];')
 		g.writeln('bool _isrlck_$mtxs[$node.lockeds.len];')
 		mut j := 0
-		for i, id in node.lockeds {
+		for i, is_rlock in node.is_rlock {
-			if !node.is_rlock[i] {
+			if !is_rlock {
-				name := id.name
+				g.write('_arr_$mtxs[$j] = (uintptr_t)&')
-				deref := if id.is_mut { '->' } else { '.' }
+				g.expr(node.lockeds[i])
-				g.writeln('_arr_$mtxs[$j] = (uintptr_t)&$name${deref}mtx;')
+				g.writeln('->mtx;')
 				g.writeln('_isrlck_$mtxs[$j] = false;')
 				j++
 			}
 		}
-		for i, id in node.lockeds {
+		for i, is_rlock in node.is_rlock {
-			if node.is_rlock[i] {
+			if is_rlock {
-				name := id.name
+				g.write('_arr_$mtxs[$j] = (uintptr_t)&')
-				deref := if id.is_mut { '->' } else { '.' }
+				g.expr(node.lockeds[i])
-				g.writeln('_arr_$mtxs[$j] = (uintptr_t)&$name${deref}mtx;')
+				g.writeln('->mtx;')
 				g.writeln('_isrlck_$mtxs[$j] = true;')
 				j++
 			}
 		}
-		g.writeln('__sort_ptr(_arr_$mtxs, _isrlck_$mtxs, $node.lockeds.len);')
+		if node.lockeds.len == 2 {
+			g.writeln('if (_arr_$mtxs[0] > _arr_$mtxs[1]) {')
+			g.writeln('\tuintptr_t _ptr_$mtxs = _arr_$mtxs[0];')
+			g.writeln('\t_arr_$mtxs[0] = _arr_$mtxs[1];')
+			g.writeln('\t_arr_$mtxs[1] = _ptr_$mtxs;')
+			g.writeln('\tbool _bool_$mtxs = _isrlck_$mtxs[0];')
+			g.writeln('\t_isrlck_$mtxs[0] = _isrlck_$mtxs[1];')
+			g.writeln('\t_isrlck_$mtxs[1] = _bool_$mtxs;')
+			g.writeln('}')
+		} else {
+			g.writeln('__sort_ptr(_arr_$mtxs, _isrlck_$mtxs, $node.lockeds.len);')
+		}
 		g.writeln('for (int $mtxs=0; $mtxs<$node.lockeds.len; $mtxs++) {')
 		g.writeln('\tif ($mtxs && _arr_$mtxs[$mtxs] == _arr_$mtxs[$mtxs-1]) continue;')
 		g.writeln('\tif (_isrlck_$mtxs[$mtxs])')
@@ -3993,12 +4003,10 @@ fn (mut g Gen) lock_expr(node ast.LockExpr) {
 		g.writeln('\t\tsync__RwMutex_lock((sync__RwMutex*)_arr_$mtxs[$mtxs]);')
 		g.writeln('}')
 	}
-	println('')
 	g.mtxs = mtxs
 	defer {
 		g.mtxs = ''
 	}
-
 	g.writeln('/*lock*/ {')
 	g.stmts_with_tmp_var(node.stmts, tmp_result)
 	if node.is_expr {
@@ -4016,11 +4024,10 @@ fn (mut g Gen) lock_expr(node ast.LockExpr) {
 fn (mut g Gen) unlock_locks() {
 	if g.cur_lock.lockeds.len == 0 {
 	} else if g.cur_lock.lockeds.len == 1 {
-		id := g.cur_lock.lockeds[0]
-		name := id.name
-		deref := if id.is_mut { '->' } else { '.' }
 		lock_prefix := if g.cur_lock.is_rlock[0] { 'r' } else { '' }
-		g.writeln('sync__RwMutex_${lock_prefix}unlock(&$name${deref}mtx);')
+		g.write('sync__RwMutex_${lock_prefix}unlock(&')
+		g.expr(g.cur_lock.lockeds[0])
+		g.write('->mtx);')
 	} else {
 		g.writeln('for (int $g.mtxs=${g.cur_lock.lockeds.len - 1}; $g.mtxs>=0; $g.mtxs--) {')
 		g.writeln('\tif ($g.mtxs && _arr_$g.mtxs[$g.mtxs] == _arr_$g.mtxs[$g.mtxs-1]) continue;')

vlib/v/parser/lock.v

@@ -1,43 +1,102 @@
 module parser
 
+import v.token
 import v.ast
 
+// parse `x` or `x.y.z` - no index, no struct literals (`{` starts lock block)
+fn (mut p Parser) lockable() ast.Expr {
+	mut names := []string{}
+	mut positions := []token.Position{}
+	mut pos := p.tok.position()
+	for {
+		if p.tok.kind != .name {
+			p.error_with_pos('unexpected `$p.tok.lit` (field/variable name expected)',
+				p.tok.position())
+		}
+		names << p.tok.lit
+		positions << pos
+		p.next()
+		if p.tok.kind != .dot {
+			break
+		}
+		p.next()
+		pos.extend(p.tok.position())
+	}
+	mut expr := ast.Expr(ast.Ident{
+		language: ast.Language.v
+		pos: positions[0]
+		mod: p.mod
+		name: names[0]
+		is_mut: true
+		info: ast.IdentVar{}
+		scope: p.scope
+	})
+	for i := 1; i < names.len; i++ {
+		expr = ast.SelectorExpr{
+			expr: expr
+			field_name: names[i]
+			next_token: if i < names.len - 1 { token.Kind.dot } else { p.tok.kind }
+			is_mut: true
+			pos: positions[i]
+			scope: p.scope
+		}
+	}
+	return expr
+}
+
+// like `expr_list()` but only lockables are allowed, `{` starts lock block (not struct literal)
+fn (mut p Parser) lockable_list() ([]ast.Expr, []ast.Comment) {
+	mut exprs := []ast.Expr{}
+	mut comments := []ast.Comment{}
+	for {
+		expr := p.lockable()
+		if expr is ast.Comment {
+			comments << expr
+		} else {
+			exprs << expr
+			if p.tok.kind != .comma {
+				break
+			}
+			p.next()
+		}
+	}
+	return exprs, comments
+}
+
 fn (mut p Parser) lock_expr() ast.LockExpr {
 	// TODO Handle aliasing sync
 	p.register_auto_import('sync')
 	p.open_scope()
 	mut pos := p.tok.position()
-	mut lockeds := []ast.Ident{}
+	mut lockeds := []ast.Expr{}
+	mut comments := []ast.Comment{}
 	mut is_rlocked := []bool{}
-	outer: for {
+	for {
+		is_rlock := p.tok.kind == .key_rlock
+		if !is_rlock && p.tok.kind != .key_lock {
+			p.error_with_pos('unexpected `$p.tok`, expected `lock` or `rlock`', p.tok.position())
+		}
+		p.next()
 		if p.tok.kind == .lcbr {
 			break
 		}
-		is_rlock := p.tok.kind == .key_rlock
+		if p.tok.kind == .name {
-		if !is_rlock && p.tok.kind != .key_lock {
+			exprs, comms := p.lockable_list()
-			p.error_with_pos('unexpected $p.tok, expected `lock` or `rlock`', p.tok.position())
+			for e in exprs {
+				if !e.is_lockable() {
+					p.error_with_pos('`$e` cannot be locked - only `x` or `x.y` are supported',
+						e.position())
+				}
+				lockeds << e
+				is_rlocked << is_rlock
+			}
+			comments << comms
 		}
-		p.next()
+		if p.tok.kind == .lcbr {
-		for p.tok.kind == .name {
+			break
-			lockeds << ast.Ident{
+		}
-				language: ast.Language.v
+		if p.tok.kind == .semicolon {
-				pos: p.tok.position()
-				mod: p.mod
-				name: p.tok.lit
-				is_mut: true
-				info: ast.IdentVar{}
-				scope: p.scope
-			}
-			is_rlocked << is_rlock
 			p.next()
-			if p.tok.kind == .lcbr {
-				break outer
-			}
-			if p.tok.kind == .semicolon {
-				p.next()
-				break
-			}
-			p.check(.comma)
 		}
 	}
 	stmts := p.parse_block_no_scope(false)

vlib/v/tests/lock_selector_test.v

@@ -0,0 +1,50 @@
+struct St {
+mut:
+	x f64
+}
+
+fn (s &St) get_f64() f64 {
+	return s.x
+}
+
+struct Gen {
+	s shared St
+}
+
+fn (g Gen) set_val() bool {
+	lock g.s {
+		g.s.x = 6.25
+		if g.s.x == 6.25 {
+			return true
+		}
+		g.s.x == 7.125
+	}
+	return false
+}
+
+fn (g &Gen) inc_val() {
+	shared q := St{
+		x: 1.0
+	}
+	shared v := St{
+		x: 0.25
+	}
+	lock q, g.s, v {
+		g.s.x += q.x
+		g.s.x += v.x
+	}
+}
+
+fn test_lock_selector_expression() {
+	g := Gen{
+		s: St{
+			x: 12.5
+		}
+	}
+	g.set_val()
+	g.inc_val()
+	a := rlock g.s {
+		g.s.get_f64()
+	}
+	assert a == 7.5
+}