From f32a76b268b91ab558b7fca6f1eb5c22bf0bbf9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kr=C3=BCger?= <45282134+UweKrueger@users.noreply.github.com> Date: Sun, 20 Jun 2021 17:40:24 +0200 Subject: [PATCH] all: promote value type function arguments to heap if necessary (#10528) --- vlib/v/checker/checker.v | 21 +++++- .../tests/function_missing_return_type.out | 7 -- vlib/v/gen/c/cgen.v | 6 +- vlib/v/gen/c/fn.v | 30 ++++++-- vlib/v/parser/fn.v | 10 +-- vlib/v/tests/break_in_lock_test.v | 9 ++- vlib/v/tests/heap_struct_test.v | 73 +++++++++++++++++++ vlib/v/tests/return_in_lock_test.v | 9 ++- 8 files changed, 130 insertions(+), 35 deletions(-) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index b68fd7418c..e9bf1be230 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -6411,9 +6411,14 @@ pub fn (mut c Checker) mark_as_referenced(mut node ast.Expr) { c.error('cannot reference fixed array `$node.name` outside `unsafe` blocks as it is supposed to be stored on stack', node.pos) } else { - if type_sym.kind == .struct_ { - info := type_sym.info as ast.Struct - if !info.is_heap { + match type_sym.kind { + .struct_ { + info := type_sym.info as ast.Struct + if !info.is_heap { + node.obj.is_auto_heap = true + } + } + else { node.obj.is_auto_heap = true } } @@ -7291,6 +7296,16 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) { // Make sure all types are valid for arg in node.params { c.ensure_type_exists(arg.typ, arg.type_pos) or { return } + if !arg.typ.is_ptr() { // value parameter, i.e. on stack - check for `[heap]` + arg_typ_sym := c.table.get_type_symbol(arg.typ) + if arg_typ_sym.kind == .struct_ { + info := arg_typ_sym.info as ast.Struct + if info.is_heap { // set auto_heap to promote value parameter + mut v := node.scope.find_var(arg.name) or { continue } + v.is_auto_heap = true + } + } + } } } if node.language == .v && node.name.after_char(`.`) == 'init' && !node.is_method diff --git a/vlib/v/checker/tests/function_missing_return_type.out b/vlib/v/checker/tests/function_missing_return_type.out index cf9872854b..73292af5ea 100644 --- a/vlib/v/checker/tests/function_missing_return_type.out +++ b/vlib/v/checker/tests/function_missing_return_type.out @@ -3,13 +3,6 @@ vlib/v/checker/tests/function_missing_return_type.vv:1:1: error: missing return | ~~~~~~~~~~ 2 | } 3 | -vlib/v/checker/tests/function_missing_return_type.vv:14:11: error: `s` cannot be referenced outside `unsafe` blocks as it might be stored on stack. Consider declaring `Abc` as `[heap]`. - 12 | fn (s Abc) abc() &int { - 13 | if true { - 14 | return &s.x - | ^ - 15 | } - 16 | s.panic(@FN) vlib/v/checker/tests/function_missing_return_type.vv:12:1: error: missing return at end of function `abc` 10 | } 11 | diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 616e742b03..ae3343c482 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -2444,7 +2444,7 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { ret_styp := g.typ(val.decl.return_type) g.write('$ret_styp (*$ident.name) (') def_pos := g.definitions.len - g.fn_args(val.decl.params, val.decl.is_variadic) + g.fn_args(val.decl.params, val.decl.is_variadic, voidptr(0)) g.definitions.go_back(g.definitions.len - def_pos) g.write(') = ') } else { @@ -2572,7 +2572,7 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { ret_styp := g.typ(func.func.return_type) g.write('$ret_styp (*${g.get_ternary_name(ident.name)}) (') def_pos := g.definitions.len - g.fn_args(func.func.params, func.func.is_variadic) + g.fn_args(func.func.params, func.func.is_variadic, voidptr(0)) g.definitions.go_back(g.definitions.len - def_pos) g.write(')') } else { @@ -6150,7 +6150,7 @@ static inline $interface_name I_${cctype}_to_Interface_${interface_name}($cctype ...params[0] typ: params[0].typ.set_nr_muls(1) } - fargs, _ := g.fn_args(params, false) // second argument is ignored anyway + fargs, _, _ := g.fn_args(params, false, voidptr(0)) // second argument is ignored anyway methods_wrapper.write_string(g.out.cut_last(g.out.len - params_start_pos)) methods_wrapper.writeln(') {') methods_wrapper.write_string('\t') diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index 55fc1c430d..32f7e78c46 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -264,7 +264,7 @@ fn (mut g Gen) gen_fn_decl(node &ast.FnDecl, skip bool) { g.write(fn_header) } arg_start_pos := g.out.len - fargs, fargtypes := g.fn_args(node.params, node.is_variadic) + fargs, fargtypes, heap_promoted := g.fn_args(node.params, node.is_variadic, node.scope) arg_str := g.out.after(arg_start_pos) if node.no_body || ((g.pref.use_cache && g.pref.build_mode != .build_module) && node.is_builtin && !g.is_test) || skip { @@ -275,6 +275,11 @@ fn (mut g Gen) gen_fn_decl(node &ast.FnDecl, skip bool) { } g.definitions.writeln(');') g.writeln(') {') + for i, is_promoted in heap_promoted { + if is_promoted { + g.writeln('${fargtypes[i]}* ${fargs[i]} = HEAP(${fargtypes[i]}, _v_toheap_${fargs[i]});') + } + } for defer_stmt in node.defer_stmts { g.writeln('bool ${g.defer_flag_var(defer_stmt)} = false;') for var in defer_stmt.defer_vars { @@ -386,15 +391,16 @@ fn (mut g Gen) write_defer_stmts_when_needed() { } // fn decl args -fn (mut g Gen) fn_args(args []ast.Param, is_variadic bool) ([]string, []string) { +fn (mut g Gen) fn_args(args []ast.Param, is_variadic bool, scope &ast.Scope) ([]string, []string, []bool) { mut fargs := []string{} mut fargtypes := []string{} + mut heap_promoted := []bool{} if args.len == 0 { // in C, `()` is untyped, unlike `(void)` g.write('void') } for i, arg in args { - caname := if arg.name == '_' { g.new_tmp_var() } else { c_name(arg.name) } + mut caname := if arg.name == '_' { g.new_tmp_var() } else { c_name(arg.name) } typ := g.unwrap_generic(arg.typ) arg_type_sym := g.table.get_type_symbol(typ) mut arg_type_name := g.typ(typ) // util.no_dots(arg_type_sym.name) @@ -403,22 +409,34 @@ fn (mut g Gen) fn_args(args []ast.Param, is_variadic bool) ([]string, []string) func := info.func g.write('${g.typ(func.return_type)} (*$caname)(') g.definitions.write_string('${g.typ(func.return_type)} (*$caname)(') - g.fn_args(func.params, func.is_variadic) + g.fn_args(func.params, func.is_variadic, voidptr(0)) g.write(')') g.definitions.write_string(')') } else { - s := '$arg_type_name $caname' + mut heap_prom := false + if scope != voidptr(0) { + if arg.name != '_' { + if v := scope.find_var(arg.name) { + if !v.is_stack_obj && v.is_auto_heap { + heap_prom = true + } + } + } + } + var_name_prefix := if heap_prom { '_v_toheap_' } else { '' } + s := '$arg_type_name $var_name_prefix$caname' g.write(s) g.definitions.write_string(s) fargs << caname fargtypes << arg_type_name + heap_promoted << heap_prom } if i < args.len - 1 { g.write(', ') g.definitions.write_string(', ') } } - return fargs, fargtypes + return fargs, fargtypes, heap_promoted } fn (mut g Gen) call_expr(node ast.CallExpr) { diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 89bc039a09..9a03e61479 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -310,10 +310,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl { scope: 0 } } - mut is_stack_obj := true - if param.typ.has_flag(.shared_f) { - is_stack_obj = false - } + is_stack_obj := !param.typ.has_flag(.shared_f) && (param.is_mut || param.typ.is_ptr()) p.scope.register(ast.Var{ name: param.name typ: param.typ @@ -628,10 +625,7 @@ fn (mut p Parser) anon_fn() ast.AnonFn { if arg.name.len == 0 { p.error_with_pos('use `_` to name an unused parameter', arg.pos) } - mut is_stack_obj := true - if arg.typ.has_flag(.shared_f) { - is_stack_obj = false - } + is_stack_obj := !arg.typ.has_flag(.shared_f) && (arg.is_mut || arg.typ.is_ptr()) p.scope.register(ast.Var{ name: arg.name typ: arg.typ diff --git a/vlib/v/tests/break_in_lock_test.v b/vlib/v/tests/break_in_lock_test.v index 552ac313e3..d40079c2c9 100644 --- a/vlib/v/tests/break_in_lock_test.v +++ b/vlib/v/tests/break_in_lock_test.v @@ -6,7 +6,8 @@ mut: } const ( - sleep_time = time.millisecond * 50 + run_time = time.millisecond * 200 // must be big enough to ensure threads have started + sleep_time = time.millisecond * 250 // some tolerance added ) fn test_return_lock() { @@ -16,12 +17,12 @@ fn test_return_lock() { go fn (shared s AA, start time.Time) { for { reader(shared s) - if time.now() - start > sleep_time { + if time.now() - start > run_time { exit(0) } } }(shared s, start) - time.sleep(sleep_time * 2) + time.sleep(sleep_time) assert false } @@ -30,7 +31,7 @@ fn printer(shared s AA, start time.Time) { lock s { assert s.b in ['0', '1', '2', '3', '4', '5'] } - if time.now() - start > time.millisecond * 50 { + if time.now() - start > run_time { exit(0) } } diff --git a/vlib/v/tests/heap_struct_test.v b/vlib/v/tests/heap_struct_test.v index e3a488c13c..a407b561d9 100644 --- a/vlib/v/tests/heap_struct_test.v +++ b/vlib/v/tests/heap_struct_test.v @@ -58,4 +58,77 @@ fn test_ref_struct() { assert u.n == 3 assert v.n == 7 assert w.a.n == 23 + assert d == 16.0 +} + +fn return_heap_obj_value_as_ref(qpast Qwe) &Qwe { + return &qpast +} + +fn test_value_ref_heap_struct() { + mut x := Qwe{ + f: -13.25 + a: Abc{ + n: -129 + } + } + y := return_heap_obj_value_as_ref(x) + x.f = 22.0625 + d := owerwrite_stack() + assert typeof(y).name == '&Qwe' + assert x.f == 22.0625 + assert x.a.n == -129 + assert y.f == -13.25 + assert y.a.n == -129 + assert d == 16.0 +} + +struct NotHeap { +mut: + f f64 +} + +fn return_struct_value_as_ref(q NotHeap) &NotHeap { + return &q +} + +fn test_value_ref_struct() { + mut x := NotHeap{ + f: -17.125 + } + y := return_struct_value_as_ref(x) + x.f = 91.0625 + d := owerwrite_stack() + assert typeof(y).name == '&NotHeap' + assert y.f == -17.125 + assert x.f == 91.0625 + assert d == 16.0 +} + +fn get_int_ref() &int { + i := 49154 + return &i +} + +fn test_int_ref() { + iptr := get_int_ref() + assert typeof(iptr).name == '&int' + d := owerwrite_stack() + assert *iptr == 49154 + assert d == 16.0 +} + +fn pass_f64_as_ref(f f64) &f64 { + return &f +} + +fn test_value_as_ref() { + mut x := -31.75 + y := pass_f64_as_ref(x) + assert typeof(y).name == '&f64' + x = 23.0625 + d := owerwrite_stack() + assert x == 23.0625 + assert *y == -31.75 + assert d == 16.0 } diff --git a/vlib/v/tests/return_in_lock_test.v b/vlib/v/tests/return_in_lock_test.v index a7e805143f..feb37a5d32 100644 --- a/vlib/v/tests/return_in_lock_test.v +++ b/vlib/v/tests/return_in_lock_test.v @@ -6,7 +6,8 @@ mut: } const ( - sleep_time = time.millisecond * 50 + run_time = time.millisecond * 200 // must be big enough to ensure threads have started + sleep_time = time.millisecond * 250 // some tolerance added ) fn test_return_lock() { @@ -16,12 +17,12 @@ fn test_return_lock() { go fn (shared s AA, start time.Time) { for { reader(shared s) - if time.now() - start > sleep_time { + if time.now() - start > run_time { exit(0) } } }(shared s, start) - time.sleep(sleep_time * 2) + time.sleep(sleep_time) assert false } @@ -30,7 +31,7 @@ fn printer(shared s AA, start time.Time) { lock s { assert s.b in ['0', '1', '2', '3', '4', '5'] } - if time.now() - start > time.millisecond * 50 { + if time.now() - start > run_time { exit(0) } }