From df0520b43adcdec805792d0adfe734e2e9510d31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kr=C3=BCger?= <45282134+UweKrueger@users.noreply.github.com> Date: Wed, 3 Feb 2021 00:20:19 +0100 Subject: [PATCH] checker,cgen: make `shared` behave like `mut` inside `lock` - and like non-mut inside `rlock` (#8526) --- vlib/v/checker/checker.v | 97 +++++++++++++++++++++--- vlib/v/checker/tests/shared_bad_args.out | 55 ++++++++++++++ vlib/v/checker/tests/shared_bad_args.vv | 68 +++++++++++++++++ vlib/v/gen/c/fn.v | 14 +++- vlib/v/tests/shared_arg_test.v | 78 +++++++++++++++++++ 5 files changed, 300 insertions(+), 12 deletions(-) create mode 100644 vlib/v/checker/tests/shared_bad_args.out create mode 100644 vlib/v/checker/tests/shared_bad_args.vv create mode 100644 vlib/v/tests/shared_arg_test.v diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index b29af85438..237d991271 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -1033,6 +1033,30 @@ pub fn (mut c Checker) infix_expr(mut infix_expr ast.InfixExpr) table.Type { } } +// returns name and position of variable that needs write lock +fn (mut c Checker) needs_rlock(expr ast.Expr) (string, token.Position) { + mut to_lock := '' // name of variable that needs lock + mut pos := token.Position{} // and its position + match mut expr { + ast.Ident { + if expr.obj is ast.Var { + mut v := expr.obj as ast.Var + if v.typ.share() == .shared_t { + if expr.name !in c.rlocked_names && expr.name !in c.locked_names { + to_lock = expr.name + pos = expr.pos + } + } + } + } + else { + to_lock = '' + pos = token.Position{} + } + } + return to_lock, pos +} + // returns name and position of variable that needs write lock // also sets `is_changed` to true (TODO update the name to reflect this?) fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) { @@ -1465,9 +1489,26 @@ pub fn (mut c Checker) call_method(mut call_expr ast.CallExpr) table.Type { // println('warn $method_name lef.mod=$left_type_sym.mod c.mod=$c.mod') c.error('method `${left_type_sym.name}.$method_name` is private', call_expr.pos) } + rec_share := method.params[0].typ.share() + if rec_share == .shared_t && (c.locked_names.len > 0 || c.rlocked_names.len > 0) { + c.error('method with `shared` receiver cannot be called inside `lock`/`rlock` block', + call_expr.pos) + } if method.params[0].is_mut { - c.fail_if_immutable(call_expr.left) + to_lock, pos := c.fail_if_immutable(call_expr.left) // call_expr.is_mut = true + if to_lock != '' && rec_share != .shared_t { + c.error('$to_lock is `shared` and must be `lock`ed to be passed as `mut`', + pos) + } + } else { + if left_type.has_flag(.shared_f) { + to_lock, pos := c.needs_rlock(call_expr.left) + if to_lock != '' { + c.error('$to_lock is `shared` and must be `rlock`ed or `lock`ed to be used as non-mut receiver', + pos) + } + } } if (!left_type_sym.is_builtin() && method.mod != 'builtin') && method.language == .v && method.no_body { @@ -1535,20 +1576,37 @@ pub fn (mut c Checker) call_method(mut call_expr ast.CallExpr) table.Type { } else { method.params[i + 1] } + param_share := param.typ.share() + if param_share == .shared_t && (c.locked_names.len > 0 || c.rlocked_names.len > 0) { + c.error('method with `shared` arguments cannot be called inside `lock`/`rlock` block', + call_expr.pos) + } if arg.is_mut { - c.fail_if_immutable(arg.expr) + to_lock, pos := c.fail_if_immutable(arg.expr) if !param.is_mut { tok := arg.share.str() c.error('`$call_expr.name` parameter `$param.name` is not `$tok`, `$tok` is not needed`', arg.expr.position()) - } else if param.typ.share() != arg.share { - c.error('wrong shared type', arg.expr.position()) + } else { + if param.typ.share() != arg.share { + c.error('wrong shared type', arg.expr.position()) + } + if to_lock != '' && param_share != .shared_t { + c.error('$to_lock is `shared` and must be `lock`ed to be passed as `mut`', + pos) + } } } else { - if param.is_mut && (!arg.is_mut || param.typ.share() != arg.share) { + if param.is_mut { tok := arg.share.str() - c.warn('`$call_expr.name` parameter `$param.name` is `$tok`, you need to provide `$tok` e.g. `$tok arg${ + c.error('`$call_expr.name` parameter `$param.name` is `$tok`, you need to provide `$tok` e.g. `$tok arg${ i + 1}`', arg.expr.position()) + } else { + to_lock, pos := c.needs_rlock(arg.expr) + if to_lock != '' { + c.error('$to_lock is `shared` and must be `rlock`ed or `locked` to be passed as non-mut argument', + pos) + } } } } @@ -1874,20 +1932,37 @@ pub fn (mut c Checker) call_fn(mut call_expr ast.CallExpr) table.Type { c.error('when forwarding a varg variable, it must be the final argument', call_expr.pos) } + arg_share := arg.typ.share() + if arg_share == .shared_t && (c.locked_names.len > 0 || c.rlocked_names.len > 0) { + c.error('function with `shared` arguments cannot be called inside `lock`/`rlock` block', + call_expr.pos) + } if call_arg.is_mut { - c.fail_if_immutable(call_arg.expr) + to_lock, pos := c.fail_if_immutable(call_arg.expr) if !arg.is_mut { tok := call_arg.share.str() c.error('`$call_expr.name` parameter `$arg.name` is not `$tok`, `$tok` is not needed`', call_arg.expr.position()) - } else if arg.typ.share() != call_arg.share { - c.error('wrong shared type', call_arg.expr.position()) + } else { + if arg.typ.share() != call_arg.share { + c.error('wrong shared type', call_arg.expr.position()) + } + if to_lock != '' && !arg.typ.has_flag(.shared_f) { + c.error('$to_lock is `shared` and must be `lock`ed to be passed as `mut`', + pos) + } } } else { - if arg.is_mut && (!call_arg.is_mut || arg.typ.share() != call_arg.share) { + if arg.is_mut { tok := call_arg.share.str() - c.warn('`$call_expr.name` parameter `$arg.name` is `$tok`, you need to provide `$tok` e.g. `$tok arg${ + c.error('`$call_expr.name` parameter `$arg.name` is `$tok`, you need to provide `$tok` e.g. `$tok arg${ i + 1}`', call_arg.expr.position()) + } else { + to_lock, pos := c.needs_rlock(call_arg.expr) + if to_lock != '' { + c.error('$to_lock is `shared` and must be `rlock`ed or `lock`ed to be passed as non-mut argument', + pos) + } } } // Handle expected interface diff --git a/vlib/v/checker/tests/shared_bad_args.out b/vlib/v/checker/tests/shared_bad_args.out new file mode 100644 index 0000000000..2030b813ec --- /dev/null +++ b/vlib/v/checker/tests/shared_bad_args.out @@ -0,0 +1,55 @@ +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 `locked` 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 + 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 + 49 | } + 50 | w := m_val(m) + 51 | x := a_val(a) + | ^ + 52 | println('$w $x') + 53 | } +vlib/v/checker/tests/shared_bad_args.vv:61:3: error: r is `shared` and must be `lock`ed to be passed as `mut` + 59 | shared r := Qr{ a: 7 } + 60 | lock s { + 61 | r.s_mut(mut s) + | ^ + 62 | } + 63 | lock r { +vlib/v/checker/tests/shared_bad_args.vv:64:15: error: s is `shared` and must be `lock`ed to be passed as `mut` + 62 | } + 63 | lock r { + 64 | r.s_mut(mut s) + | ^ + 65 | } + 66 | m_mut(mut m) +vlib/v/checker/tests/shared_bad_args.vv:66:12: error: m is `shared` and must be `lock`ed to be passed as `mut` + 64 | r.s_mut(mut s) + 65 | } + 66 | m_mut(mut m) + | ^ + 67 | a_mut(mut a) + 68 | } +vlib/v/checker/tests/shared_bad_args.vv:67:12: error: a is `shared` and must be `lock`ed to be passed as `mut` + 65 | } + 66 | m_mut(mut m) + 67 | a_mut(mut a) + | ^ + 68 | } diff --git a/vlib/v/checker/tests/shared_bad_args.vv b/vlib/v/checker/tests/shared_bad_args.vv new file mode 100644 index 0000000000..d3d5dfc37f --- /dev/null +++ b/vlib/v/checker/tests/shared_bad_args.vv @@ -0,0 +1,68 @@ +struct St { +mut: + a int +} + +struct Qr { +mut: + a int +} + +fn (mut r Qr) s_mut(mut s St) { + r.a = 5 + s.a = 7 +} + +fn (r Qr) s_val(s St) int { + return r.a * s.a +} + +fn m_mut(mut a map[string]f64) { + a['yxcv'] = -2.25 +} + +fn m_val(a map[string]f64) f64 { + x := a['yxcv'] + return x +} + +fn a_mut(mut a []int) { + a[2] = 42 +} + +fn a_val(a []int) int { + return a[1] +} + +fn test_shared_as_value() { + shared s := St{ a: 5 } + shared a := [3, 4, 6, 13, -23] + shared m := {'qw': 12.75, 'yxcv': -3.125, 'poiu': 88.0625} + shared r := Qr{ a: 7 } + lock s { + u := r.s_val(s) + println(u) + } + lock r { + v := r.s_val(s) + println(v) + } + w := m_val(m) + x := a_val(a) + println('$w $x') +} + +fn test_shared_as_mut() { + shared s := St{ a: 5 } + shared a := [3, 4, 6, 13, -23] + shared m := {'qw': 12.75, 'yxcv': -3.125, 'poiu': 88.0625} + shared r := Qr{ a: 7 } + lock s { + r.s_mut(mut s) + } + lock r { + r.s_mut(mut s) + } + m_mut(mut m) + a_mut(mut a) +} diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index e40756aee3..8bcafc0177 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -522,7 +522,9 @@ fn (mut g Gen) method_call(node ast.CallExpr) { } } else if !node.receiver_type.is_ptr() && node.left_type.is_ptr() && node.name != 'str' && node.from_embed_type == 0 { - g.write('/*rec*/*') + if !node.left_type.has_flag(.shared_f) { + g.write('/*rec*/*') + } } if g.is_autofree && node.free_receiver && !g.inside_lambda && !g.is_builtin_mod { // The receiver expression needs to be freed, use the temp var. @@ -540,6 +542,9 @@ fn (mut g Gen) method_call(node ast.CallExpr) { } g.write(embed_name) } + if node.left_type.has_flag(.shared_f) && !node.receiver_type.is_ptr() { + g.write('->val') + } } if has_cast { g.write(')') @@ -958,6 +963,13 @@ fn (mut g Gen) ref_or_deref_arg(arg ast.CallArg, expected_type table.Type) { g.write('(voidptr)&/*qq*/') } } + } else if arg.typ.has_flag(.shared_f) && !expected_type.has_flag(.shared_f) { + if expected_type.is_ptr() { + g.write('&') + } + g.expr(arg.expr) + g.write('->val') + return } g.expr_with_cast(arg.expr, arg.typ, expected_type) } diff --git a/vlib/v/tests/shared_arg_test.v b/vlib/v/tests/shared_arg_test.v new file mode 100644 index 0000000000..43ffa513eb --- /dev/null +++ b/vlib/v/tests/shared_arg_test.v @@ -0,0 +1,78 @@ +struct St { +mut: + a int +} + +struct Qr { +mut: + a int +} + +fn (mut r Qr) s_mut(mut s St) { + r.a = 5 + s.a = 7 +} + +fn (r Qr) s_val(s St) int { + return r.a * s.a +} + +fn m_mut(mut a map[string]f64) { + a['yxcv'] = -2.25 +} + +fn m_val(a map[string]f64) f64 { + x := a['yxcv'] + return x +} + +fn a_mut(mut a []int) { + a[2] = 42 +} + +fn a_val(a []int) int { + return a[1] +} + +fn test_shared_as_value() { + shared s := St{ a: 5 } + shared a := [3, 4, 6, 13, -23] + shared m := {'qw': 12.75, 'yxcv': -3.125, 'poiu': 88.0625} + shared r := Qr{ a: 7 } + rlock s, r { + u := r.s_val(s) + assert u == 35 + } + lock s, r { + v := r.s_val(s) + assert v == 35 + } + rlock m { + w := m_val(m) + assert w == -3.125 + } + lock a { + x := a_val(a) + assert x == 4 + } +} + +fn test_shared_as_mut() { + shared s := St{ a: 5 } + shared a := [3, 4, 6, 13, -23] + shared m := {'qw': 12.75, 'yxcv': -3.125, 'poiu': 88.0625} + shared r := Qr{ a: 7 } + lock s, r { + r.s_mut(mut s) + x := r.a * s.a + assert x == 35 + } + lock a, m { + m_mut(mut m) + a_mut(mut a) + y := m['yxcv'] + z := a[2] + assert y == -2.25 + assert z == 42 + } +}