From 717aad4072b1c68d2c0fb8786270c0b86f6f5514 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Tue, 14 Apr 2026 12:45:24 +0300 Subject: [PATCH] checker: V error not allowing locking of element in array of shared structs (fixes #23449) --- vlib/v/ast/ast.v | 36 +++++++++++++++- vlib/v/checker/checker.v | 46 +++++++++++++++++---- vlib/v/parser/lock.v | 2 +- vlib/v/tests/concurrency/shared_elem_test.v | 26 ++++++++++++ 4 files changed, 101 insertions(+), 9 deletions(-) diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 13bf18da4..af531b6d8 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -2597,10 +2597,44 @@ pub fn (expr Expr) is_auto_deref_arg() bool { } } -// returns if an expression can be used in `lock x, y.z {` +// returns if an expression can be used as an index in `lock arr[i] {` +pub fn (e &Expr) is_lockable_index() bool { + return match e { + BoolLiteral, CharLiteral, EnumVal, FloatLiteral, IntegerLiteral, StringLiteral { + true + } + CastExpr { + e.expr.is_lockable_index() + } + ComptimeSelector { + true + } + Ident { + true + } + InfixExpr { + e.left.is_lockable_index() && e.right.is_lockable_index() + } + ParExpr { + e.expr.is_lockable_index() + } + PrefixExpr { + e.right.is_lockable_index() + } + SelectorExpr { + e.expr.is_lockable_index() + } + else { + false + } + } +} + +// returns if an expression can be used in `lock x, y.z, arr[i] {` pub fn (e &Expr) is_lockable() bool { return match e { Ident { true } + IndexExpr { e.left.is_lockable() && e.index !is RangeExpr && e.index.is_lockable_index() } SelectorExpr { e.expr.is_lockable() } ComptimeSelector { true } else { false } diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 4af458ff2..c053f3227 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1687,8 +1687,23 @@ fn (mut c Checker) fail_if_immutable(mut expr ast.Expr) (string, token.Pos) { else {} } if elem_type.has_flag(.shared_f) { - c.error('you have to create a handle and `lock` it to modify `shared` ${kind} element', - expr.left.pos().extend(expr.pos)) + expr_name := ast.Expr(expr).str() + if expr_name !in c.locked_names { + 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.left.pos().extend(expr.pos)) + } else { + c.error('${expr_name} must be added to the `lock` list above', + expr.left.pos().extend(expr.pos)) + } + } else { + c.error('you have to create a handle and `lock` it to modify `shared` ${kind} element', + expr.left.pos().extend(expr.pos)) + } + return '', expr.pos + } + return '', expr.pos } to_lock, pos = c.fail_if_immutable(mut expr.left) } @@ -6849,6 +6864,14 @@ fn (mut c Checker) lock_expr(mut node ast.LockExpr) ast.Type { mut expr_ := node.lockeds[i] e_typ := c.expr(mut expr_) id_name := node.lockeds[i].str() + if expr_ is ast.IndexExpr && (expr_ as ast.IndexExpr).left_type != 0 { + index_expr := expr_ as ast.IndexExpr + left_sym := c.table.final_sym(c.unwrap_generic(index_expr.left_type)) + if left_sym.kind !in [.array, .array_fixed] { + c.error('`${id_name}` cannot be locked - only indexed elements of arrays or fixed arrays are supported', + node.lockeds[i].pos()) + } + } if !e_typ.has_flag(.shared_f) { obj_type := if node.lockeds[i] is ast.Ident { 'variable' } else { 'struct element' } c.error('`${id_name}` must be declared as `shared` ${obj_type} to be locked', @@ -8016,6 +8039,7 @@ fn (mut c Checker) ensure_supported_map_key_types_in_type(typ ast.Type, pos toke // return true if a violation of a shared variable access rule is detected fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what string) bool { mut pos := token.Pos{} + mut shared_expr_name := '' match expr { ast.Ident { if typ.has_flag(.shared_f) { @@ -8060,12 +8084,14 @@ fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what string) } ast.IndexExpr { pos = expr.left.pos().extend(expr.pos) + shared_expr_name = ast.Expr(expr).str() + if typ.has_flag(.shared_f) + && (shared_expr_name in c.rlocked_names || shared_expr_name in c.locked_names) { + return false + } if c.fail_if_unreadable(expr.left, expr.left_type, what) { return true } - if typ.has_flag(.shared_f) && expr.left is ast.SelectorExpr { - return false - } } ast.InfixExpr { pos = expr.left.pos().extend(expr.pos) @@ -8081,8 +8107,14 @@ fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what string) } } if typ.has_flag(.shared_f) { - c.error('you have to create a handle and `rlock` it to use a `shared` element as non-mut ${what}', - pos) + if shared_expr_name != '' && (c.locked_names.len > 0 || c.rlocked_names.len > 0) { + action := if what == 'argument' { 'passed' } else { 'used' } + c.error('`${shared_expr_name}` is `shared` and must be `rlock`ed or `lock`ed to be ${action} as non-mut ${what}', + pos) + } else { + c.error('you have to create a handle and `rlock` it to use a `shared` element as non-mut ${what}', + pos) + } return true } return false diff --git a/vlib/v/parser/lock.v b/vlib/v/parser/lock.v index 566abbfb5..3fe9fcc4f 100644 --- a/vlib/v/parser/lock.v +++ b/vlib/v/parser/lock.v @@ -31,7 +31,7 @@ fn (mut p Parser) lock_expr() ast.LockExpr { p.inside_lock_exprs = false for e in exprs { if !e.is_lockable() { - p.error_with_pos('`${e}` cannot be locked - only `x`, `x.y` or `x.$(y)` are supported', + p.error_with_pos('`${e}` cannot be locked - only `x`, `x.y`, `x.$(y)` or `x[i]` are supported', e.pos()) } lockeds << e diff --git a/vlib/v/tests/concurrency/shared_elem_test.v b/vlib/v/tests/concurrency/shared_elem_test.v index a064957c1..fd9757c25 100644 --- a/vlib/v/tests/concurrency/shared_elem_test.v +++ b/vlib/v/tests/concurrency/shared_elem_test.v @@ -169,6 +169,32 @@ fn test_array_of_shared() { assert f == -17 } +struct FooWithSharedArray { +mut: + bars []shared Xyz +} + +fn test_lock_element_in_array_of_shared_structs() { + mut foo := FooWithSharedArray{} + foo.bars << Xyz{ + n: 3 + } + foo.bars << Xyz{ + n: 7 + } + lock foo.bars[0] { + foo.bars[0].n = 11 + } + first := rlock foo.bars[0] { + foo.bars[0].n + } + second := rlock foo.bars[1] { + foo.bars[1].n + } + assert first == 11 + assert second == 7 +} + // Test for issue #16234: empty shared struct initialized as null causing segmentation fault struct EmptyStruct {} -- 2.39.5