From 8f2f377cb1006d63f05af03a01dfe5abf81cc19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kr=C3=BCger?= <45282134+UweKrueger@users.noreply.github.com> Date: Mon, 21 Jun 2021 06:10:10 +0200 Subject: [PATCH] v.checker,v.gen.c: extend auto heap mechanism to objects used as interfaces (#10529) --- vlib/v/ast/ast.v | 18 ++--- vlib/v/checker/checker.v | 106 +++++++++++++++++++++++------ vlib/v/gen/c/cgen.v | 10 +-- vlib/v/gen/c/fn.v | 12 +++- vlib/v/tests/heap_interface_test.v | 53 +++++++++++++++ 5 files changed, 162 insertions(+), 37 deletions(-) create mode 100644 vlib/v/tests/heap_interface_test.v diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 238204b965..5e4c8a19a4 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -179,7 +179,6 @@ pub: pos token.Position type_pos token.Position comments []Comment - default_expr Expr has_default_expr bool attrs []Attr is_pub bool @@ -187,6 +186,7 @@ pub: is_mut bool is_global bool pub mut: + default_expr Expr default_expr_typ Type name string typ Type @@ -283,12 +283,12 @@ pub mut: pub struct StructInitField { pub: - expr Expr pos token.Position name_pos token.Position comments []Comment next_comments []Comment pub mut: + expr Expr name string typ Type expected_type Type @@ -449,9 +449,9 @@ pub struct CallArg { pub: is_mut bool share ShareType - expr Expr comments []Comment pub mut: + expr Expr typ Type is_tmp_autofree bool // this tells cgen that a tmp variable has to be used for the arg expression in order to free it after the call pos token.Position @@ -462,9 +462,9 @@ pub mut: pub struct Return { pub: pos token.Position - exprs []Expr comments []Comment pub mut: + exprs []Expr types []Type } @@ -769,13 +769,13 @@ pub mut: pub struct MatchBranch { pub: - exprs []Expr // left side ecmnts [][]Comment // inline comments for each left side expr stmts []Stmt // right side pos token.Position is_else bool post_comments []Comment // comments below ´... }´ pub mut: + exprs []Expr // left side scope &Scope } @@ -1093,11 +1093,11 @@ pub: // `string(x,y)`, while skipping the real pointer casts like `&string(x)`. pub struct CastExpr { pub: - expr Expr // `buf` in `string(buf, n)` - arg Expr // `n` in `string(buf, n)` - typ Type // `string` TODO rename to `type_to_cast_to` - pos token.Position + arg Expr // `n` in `string(buf, n)` + typ Type // `string` TODO rename to `type_to_cast_to` + pos token.Position pub mut: + expr Expr // `buf` in `string(buf, n)` typname string // TypeSymbol.name expr_type Type // `byteptr` has_arg bool diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index e9bf1be230..3200d97392 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -585,8 +585,17 @@ pub fn (mut c Checker) struct_decl(mut decl ast.StructDecl) { } struct_sym.info.fields[i].default_expr_typ = field_expr_type c.check_expected(field_expr_type, field.typ) or { - if !(sym.kind == .interface_ - && c.type_implements(field_expr_type, field.typ, field.pos)) { + if sym.kind == .interface_ + && c.type_implements(field_expr_type, field.typ, field.pos) { + if !field_expr_type.is_ptr() && !field_expr_type.is_pointer() + && !c.inside_unsafe { + field_expr_type_sym := c.table.get_type_symbol(field_expr_type) + if field_expr_type_sym.kind != .interface_ { + c.mark_as_referenced(mut &decl.fields[i].default_expr, + true) + } + } + } else { c.error('incompatible initializer for field `$field.name`: $err.msg', field.default_expr.position()) } @@ -793,7 +802,7 @@ pub fn (mut c Checker) struct_init(mut node ast.StructInit) ast.Type { } } mut inited_fields := []string{} - for i, field in node.fields { + for i, mut field in node.fields { mut info_field := ast.StructField{} mut embed_type := ast.Type(0) mut is_embed := false @@ -865,7 +874,12 @@ pub fn (mut c Checker) struct_init(mut node ast.StructInit) ast.Type { } expr_type_sym := c.table.get_type_symbol(expr_type) if field_type_sym.kind == .interface_ { - c.type_implements(expr_type, info_field.typ, field.pos) + if c.type_implements(expr_type, info_field.typ, field.pos) { + if !expr_type.is_ptr() && !expr_type.is_pointer() + && expr_type_sym.kind != .interface_ && !c.inside_unsafe { + c.mark_as_referenced(mut &field.expr, true) + } + } } else if expr_type != ast.void_type && expr_type_sym.kind != .placeholder { c.check_expected(expr_type, info_field.typ) or { c.error('cannot assign to field `$info_field.name`: $err.msg', @@ -1255,7 +1269,12 @@ pub fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type { if left_value_sym.kind == .interface_ { if right_final.kind != .array { // []Animal << Cat - c.type_implements(right_type, left_value_type, right_pos) + if c.type_implements(right_type, left_value_type, right_pos) { + if !right_type.is_ptr() && !right_type.is_pointer() && !c.inside_unsafe + && right_sym.kind != .interface_ { + c.mark_as_referenced(mut &node.right, true) + } + } } else { // []Animal << []Cat c.type_implements(c.table.value_type(right_type), left_value_type, @@ -1903,7 +1922,7 @@ pub fn (mut c Checker) method_call(mut call_expr ast.CallExpr) ast.Type { // } // call_expr.args << method.args[0].typ // call_expr.exp_arg_types << method.args[0].typ - for i, arg in call_expr.args { + for i, mut arg in call_expr.args { if i > 0 || exp_arg_typ == ast.Type(0) { exp_arg_typ = if method.is_variadic && i >= method.params.len - 1 { method.params[method.params.len - 1].typ @@ -1933,7 +1952,14 @@ pub fn (mut c Checker) method_call(mut call_expr ast.CallExpr) ast.Type { } // Handle expected interface if final_arg_sym.kind == .interface_ { - c.type_implements(got_arg_typ, exp_arg_typ, arg.expr.position()) + if c.type_implements(got_arg_typ, exp_arg_typ, arg.expr.position()) { + if !got_arg_typ.is_ptr() && !got_arg_typ.is_pointer() && !c.inside_unsafe { + got_arg_typ_sym := c.table.get_type_symbol(got_arg_typ) + if got_arg_typ_sym.kind != .interface_ { + c.mark_as_referenced(mut &arg.expr, true) + } + } + } continue } if method.generic_names.len > 0 { @@ -2466,7 +2492,7 @@ pub fn (mut c Checker) fn_call(mut call_expr ast.CallExpr) ast.Type { call_expr.expected_arg_types << param.typ } } - for i, call_arg in call_expr.args { + for i, mut call_arg in call_expr.args { param := if func.is_variadic && i >= func.params.len - 1 { func.params[func.params.len - 1] } else { @@ -2521,7 +2547,12 @@ pub fn (mut c Checker) fn_call(mut call_expr ast.CallExpr) ast.Type { } // Handle expected interface if final_param_sym.kind == .interface_ { - c.type_implements(typ, param.typ, call_arg.expr.position()) + if c.type_implements(typ, param.typ, call_arg.expr.position()) { + if !typ.is_ptr() && !typ.is_pointer() && !c.inside_unsafe + && typ_sym.kind != .interface_ { + c.mark_as_referenced(mut &call_arg.expr, true) + } + } continue } c.check_expected_call_arg(typ, c.unwrap_generic(param.typ), call_expr.language) or { @@ -3086,7 +3117,12 @@ pub fn (mut c Checker) return_stmt(mut node ast.Return) { continue } if exp_typ_sym.kind == .interface_ { - c.type_implements(got_typ, exp_type, node.pos) + if c.type_implements(got_typ, exp_type, node.pos) { + if !got_typ.is_ptr() && !got_typ.is_pointer() && got_typ_sym.kind != .interface_ + && !c.inside_unsafe { + c.mark_as_referenced(mut &node.exprs[i], true) + } + } continue } c.error('cannot use `$got_typ_sym.name` as type `${c.table.type_to_str(exp_type)}` in return argument', @@ -3653,7 +3689,12 @@ pub fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) { } } if left_sym.kind == .interface_ { - c.type_implements(right_type, left_type, right.position()) + if c.type_implements(right_type, left_type, right.position()) { + if !right_type.is_ptr() && !right_type.is_pointer() && right_sym.kind != .interface_ + && !c.inside_unsafe { + c.mark_as_referenced(mut &node.right[i], true) + } + } } } // this needs to run after the assign stmt left exprs have been run through checker @@ -3812,7 +3853,7 @@ pub fn (mut c Checker) array_init(mut array_init ast.ArrayInit) ast.Type { // if expecting_interface_array { // println('ex $c.expected_type') // } - for i, expr in array_init.exprs { + for i, mut expr in array_init.exprs { typ := c.check_expr_opt_call(expr, c.expr(expr)) array_init.expr_types << typ // The first element's type @@ -3822,6 +3863,12 @@ pub fn (mut c Checker) array_init(mut array_init ast.ArrayInit) ast.Type { c.expected_type = elem_type c.type_implements(typ, elem_type, expr.position()) } + if !typ.is_ptr() && !typ.is_pointer() && !c.inside_unsafe { + typ_sym := c.table.get_type_symbol(typ) + if typ_sym.kind != .interface_ { + c.mark_as_referenced(mut &expr, true) + } + } continue } // The first element's type @@ -5014,7 +5061,12 @@ pub fn (mut c Checker) cast_expr(mut node ast.CastExpr) ast.Type { c.error('cannot cast `$type_name` to struct', node.pos) } } else if to_type_sym.kind == .interface_ { - c.type_implements(node.expr_type, node.typ, node.pos) + if c.type_implements(node.expr_type, node.typ, node.pos) { + if !node.expr_type.is_ptr() && !node.expr_type.is_pointer() + && from_type_sym.kind != .interface_ && !c.inside_unsafe { + c.mark_as_referenced(mut &node.expr, true) + } + } } else if node.typ == ast.bool_type && !c.inside_unsafe { c.error('cannot cast to bool - use e.g. `some_int != 0` instead', node.pos) } else if node.expr_type == ast.none_type && !node.typ.has_flag(.optional) { @@ -5529,7 +5581,7 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, cond_type_sym ast.TypeSym for branch_i, _ in node.branches { mut branch := node.branches[branch_i] mut expr_types := []ast.TypeNode{} - for expr in branch.exprs { + for k, expr in branch.exprs { mut key := '' if expr is ast.RangeExpr { mut low := i64(0) @@ -5599,7 +5651,14 @@ fn (mut c Checker) match_exprs(mut node ast.MatchExpr, cond_type_sym ast.TypeSym // Current solution is to move expr.position() to its own statement // c.type_implements(expr_type, c.expected_type, expr.position()) expr_pos := expr.position() - c.type_implements(expr_type, c.expected_type, expr_pos) + if c.type_implements(expr_type, c.expected_type, expr_pos) { + if !expr_type.is_ptr() && !expr_type.is_pointer() && !c.inside_unsafe { + expr_type_sym := c.table.get_type_symbol(expr_type) + if expr_type_sym.kind != .interface_ { + c.mark_as_referenced(mut &branch.exprs[k], true) + } + } + } } else if mut cond_type_sym.info is ast.SumType { if expr_type !in cond_type_sym.info.variants { expr_str := c.table.type_to_str(expr_type) @@ -6388,7 +6447,7 @@ pub fn (mut c Checker) postfix_expr(mut node ast.PostfixExpr) ast.Type { return typ } -pub fn (mut c Checker) mark_as_referenced(mut node ast.Expr) { +pub fn (mut c Checker) mark_as_referenced(mut node ast.Expr, as_interface bool) { match mut node { ast.Ident { if mut node.obj is ast.Var { @@ -6404,7 +6463,12 @@ pub fn (mut c Checker) mark_as_referenced(mut node ast.Expr) { 'wrapping the `$type_sym.name` object in a `struct` declared as `[heap]`' } if !c.pref.translated { - c.error('`$node.name` cannot be referenced outside `unsafe` blocks as it might be stored on stack. Consider ${suggestion}.', + mischief := if as_interface { + 'used as interface object' + } else { + 'referenced' + } + c.error('`$node.name` cannot be $mischief outside `unsafe` blocks as it might be stored on stack. Consider ${suggestion}.', node.pos) } } else if type_sym.kind == .array_fixed { @@ -6427,11 +6491,11 @@ pub fn (mut c Checker) mark_as_referenced(mut node ast.Expr) { } ast.SelectorExpr { if !node.expr_type.is_ptr() { - c.mark_as_referenced(mut &node.expr) + c.mark_as_referenced(mut &node.expr, as_interface) } } ast.IndexExpr { - c.mark_as_referenced(mut &node.left) + c.mark_as_referenced(mut &node.left, as_interface) } else {} } @@ -6496,12 +6560,12 @@ pub fn (mut c Checker) prefix_expr(mut node ast.PrefixExpr) ast.Type { } } if !c.inside_fn_arg && !c.inside_unsafe { - c.mark_as_referenced(mut &node.right) + c.mark_as_referenced(mut &node.right, false) } return right_type.to_ptr() } else if node.op == .amp && node.right !is ast.CastExpr { if !c.inside_fn_arg && !c.inside_unsafe { - c.mark_as_referenced(mut &node.right) + c.mark_as_referenced(mut &node.right, false) } if node.right.is_auto_deref_var() { return right_type diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index ae3343c482..605e160b26 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -2422,7 +2422,7 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { is_auto_heap = left.obj.is_auto_heap } } - styp := if ident.name in g.defer_vars { '' } else { g.typ(var_type) } + styp := g.typ(var_type) mut is_fixed_array_init := false mut has_val := false match val { @@ -2580,9 +2580,11 @@ fn (mut g Gen) gen_assign_stmt(assign_stmt ast.AssignStmt) { if is_inside_ternary { g.out.write_string(util.tabs(g.indent - g.inside_ternary)) } - g.write('$styp ') - if is_auto_heap { - g.write('*') + if ident.name !in g.defer_vars { + g.write('$styp ') + if is_auto_heap { + g.write('*') + } } } if left is ast.Ident || left is ast.SelectorExpr { diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index 32f7e78c46..7243cf47b5 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -286,11 +286,17 @@ fn (mut g Gen) gen_fn_decl(node &ast.FnDecl, skip bool) { if var.name in fargs || var.kind == .constant { continue } - if var.info is ast.IdentVar { - info := var.info + if var.kind == .variable { if var.name !in g.defer_vars { g.defer_vars << var.name - g.writeln('${g.typ(info.typ)} $var.name;') + mut deref := '' + if v := var.scope.find_var(var.name) { + if v.is_auto_heap { + deref = '*' + } + } + info := var.obj as ast.Var + g.writeln('${g.typ(info.typ)}$deref $var.name;') } } } diff --git a/vlib/v/tests/heap_interface_test.v b/vlib/v/tests/heap_interface_test.v new file mode 100644 index 0000000000..03ce7e72cf --- /dev/null +++ b/vlib/v/tests/heap_interface_test.v @@ -0,0 +1,53 @@ +// declare interface +interface MyInterface { + val() int +} + +// define struct type +struct St { +mut: + n int +} + +// make the struct type implement the interface +fn (x St) val() int { + return x.n +} + +fn owerwrite_stack() f64 { + a := 12.5 + b := 3.5 + c := a + b + return c +} + +// nothing special so far, but now some functions that return an interfaces +// these used to cause memory corruptions, but work with this PR: +fn gen_interface() MyInterface { + x := St{ + n: -123 + } // `x`will be allocated on heap + return x // because an interface object is returned here that contains the address of x +} + +fn return_interface(x St) MyInterface { + // x will be copied to stack (requires #10528) + return x // because it's address is returned inside the interface +} + +fn test_gen_interface() { + i1 := gen_interface() + d := owerwrite_stack() + assert i1.val() == -123 + assert d == 16.0 +} + +fn test_convert_to_interface() { + x := St{ + n: 5 + } + i2 := return_interface(x) + d := owerwrite_stack() + assert i2.val() == 5 + assert d == 16.0 +}