From 40066a5daa1b840f26d872d902936b40b36e5a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kr=C3=BCger?= <45282134+UweKrueger@users.noreply.github.com> Date: Sat, 13 Feb 2021 00:47:37 +0100 Subject: [PATCH] checker: obey [ref_only] tag, allow embedding in other ref struct (#8707) --- cmd/tools/vtest-self.v | 1 + vlib/sync/channels.v | 2 +- vlib/sync/sync_default.c.v | 3 ++ vlib/sync/sync_macos.c.v | 3 ++ vlib/sync/sync_windows.c.v | 3 ++ vlib/v/README.md | 2 +- vlib/v/checker/checker.v | 39 ++++++++++++----- vlib/v/checker/tests/ref_only_struct.out | 35 +++++++++++++++ vlib/v/checker/tests/ref_only_struct.vv | 27 ++++++++++++ vlib/v/gen/c/cgen_test.v | 2 +- vlib/v/tests/ref_struct_test.v | 55 ++++++++++++++++++++++++ vlib/vweb/sse/sse.v | 4 +- 12 files changed, 161 insertions(+), 15 deletions(-) create mode 100644 vlib/v/checker/tests/ref_only_struct.out create mode 100644 vlib/v/checker/tests/ref_only_struct.vv create mode 100644 vlib/v/tests/ref_struct_test.v diff --git a/cmd/tools/vtest-self.v b/cmd/tools/vtest-self.v index 150abd9e39..e3d78ef75c 100644 --- a/cmd/tools/vtest-self.v +++ b/cmd/tools/vtest-self.v @@ -127,6 +127,7 @@ const ( 'vlib/v/tests/module_test.v', 'vlib/v/tests/operator_overloading_with_string_interpolation_test.v', 'vlib/v/tests/orm_sub_struct_test.v', + 'vlib/v/tests/ref_struct_test.v', 'vlib/v/tests/repl/repl_test.v', 'vlib/v/tests/semaphore_test.v', 'vlib/v/tests/shared_arg_test.v', diff --git a/vlib/sync/channels.v b/vlib/sync/channels.v index e260989235..665e24bca4 100644 --- a/vlib/sync/channels.v +++ b/vlib/sync/channels.v @@ -529,7 +529,7 @@ pub fn channel_select(mut channels []&Channel, dir []Direction, mut objrefs []vo assert channels.len == dir.len assert dir.len == objrefs.len mut subscr := []Subscription{len: channels.len} - mut sem := Semaphore{} + mut sem := unsafe { Semaphore{} } sem.init(0) for i, ch in channels { subscr[i].sem = &sem diff --git a/vlib/sync/sync_default.c.v b/vlib/sync/sync_default.c.v index 1fa67aa807..b86703a355 100644 --- a/vlib/sync/sync_default.c.v +++ b/vlib/sync/sync_default.c.v @@ -28,10 +28,12 @@ fn C.sem_timedwait(voidptr, voidptr) int fn C.sem_destroy(voidptr) int // [init_with=new_mutex] // TODO: implement support for this struct attribute, and disallow Mutex{} from outside the sync.new_mutex() function. +[ref_only] pub struct Mutex { mutex C.pthread_mutex_t } +[ref_only] pub struct RwMutex { mutex C.pthread_rwlock_t } @@ -40,6 +42,7 @@ struct RwMutexAttr { attr C.pthread_rwlockattr_t } +[ref_only] struct Semaphore { sem C.sem_t } diff --git a/vlib/sync/sync_macos.c.v b/vlib/sync/sync_macos.c.v index 8c3e320039..bd9fc7669c 100644 --- a/vlib/sync/sync_macos.c.v +++ b/vlib/sync/sync_macos.c.v @@ -31,10 +31,12 @@ fn C.pthread_cond_timedwait(voidptr, voidptr, voidptr) int fn C.pthread_cond_destroy(voidptr) int // [init_with=new_mutex] // TODO: implement support for this struct attribute, and disallow Mutex{} from outside the sync.new_mutex() function. +[ref_only] pub struct Mutex { mutex C.pthread_mutex_t } +[ref_only] pub struct RwMutex { mutex C.pthread_rwlock_t } @@ -49,6 +51,7 @@ struct CondAttr { /* MacOSX has no unnamed semaphores and no `timed_wait()` at all so we emulate the behaviour with other devices */ +[ref_only] struct Semaphore { mtx C.pthread_mutex_t cond C.pthread_cond_t diff --git a/vlib/sync/sync_windows.c.v b/vlib/sync/sync_windows.c.v index 2a3e365b10..4bd9f302b6 100644 --- a/vlib/sync/sync_windows.c.v +++ b/vlib/sync/sync_windows.c.v @@ -22,16 +22,19 @@ type SHANDLE = voidptr //[init_with=new_mutex] // TODO: implement support for this struct attribute, and disallow Mutex{} from outside the sync.new_mutex() function. // `SRWLOCK` is much more performant that `Mutex` on Windows, so use that in both cases since we don't want to share with other processes +[ref_only] pub struct Mutex { mut: mx C.SRWLOCK // mutex handle } +[ref_only] pub struct RwMutex { mut: mx C.SRWLOCK // mutex handle } +[ref_only] struct Semaphore { mtx C.SRWLOCK cond C.CONDITION_VARIABLE diff --git a/vlib/v/README.md b/vlib/v/README.md index 65d46f12e7..2787388c8f 100644 --- a/vlib/v/README.md +++ b/vlib/v/README.md @@ -34,7 +34,7 @@ a new preference is created: ```v import v.pref -pref := pref.Preferences{} +pref := &pref.Preferences{} ``` and a new scope is created: diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 78f38fa694..878ec0a4ef 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -60,6 +60,7 @@ pub mut: inside_unsafe bool inside_const bool inside_anon_fn bool + inside_ref_lit bool skip_flags bool // should `#flag` and `#include` be skipped cur_generic_types []table.Type mut: @@ -372,13 +373,6 @@ pub fn (mut c Checker) interface_decl(decl ast.InterfaceDecl) { field.type_pos) } } - if sym.kind == .struct_ { - info := sym.info as table.Struct - if info.is_ref_only && !field.typ.is_ptr() { - c.error('`$sym.name` type can only be used as a reference: `&$sym.name`', - field.type_pos) - } - } if sym.kind == .map { info := sym.map_info() key_sym := c.table.get_type_symbol(info.key_type) @@ -403,6 +397,11 @@ pub fn (mut c Checker) struct_decl(mut decl ast.StructDecl) { embed_sym := c.table.get_type_symbol(embed.typ) if embed_sym.kind != .struct_ { c.error('`$embed_sym.name` is not a struct', embed.pos) + } else { + info := embed_sym.info as table.Struct + if info.is_ref_only && !embed.typ.is_ptr() { + struct_sym.info.is_ref_only = true + } } } for attr in decl.attrs { @@ -445,8 +444,7 @@ pub fn (mut c Checker) struct_decl(mut decl ast.StructDecl) { if sym.kind == .struct_ { info := sym.info as table.Struct if info.is_ref_only && !field.typ.is_ptr() { - c.error('`$sym.name` type can only be used as a reference: `&$sym.name`', - field.type_pos) + struct_sym.info.is_ref_only = true } } if sym.kind == .map { @@ -545,6 +543,10 @@ pub fn (mut c Checker) struct_init(mut struct_init ast.StructInit) table.Type { c.error('struct `$type_sym.name` is declared with a `[noinit]` attribute, so ' + 'it cannot be initialized with `$type_sym.name{}`', struct_init.pos) } + if info.is_ref_only && !c.inside_ref_lit && !c.inside_unsafe && !struct_init.typ.is_ptr() { + c.error('`$type_sym.name` type can only be used as a reference `&$type_sym.name` or inside a `struct` reference', + struct_init.pos) + } } if type_sym.name.len == 1 && c.cur_fn.generic_params.len == 0 { c.error('unknown struct `$type_sym.name`', struct_init.pos) @@ -2610,7 +2612,14 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { } } if assign_stmt.right_types.len < assign_stmt.left.len { // first type or multi return types added above + old_inside_ref_lit := c.inside_ref_lit + if left is ast.Ident { + if left.info is ast.IdentVar { + c.inside_ref_lit = c.inside_ref_lit || left.info.share == .shared_t + } + } right_type := c.expr(assign_stmt.right[i]) + c.inside_ref_lit = old_inside_ref_lit if assign_stmt.right_types.len == i { assign_stmt.right_types << c.check_expr_opt_call(assign_stmt.right[i], right_type) @@ -2875,7 +2884,14 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { left_first := assign_stmt.left[0] if left_first is ast.Ident { assigned_var := left_first + mut is_shared := false + if left_first.info is ast.IdentVar { + is_shared = left_first.info.share == .shared_t + } + old_inside_ref_lit := c.inside_ref_lit + c.inside_ref_lit = (c.inside_ref_lit || node.op == .amp || is_shared) c.expr(node.right) + c.inside_ref_lit = old_inside_ref_lit if node.right is ast.Ident { if node.right.obj is ast.Var { v := node.right.obj @@ -3921,7 +3937,7 @@ fn (mut c Checker) comptime_call(mut node ast.ComptimeCall) table.Type { if node.is_vweb { // TODO assoc parser bug pref_ := *c.pref - pref2 := pref.Preferences{ + pref2 := &pref.Preferences{ ...pref_ is_vweb: true } @@ -5121,7 +5137,10 @@ pub fn (mut c Checker) postfix_expr(mut node ast.PostfixExpr) table.Type { } pub fn (mut c Checker) prefix_expr(mut node ast.PrefixExpr) table.Type { + old_inside_ref_lit := c.inside_ref_lit + c.inside_ref_lit = c.inside_ref_lit || node.op == .amp right_type := c.expr(node.right) + c.inside_ref_lit = old_inside_ref_lit node.right_type = right_type // TODO: testing ref/deref strategy if node.op == .amp && !right_type.is_ptr() { diff --git a/vlib/v/checker/tests/ref_only_struct.out b/vlib/v/checker/tests/ref_only_struct.out new file mode 100644 index 0000000000..b9ac13b0f9 --- /dev/null +++ b/vlib/v/checker/tests/ref_only_struct.out @@ -0,0 +1,35 @@ +vlib/v/checker/tests/ref_only_struct.vv:18:7: error: `Abc` type can only be used as a reference `&Abc` or inside a `struct` reference + 16 | + 17 | fn main() { + 18 | a := Abc{ n: 3 } + | ~~~~~~~~~~~ + 19 | b := St{ + 20 | Abc{ n: 7 } +vlib/v/checker/tests/ref_only_struct.vv:19:7: error: `St` type can only be used as a reference `&St` or inside a `struct` reference + 17 | fn main() { + 18 | a := Abc{ n: 3 } + 19 | b := St{ + | ~~~ + 20 | Abc{ n: 7 } + 21 | } +vlib/v/checker/tests/ref_only_struct.vv:20:3: error: `Abc` type can only be used as a reference `&Abc` or inside a `struct` reference + 18 | a := Abc{ n: 3 } + 19 | b := St{ + 20 | Abc{ n: 7 } + | ~~~~~~~~~~~ + 21 | } + 22 | x := Qwe{ +vlib/v/checker/tests/ref_only_struct.vv:22:7: error: `Qwe` type can only be used as a reference `&Qwe` or inside a `struct` reference + 20 | Abc{ n: 7 } + 21 | } + 22 | x := Qwe{ + | ~~~~ + 23 | f: 12.25 + 24 | a: Abc{ n: 23 } +vlib/v/checker/tests/ref_only_struct.vv:24:6: error: `Abc` type can only be used as a reference `&Abc` or inside a `struct` reference + 22 | x := Qwe{ + 23 | f: 12.25 + 24 | a: Abc{ n: 23 } + | ~~~~~~~~~~~~ + 25 | } + 26 | println('$a.n $b.n $x.a.n') diff --git a/vlib/v/checker/tests/ref_only_struct.vv b/vlib/v/checker/tests/ref_only_struct.vv new file mode 100644 index 0000000000..0978474d19 --- /dev/null +++ b/vlib/v/checker/tests/ref_only_struct.vv @@ -0,0 +1,27 @@ +[ref_only] +struct Abc { +mut: + n int +} + +struct St { + Abc +} + +struct Qwe { +mut: + f f64 + a Abc +} + +fn main() { + a := Abc{ n: 3 } + b := St{ + Abc{ n: 7 } + } + x := Qwe{ + f: 12.25 + a: Abc{ n: 23 } + } + println('$a.n $b.n $x.a.n') +} diff --git a/vlib/v/gen/c/cgen_test.v b/vlib/v/gen/c/cgen_test.v index 179a5f20ba..1aa5b5de50 100644 --- a/vlib/v/gen/c/cgen_test.v +++ b/vlib/v/gen/c/cgen_test.v @@ -19,7 +19,7 @@ fn test_c_files() { for i in 1 .. (nr_tests + 1) { path := '$vroot/vlib/v/gen/tests/${i}.vv' ctext := os.read_file('$vroot/vlib/v/gen/tests/${i}.c') or { panic(err) } - mut b := builder.new_builder(pref.Preferences{}) + mut b := builder.new_builder(&pref.Preferences{}) b.module_search_paths = ['$vroot/vlib/v/gen/tests/'] mut res := b.gen_c([path]).after('#endbuiltin') if res.contains('string _STR') { diff --git a/vlib/v/tests/ref_struct_test.v b/vlib/v/tests/ref_struct_test.v new file mode 100644 index 0000000000..c98656ef45 --- /dev/null +++ b/vlib/v/tests/ref_struct_test.v @@ -0,0 +1,55 @@ +[ref_only] +struct Abc { +mut: + n int +} + +struct St { + Abc +} + +struct Qwe { +mut: + f f64 + a Abc +} + +struct Rtz { +mut: + f f64 + a &Abc +} + +fn test_ref() { + a := &Abc{ n: 3 } + assert a.n == 3 +} + +fn test_shared() { + shared a := Abc{ n: 4 } + res := rlock a { a.n } + assert res == 4 +} + +fn test_embed_in_ref_struct() { + a := &St{ + Abc{ n: 5 } + } + assert a.n == 5 +} + +fn test_field_in_ref_struct() { + x := &Qwe{ + f: 12.25 + a: Abc{ n: 23 } + } + assert x.a.n == 23 +} + +fn test_ref_field() { + y := Rtz{ + f: -6.25 + a: &Abc{ n: 29 } + } + assert y.a.n == 29 +} diff --git a/vlib/vweb/sse/sse.v b/vlib/vweb/sse/sse.v index 82cd50a04d..f0fe1ffbf3 100644 --- a/vlib/vweb/sse/sse.v +++ b/vlib/vweb/sse/sse.v @@ -35,8 +35,8 @@ pub struct SSEMessage { retry int } -pub fn new_connection(conn &net.TcpConn) SSEConnection { - return SSEConnection{ +pub fn new_connection(conn &net.TcpConn) &SSEConnection { + return &SSEConnection{ conn: conn } }